diff --git a/services/core/java/com/android/server/timedetector/NetworkTimeUpdateService.java b/services/core/java/com/android/server/timedetector/NetworkTimeUpdateService.java index 080929729fe35..4d1f9d597543f 100644 --- a/services/core/java/com/android/server/timedetector/NetworkTimeUpdateService.java +++ b/services/core/java/com/android/server/timedetector/NetworkTimeUpdateService.java @@ -228,16 +228,14 @@ public class NetworkTimeUpdateService extends Binder { } private void onPollNetworkTime(@NonNull String reason) { - // If we don't have any default network, don't bother. Network network; synchronized (mLock) { network = mDefaultNetwork; } - if (network == null) return; mWakeLock.acquire(); try { - mEngine.refreshIfRequiredAndReschedule(network, reason, mRefreshCallbacks); + mEngine.refreshAndRescheduleIfRequired(network, reason, mRefreshCallbacks); } finally { mWakeLock.release(); } @@ -338,10 +336,10 @@ public class NetworkTimeUpdateService extends Binder { * Attempts to refresh the network time if required, i.e. if there isn't a recent-enough * network time available. It must also schedule the next call. This is a blocking call. * - * @param network the network to use + * @param network the network to use, or null if no network is available * @param reason the reason for the refresh (for logging) */ - void refreshIfRequiredAndReschedule(@NonNull Network network, @NonNull String reason, + void refreshAndRescheduleIfRequired(@Nullable Network network, @NonNull String reason, @NonNull RefreshCallbacks refreshCallbacks); void dump(@NonNull PrintWriter pw); @@ -391,7 +389,7 @@ public class NetworkTimeUpdateService extends Binder { /** * Records the time of the last refresh attempt (successful or otherwise) by this service. * This is used when scheduling the next refresh attempt. In cases where {@link - * #refreshIfRequiredAndReschedule} is called too frequently, this will prevent each call + * #refreshAndRescheduleIfRequired} is called too frequently, this will prevent each call * resulting in a network request. See also {@link #mShortPollingIntervalMillis}. * *

Time servers are a shared resource and so Android should avoid loading them. @@ -443,9 +441,19 @@ public class NetworkTimeUpdateService extends Binder { } @Override - public void refreshIfRequiredAndReschedule( - @NonNull Network network, @NonNull String reason, + public void refreshAndRescheduleIfRequired( + @Nullable Network network, @NonNull String reason, @NonNull RefreshCallbacks refreshCallbacks) { + if (network == null) { + // If we don't have any default network, don't do anything: When a new network + // is available then this method will be called again. + logToDebugAndDumpsys("refreshIfRequiredAndReschedule:" + + " reason=" + reason + + ": No default network available. No refresh attempted and no next" + + " attempt scheduled."); + return; + } + // Attempt to refresh the network time if there is no latest time result, or if the // latest time result is considered too old. NtpTrustedTime.TimeResult initialTimeResult = mNtpTrustedTime.getCachedTimeResult(); diff --git a/services/core/java/com/android/server/timedetector/NetworkTimeUpdateServiceShellCommand.java b/services/core/java/com/android/server/timedetector/NetworkTimeUpdateServiceShellCommand.java index 410b50f8a88b2..afc0bdd30f292 100644 --- a/services/core/java/com/android/server/timedetector/NetworkTimeUpdateServiceShellCommand.java +++ b/services/core/java/com/android/server/timedetector/NetworkTimeUpdateServiceShellCommand.java @@ -157,7 +157,7 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand { SET_SERVER_CONFIG_SERVER_ARG, SET_SERVER_CONFIG_SERVER_ARG, SET_SERVER_CONFIG_TIMEOUT_ARG); pw.printf(" NTP server URIs must be in the form \"ntp://hostname\" or" - + " \"ntp://hostname:port\""); + + " \"ntp://hostname:port\"\n"); pw.printf(" %s\n", SHELL_COMMAND_RESET_SERVER_CONFIG); pw.printf(" Resets/clears the NTP server config set via %s.\n", SHELL_COMMAND_SET_SERVER_CONFIG); diff --git a/services/core/java/com/android/server/timedetector/ServerFlags.java b/services/core/java/com/android/server/timedetector/ServerFlags.java index 678222980ea48..28d34c26e0ddf 100644 --- a/services/core/java/com/android/server/timedetector/ServerFlags.java +++ b/services/core/java/com/android/server/timedetector/ServerFlags.java @@ -304,7 +304,7 @@ public final class ServerFlags { /** * Returns an {@link Instant} from {@link DeviceConfig} from the system_time - * namespace, returns the {@code defaultValue} if the value is missing or invalid. + * namespace, returns {@link Optional#empty()} if there is no explicit value set. */ @NonNull public Optional getOptionalInstant(@DeviceConfigKey String key) { @@ -341,16 +341,17 @@ public final class ServerFlags { } /** - * Returns a boolean value from {@link DeviceConfig} from the system_time - * namespace, or {@code defaultValue} if there is no explicit value set. + * Returns a boolean value from {@link DeviceConfig} from the system_time namespace, or + * {@code defaultValue} if there is no explicit value set. */ public boolean getBoolean(@DeviceConfigKey String key, boolean defaultValue) { return DeviceConfig.getBoolean(NAMESPACE_SYSTEM_TIME, key, defaultValue); } /** - * Returns a positive duration from {@link DeviceConfig} from the system_time - * namespace, or {@code defaultValue} if there is no explicit value set. + * Returns a positive duration from {@link DeviceConfig} from the system_time namespace, + * or {@code defaultValue} if there is no explicit value set or if the value is not a number or + * is negative. */ @Nullable public Duration getDurationFromMillis( diff --git a/services/tests/servicestests/src/com/android/server/timedetector/NetworkTimeUpdateServiceTest.java b/services/tests/servicestests/src/com/android/server/timedetector/NetworkTimeUpdateServiceTest.java index 10014221f235b..2f431bd8a4168 100644 --- a/services/tests/servicestests/src/com/android/server/timedetector/NetworkTimeUpdateServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/timedetector/NetworkTimeUpdateServiceTest.java @@ -22,6 +22,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import android.app.time.UnixEpochTime; @@ -60,7 +61,28 @@ public class NetworkTimeUpdateServiceTest { } @Test - public void engineImpl_refreshIfRequiredAndReschedule_success() { + public void engineImpl_refreshAndRescheduleIfRequired_nullNetwork() { + mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS); + + int normalPollingIntervalMillis = 7777777; + int shortPollingIntervalMillis = 3333; + int tryAgainTimesMax = 5; + NetworkTimeUpdateService.Engine engine = new NetworkTimeUpdateService.EngineImpl( + mFakeElapsedRealtimeClock, + normalPollingIntervalMillis, shortPollingIntervalMillis, tryAgainTimesMax, + mMockNtpTrustedTime); + + RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); + // Trigger the engine's logic. + engine.refreshAndRescheduleIfRequired(null, "Test", mockCallback); + + // Expect nothing to have happened. + verifyNoMoreInteractions(mMockNtpTrustedTime); + verifyNoMoreInteractions(mockCallback); + } + + @Test + public void engineImpl_refreshAndRescheduleIfRequired_success() { mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS); int normalPollingIntervalMillis = 7777777; @@ -83,7 +105,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect the refresh attempt to have been made. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); @@ -98,7 +120,7 @@ public class NetworkTimeUpdateServiceTest { } @Test - public void engineImpl_refreshIfRequiredAndReschedule_failThenFailRepeatedly() { + public void engineImpl_refreshAndRescheduleIfRequired_failThenFailRepeatedly() { mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS); int normalPollingIntervalMillis = 7777777; @@ -120,7 +142,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect a refresh attempt each time: there's no currently cached result. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); @@ -141,7 +163,7 @@ public class NetworkTimeUpdateServiceTest { } @Test - public void engineImpl_refreshIfRequiredAndReschedule_successThenFailRepeatedly() { + public void engineImpl_refreshAndRescheduleIfRequired_successThenFailRepeatedly() { mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS); int normalPollingIntervalMillis = 7777777; @@ -168,7 +190,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect the refresh attempt to have been made: there is no cached network time // initially. @@ -182,7 +204,7 @@ public class NetworkTimeUpdateServiceTest { } // Increment the current time by enough so that an attempt to refresh the time should be - // made every time refreshIfRequiredAndReschedule() is called. + // made every time refreshAndRescheduleIfRequired() is called. mFakeElapsedRealtimeClock.incrementMillis(normalPollingIntervalMillis); // Test multiple follow-up calls. @@ -195,7 +217,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect a refresh attempt each time as the cached network time is too old. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); @@ -221,7 +243,7 @@ public class NetworkTimeUpdateServiceTest { } @Test - public void engineImpl_refreshIfRequiredAndReschedule_successThenFail_tryAgainTimesZero() { + public void engineImpl_refreshAndRescheduleIfRequired_successThenFail_tryAgainTimesZero() { mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS); int normalPollingIntervalMillis = 7777777; @@ -248,7 +270,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect the refresh attempt to have been made: there is no cached network time // initially. @@ -262,7 +284,7 @@ public class NetworkTimeUpdateServiceTest { } // Increment the current time by enough so that an attempt to refresh the time should be - // made every time refreshIfRequiredAndReschedule() is called. + // made every time refreshAndRescheduleIfRequired() is called. mFakeElapsedRealtimeClock.incrementMillis(normalPollingIntervalMillis); // Test multiple follow-up calls. @@ -275,7 +297,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect a refresh attempt each time as the cached network time is too old. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); @@ -297,7 +319,7 @@ public class NetworkTimeUpdateServiceTest { } @Test - public void engineImpl_refreshIfRequiredAndReschedule_successThenFail_tryAgainTimesNegative() { + public void engineImpl_refreshAndRescheduleIfRequired_successThenFail_tryAgainTimesNegative() { mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS); int normalPollingIntervalMillis = 7777777; @@ -324,7 +346,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect the refresh attempt to have been made: there is no cached network time // initially. @@ -338,7 +360,7 @@ public class NetworkTimeUpdateServiceTest { } // Increment the current time by enough so that an attempt to refresh the time should be - // made every time refreshIfRequiredAndReschedule() is called. + // made every time refreshAndRescheduleIfRequired() is called. mFakeElapsedRealtimeClock.incrementMillis(normalPollingIntervalMillis); // Test multiple follow-up calls. @@ -351,7 +373,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect a refresh attempt each time as the cached network time is too old. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); @@ -373,7 +395,7 @@ public class NetworkTimeUpdateServiceTest { } @Test - public void engineImpl_refreshIfRequiredAndReschedule_successFailSuccess() { + public void engineImpl_refreshAndRescheduleIfRequired_successFailSuccess() { mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS); int normalPollingIntervalMillis = 7777777; @@ -398,7 +420,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect the refresh attempt to have been made: there is no cached network time // initially. @@ -413,7 +435,7 @@ public class NetworkTimeUpdateServiceTest { } // Increment the current time by enough so that the cached time result is too old and an - // attempt to refresh the time should be made every time refreshIfRequiredAndReschedule() is + // attempt to refresh the time should be made every time refreshAndRescheduleIfRequired() is // called. mFakeElapsedRealtimeClock.incrementMillis(normalPollingIntervalMillis); @@ -426,7 +448,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect the refresh attempt to have been made: the timeResult is too old. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); @@ -458,7 +480,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect the refresh attempt to have been made: the timeResult is too old. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); @@ -473,12 +495,12 @@ public class NetworkTimeUpdateServiceTest { } /** - * Confirms that if a refreshIfRequiredAndReschedule() call is made, e.g. for reasons besides + * Confirms that if a refreshAndRescheduleIfRequired() call is made, e.g. for reasons besides * scheduled alerts, and the latest time is not too old, then an NTP refresh won't be attempted. * A suggestion will still be made. */ @Test - public void engineImpl_refreshIfRequiredAndReschedule_noRefreshIfLatestIsFresh() { + public void engineImpl_refreshAndRescheduleIfRequired_noRefreshIfLatestIsFresh() { mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS); int normalPollingIntervalMillis = 7777777; @@ -498,7 +520,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect no refresh attempt to have been made. verify(mMockNtpTrustedTime, never()).forceRefresh(any()); @@ -516,11 +538,11 @@ public class NetworkTimeUpdateServiceTest { } /** - * Confirms that if a refreshIfRequiredAndReschedule() call is made, e.g. for reasons besides + * Confirms that if a refreshAndRescheduleIfRequired() call is made, e.g. for reasons besides * scheduled alerts, and the latest time is too old, then an NTP refresh will be attempted. */ @Test - public void engineImpl_refreshIfRequiredAndReschedule_failureHandlingAfterLatestIsTooOld() { + public void engineImpl_refreshAndRescheduleIfRequired_failureHandlingAfterLatestIsTooOld() { mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS); int normalPollingIntervalMillis = 7777777; @@ -541,7 +563,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect a refresh attempt to have been made. verify(mMockNtpTrustedTime, times(1)).forceRefresh(mDummyNetwork); @@ -556,11 +578,11 @@ public class NetworkTimeUpdateServiceTest { } /** - * Confirms that if a refreshIfRequiredAndReschedule() call is made and there was a recently + * Confirms that if a refreshAndRescheduleIfRequired() call is made and there was a recently * failed refresh, then another won't be scheduled too soon. */ @Test - public void engineImpl_refreshIfRequiredAndReschedule_minimumRefreshTimeEnforced() { + public void engineImpl_refreshAndRescheduleIfRequired_minimumRefreshTimeEnforced() { mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS); int normalPollingIntervalMillis = 7777777; @@ -574,7 +596,7 @@ public class NetworkTimeUpdateServiceTest { NtpTrustedTime.TimeResult timeResult = createNtpTimeResult( mFakeElapsedRealtimeClock.getElapsedRealtimeMillis()); - // Simulate an initial call to refreshIfRequiredAndReschedule() prime the "last refresh + // Simulate an initial call to refreshAndRescheduleIfRequired() prime the "last refresh // attempt" time. A cached time value is available, but it's too old but the refresh // attempt will fail. long lastRefreshAttemptElapsedMillis; @@ -586,7 +608,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect a refresh attempt to have been made. verify(mMockNtpTrustedTime, times(1)).forceRefresh(mDummyNetwork); @@ -606,7 +628,7 @@ public class NetworkTimeUpdateServiceTest { reset(mMockNtpTrustedTime); } - // Simulate a second call to refreshIfRequiredAndReschedule() very soon after the first, as + // Simulate a second call to refreshAndRescheduleIfRequired() very soon after the first, as // might happen if there were a network state change. // The cached time value is available, but it's still too old. Because the last call was so // recent, no refresh should take place and the next scheduled refresh time should be @@ -619,7 +641,7 @@ public class NetworkTimeUpdateServiceTest { RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); // Trigger the engine's logic. - engine.refreshIfRequiredAndReschedule(mDummyNetwork, "Test", mockCallback); + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); // Expect no refresh attempt to have been made: time elapsed isn't enough. verify(mMockNtpTrustedTime, never()).forceRefresh(any());