From af8f5f5ebc950ae83e0d65d4d9cd23a06e57ab1f Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 14 Feb 2018 22:29:11 -0700 Subject: [PATCH] Better handling of NTP-based clocks. Now that we have a nice Clock abstraction, we can use it to represent a clock backed by an NTP fix. (This makes testing logic much easier to write.) We now rely completely on NetworkTimeUpdateService to keep our NTP fix up to date, instead of trying to refresh in the middle of critical paths which could trigger random ANRs. Add internal FallbackClock to make it easier to handle missing NTP fixes. Add internal SimpleClock to let implementers focus on single millis() method. Test: bit FrameworksNetTests:com.android.server.net.NetworkStatsServiceTest Test: bit FrameworksServicesTests:com.android.server.NetworkPolicyManagerServiceTest Bug: 69714690, 72320957 Change-Id: Ic32cdcbe093d08b73b0e4b23d6910b23ea8e1968 Exempt-From-Owner-Approval: approved in previous PS --- .../server/net/NetworkStatsServiceTest.java | 65 ++++--------------- 1 file changed, 12 insertions(+), 53 deletions(-) diff --git a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java index 0f26edb280..b1b05e8b86 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java @@ -87,12 +87,11 @@ import android.os.Looper; import android.os.Message; import android.os.Messenger; import android.os.PowerManager; +import android.os.SimpleClock; import android.support.test.InstrumentationRegistry; -import android.support.test.runner.AndroidJUnit4; import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; import android.telephony.TelephonyManager; -import android.util.Log; -import android.util.TrustedTime; import com.android.internal.net.VpnInfo; import com.android.internal.util.test.BroadcastInterceptingContext; @@ -111,6 +110,8 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.io.File; +import java.time.Clock; +import java.time.ZoneOffset; import java.util.Objects; /** @@ -155,7 +156,6 @@ public class NetworkStatsServiceTest { private File mStatsDir; private @Mock INetworkManagementService mNetManager; - private @Mock TrustedTime mTime; private @Mock NetworkStatsSettings mSettings; private @Mock IConnectivityManager mConnManager; private @Mock IBinder mBinder; @@ -167,6 +167,13 @@ public class NetworkStatsServiceTest { private INetworkStatsSession mSession; private INetworkManagementEventObserver mNetworkObserver; + private final Clock mClock = new SimpleClock(ZoneOffset.UTC) { + @Override + public long millis() { + return currentTimeMillis(); + } + }; + @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); @@ -184,7 +191,7 @@ public class NetworkStatsServiceTest { powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG); mService = new NetworkStatsService( - mServiceContext, mNetManager, mAlarmManager, wakeLock, mTime, + mServiceContext, mNetManager, mAlarmManager, wakeLock, mClock, TelephonyManager.getDefault(), mSettings, new NetworkStatsObservers(), mStatsDir, getBaseDir(mStatsDir)); mHandlerThread = new HandlerThread("HandlerThread"); @@ -196,7 +203,6 @@ public class NetworkStatsServiceTest { mElapsedRealtime = 0L; - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsUidDetail(buildEmptyStats()); expectSystemReady(); @@ -221,7 +227,6 @@ public class NetworkStatsServiceTest { mStatsDir = null; mNetManager = null; - mTime = null; mSettings = null; mConnManager = null; @@ -233,7 +238,6 @@ public class NetworkStatsServiceTest { public void testNetworkStatsWifi() throws Exception { // pretend that wifi network comes online; service should ask about full // network state, and poll any existing interfaces before updating. - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -248,7 +252,6 @@ public class NetworkStatsServiceTest { // modify some number on wifi, and trigger poll event incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 1024L, 1L, 2048L, 2L)); @@ -262,7 +265,6 @@ public class NetworkStatsServiceTest { // and bump forward again, with counters going higher. this is // important, since polling should correctly subtract last snapshot. incrementCurrentTime(DAY_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 4096L, 4L, 8192L, 8L)); @@ -280,7 +282,6 @@ public class NetworkStatsServiceTest { // pretend that wifi network comes online; service should ask about full // network state, and poll any existing interfaces before updating. - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -295,7 +296,6 @@ public class NetworkStatsServiceTest { // modify some number on wifi, and trigger poll event incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 1024L, 8L, 2048L, 16L)); @@ -324,13 +324,11 @@ public class NetworkStatsServiceTest { // graceful shutdown system, which should trigger persist of stats, and // clear any values in memory. - expectCurrentTime(); expectDefaultSettings(); mServiceContext.sendBroadcast(new Intent(Intent.ACTION_SHUTDOWN)); assertStatsFilesExist(true); // boot through serviceReady() again - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsUidDetail(buildEmptyStats()); expectSystemReady(); @@ -358,7 +356,6 @@ public class NetworkStatsServiceTest { // pretend that wifi network comes online; service should ask about full // network state, and poll any existing interfaces before updating. - expectCurrentTime(); expectSettings(0L, HOUR_IN_MILLIS, WEEK_IN_MILLIS); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -370,7 +367,6 @@ public class NetworkStatsServiceTest { // modify some number on wifi, and trigger poll event incrementCurrentTime(2 * HOUR_IN_MILLIS); - expectCurrentTime(); expectSettings(0L, HOUR_IN_MILLIS, WEEK_IN_MILLIS); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 512L, 4L, 512L, 4L)); @@ -386,7 +382,6 @@ public class NetworkStatsServiceTest { // now change bucket duration setting and trigger another poll with // exact same values, which should resize existing buckets. - expectCurrentTime(); expectSettings(0L, 30 * MINUTE_IN_MILLIS, WEEK_IN_MILLIS); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(buildEmptyStats()); @@ -403,7 +398,6 @@ public class NetworkStatsServiceTest { @Test public void testUidStatsAcrossNetworks() throws Exception { // pretend first mobile network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildMobile3gState(IMSI_1)); expectNetworkStatsSummary(buildEmptyStats()); @@ -415,7 +409,6 @@ public class NetworkStatsServiceTest { // create some traffic on first network incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 2048L, 16L, 512L, 4L)); @@ -437,7 +430,6 @@ public class NetworkStatsServiceTest { // now switch networks; this also tests that we're okay with interfaces // disappearing, to verify we don't count backwards. incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildMobile3gState(IMSI_2)); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) @@ -454,7 +446,6 @@ public class NetworkStatsServiceTest { // create traffic on second network incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 2176L, 17L, 1536L, 12L)); @@ -483,7 +474,6 @@ public class NetworkStatsServiceTest { @Test public void testUidRemovedIsMoved() throws Exception { // pretend that network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -495,7 +485,6 @@ public class NetworkStatsServiceTest { // create some traffic incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 4128L, 258L, 544L, 34L)); @@ -517,7 +506,6 @@ public class NetworkStatsServiceTest { // now pretend two UIDs are uninstalled, which should migrate stats to // special "removed" bucket. - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 4128L, 258L, 544L, 34L)); @@ -545,7 +533,6 @@ public class NetworkStatsServiceTest { @Test public void testUid3g4gCombinedByTemplate() throws Exception { // pretend that network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildMobile3gState(IMSI_1)); expectNetworkStatsSummary(buildEmptyStats()); @@ -557,7 +544,6 @@ public class NetworkStatsServiceTest { // create some traffic incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) @@ -573,7 +559,6 @@ public class NetworkStatsServiceTest { // now switch over to 4g network incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildMobile4gState(TEST_IFACE2)); expectNetworkStatsSummary(buildEmptyStats()); @@ -588,7 +573,6 @@ public class NetworkStatsServiceTest { // create traffic on second network incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) @@ -607,7 +591,6 @@ public class NetworkStatsServiceTest { @Test public void testSummaryForAllUid() throws Exception { // pretend that network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -619,7 +602,6 @@ public class NetworkStatsServiceTest { // create some traffic for two apps incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) @@ -637,7 +619,6 @@ public class NetworkStatsServiceTest { // now create more traffic in next hour, but only for one app incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) @@ -669,7 +650,6 @@ public class NetworkStatsServiceTest { @Test public void testForegroundBackground() throws Exception { // pretend that network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -681,7 +661,6 @@ public class NetworkStatsServiceTest { // create some initial traffic incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) @@ -697,7 +676,6 @@ public class NetworkStatsServiceTest { // now switch to foreground incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) @@ -730,7 +708,6 @@ public class NetworkStatsServiceTest { @Test public void testMetered() throws Exception { // pretend that network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState(true /* isMetered */)); expectNetworkStatsSummary(buildEmptyStats()); @@ -742,7 +719,6 @@ public class NetworkStatsServiceTest { // create some initial traffic incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); // Note that all traffic from NetworkManagementService is tagged as METERED_NO, ROAMING_NO @@ -772,7 +748,6 @@ public class NetworkStatsServiceTest { @Test public void testRoaming() throws Exception { // pretend that network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildMobile3gState(IMSI_1, true /* isRoaming */)); expectNetworkStatsSummary(buildEmptyStats()); @@ -784,7 +759,6 @@ public class NetworkStatsServiceTest { // Create some traffic incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); // Note that all traffic from NetworkManagementService is tagged as METERED_NO and @@ -813,7 +787,6 @@ public class NetworkStatsServiceTest { @Test public void testTethering() throws Exception { // pretend first mobile network comes online - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildMobile3gState(IMSI_1)); expectNetworkStatsSummary(buildEmptyStats()); @@ -825,7 +798,6 @@ public class NetworkStatsServiceTest { // create some tethering traffic incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); // Traffic seen by kernel counters (includes software tethering). @@ -858,7 +830,6 @@ public class NetworkStatsServiceTest { public void testRegisterUsageCallback() throws Exception { // pretend that wifi network comes online; service should ask about full // network state, and poll any existing interfaces before updating. - expectCurrentTime(); expectDefaultSettings(); expectNetworkState(buildWifiState()); expectNetworkStatsSummary(buildEmptyStats()); @@ -880,7 +851,6 @@ public class NetworkStatsServiceTest { Messenger messenger = new Messenger(latchedHandler); // Force poll - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(buildEmptyStats()); @@ -909,7 +879,6 @@ public class NetworkStatsServiceTest { // modify some number on wifi, and trigger poll event // not enough traffic to call data usage callback incrementCurrentTime(HOUR_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 1024L, 1L, 2048L, 2L)); @@ -925,7 +894,6 @@ public class NetworkStatsServiceTest { // and bump forward again, with counters going higher. this is // important, since it will trigger the data usage callback incrementCurrentTime(DAY_IN_MILLIS); - expectCurrentTime(); expectDefaultSettings(); expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) .addIfaceValues(TEST_IFACE, 4096000L, 4L, 8192000L, 8L)); @@ -1068,7 +1036,6 @@ public class NetworkStatsServiceTest { private void expectSettings(long persistBytes, long bucketDuration, long deleteAge) throws Exception { when(mSettings.getPollInterval()).thenReturn(HOUR_IN_MILLIS); - when(mSettings.getTimeCacheMaxAge()).thenReturn(DAY_IN_MILLIS); when(mSettings.getSampleEnabled()).thenReturn(true); final Config config = new Config(bucketDuration, deleteAge, deleteAge); @@ -1084,14 +1051,6 @@ public class NetworkStatsServiceTest { when(mSettings.getUidTagPersistBytes(anyLong())).thenReturn(MB_IN_BYTES); } - private void expectCurrentTime() throws Exception { - when(mTime.forceRefresh()).thenReturn(false); - when(mTime.hasCache()).thenReturn(true); - when(mTime.currentTimeMillis()).thenReturn(currentTimeMillis()); - when(mTime.getCacheAge()).thenReturn(0L); - when(mTime.getCacheCertainty()).thenReturn(0L); - } - private void expectBandwidthControlCheck() throws Exception { when(mNetManager.isBandwidthControlEnabled()).thenReturn(true); }