When a valid network time is available, the time is always suggested to the {@link + *
When a valid network time is available, the network time is always suggested to the {@link * com.android.server.timedetector.TimeDetectorService} where it may be used to set the device * system clock, depending on user settings and what other signals are available. */ @@ -286,7 +286,7 @@ public class NetworkTimeUpdateService extends Binder { } /** - * Checks if the user prefers to automatically set the time. + * Checks if the user prefers to automatically set the device's system clock time. */ private boolean isAutomaticTimeEnabled() { ContentResolver resolver = mContext.getContentResolver(); @@ -313,7 +313,7 @@ public class NetworkTimeUpdateService extends Binder { } /** - * The interface the service uses to interact with the time refresh logic. + * The interface the service uses to interact with the network time refresh logic. * Extracted for testing. */ @VisibleForTesting @@ -387,10 +387,10 @@ public class NetworkTimeUpdateService extends Binder { private final NtpTrustedTime mNtpTrustedTime; /** - * 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 - * #refreshAndRescheduleIfRequired} is called too frequently, this will prevent each call - * resulting in a network request. See also {@link #mShortPollingIntervalMillis}. + * Records the elapsed realtime of the last refresh attempt (successful or otherwise) by + * this service. This is used when scheduling the next refresh attempt. In cases where + * {@link #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. * Generally, a refresh attempt will succeed and the service won't need to make further @@ -454,8 +454,11 @@ public class NetworkTimeUpdateService extends Binder { return; } - // Attempt to refresh the network time if there is no latest time result, or if the - // latest time result is considered too old. + // Step 1: Work out if the latest time result, if any, needs to be refreshed and handle + // the refresh. + + // A refresh should be attempted if there is no latest time result, or if the latest + // time result is considered too old. NtpTrustedTime.TimeResult initialTimeResult = mNtpTrustedTime.getCachedTimeResult(); boolean shouldAttemptRefresh; synchronized (this) { @@ -472,22 +475,42 @@ public class NetworkTimeUpdateService extends Binder { boolean refreshSuccessful = false; if (shouldAttemptRefresh) { // This is a blocking call. Deliberately invoked without holding the "this" monitor - // to avoid blocking logic that wants to use the "this" monitor. + // to avoid blocking other logic that wants to use the "this" monitor, e.g. dump(). refreshSuccessful = tryRefresh(network); } synchronized (this) { - // Manage mTryAgainCounter. + // This section of code deliberately doesn't assume it is the only component using + // the NtpTrustedTime singleton to obtain NTP times: another component in the same + // process could be gathering NTP signals (which then won't have been suggested to + // the time detector). + // TODO(b/222295093): Make this class the sole user of the NtpTrustedTime singleton + // and simplify / reduce duplicate suggestions and other logic. + NtpTrustedTime.TimeResult latestTimeResult = mNtpTrustedTime.getCachedTimeResult(); + + // currentElapsedRealtimeMillis is used to evaluate ages and refresh scheduling + // below. Capturing this after obtaining the cached time result ensures that latest + // time result ages will be >= 0. + long currentElapsedRealtimeMillis = mElapsedRealtimeMillisSupplier.get(); + + long latestTimeResultAgeMillis = calculateTimeResultAgeMillis( + latestTimeResult, currentElapsedRealtimeMillis); + + // Step 2: Set mTryAgainCounter. + // + == 0: The last attempt was successful OR the latest time result is acceptable + // OR the mTryAgainCounter exceeded mTryAgainTimesMax and has been reset + // to 0. In all these cases the normal refresh interval should be used. + // + > 0: The last refresh attempt was unsuccessful. Some number of retries are + // allowed using the short interval depending on mTryAgainTimesMax. if (shouldAttemptRefresh) { if (refreshSuccessful) { - // Reset failure tracking. mTryAgainCounter = 0; } else { if (mTryAgainTimesMax < 0) { // When mTryAgainTimesMax is negative there's no enforced maximum and // short intervals should be used until a successful refresh. Setting // mTryAgainCounter to 1 is sufficient for the interval calculations - // below. There's no need to increment. + // below, i.e. there's no need to increment. mTryAgainCounter = 1; } else { mTryAgainCounter++; @@ -497,58 +520,97 @@ public class NetworkTimeUpdateService extends Binder { } } } - - // currentElapsedRealtimeMillis is used to evaluate ages and refresh scheduling - // below. Capturing this after a possible successful refresh ensures that latest - // time result ages will be >= 0. - long currentElapsedRealtimeMillis = mElapsedRealtimeMillisSupplier.get(); - - // This section of code deliberately doesn't assume it is the only component using - // mNtpTrustedTime to obtain NTP times: another component in the same process could - // be gathering NTP signals (which then won't have been suggested to the time - // detector). - // TODO(b/222295093): Make this class the sole owner of mNtpTrustedTime and - // simplify / reduce duplicate suggestions. - NtpTrustedTime.TimeResult latestTimeResult = mNtpTrustedTime.getCachedTimeResult(); - long latestTimeResultAgeMillis = calculateTimeResultAgeMillis( - latestTimeResult, currentElapsedRealtimeMillis); - - // Suggest the latest time result to the time detector if it is fresh regardless of - // whether refresh happened above. if (latestTimeResultAgeMillis < mNormalPollingIntervalMillis) { - // We assume the time detector service will detect duplicate suggestions and not - // do more work than it has to, so no need to avoid making duplicate - // suggestions. + // The latest time result may indicate a successful refresh has been achieved by + // another user of the NtpTrustedTime singleton. This could be an "else if", but + // this is deliberately done defensively in all cases to maintain the invariant + // that mTryAgainCounter will be 0 if the latest time result is currently ok. + mTryAgainCounter = 0; + } + + // Step 3: Suggest the latest time result to the time detector if it is fresh + // regardless of whether a refresh happened / succeeded above. The time detector + // service can detect duplicate suggestions and not do more work than it has to, so + // there is no need to avoid making duplicate suggestions. + if (latestTimeResultAgeMillis < mNormalPollingIntervalMillis) { makeNetworkTimeSuggestion(latestTimeResult, reason, refreshCallbacks); } - // (Re)schedule the next refresh based on the latest state. - // Determine which refresh delay to use by using the current value of - // mTryAgainCounter. The refresh delay is applied to a different point in time - // depending on whether the latest available time result (if any) is still - // considered fresh to ensure the delay acts correctly. - long refreshDelayMillis = mTryAgainCounter > 0 + // Step 4: (Re)schedule the next refresh attempt based on the latest state. + + // Determine which refresh attempt delay to use by using the current value of + // mTryAgainCounter. + long refreshAttemptDelayMillis = mTryAgainCounter > 0 ? mShortPollingIntervalMillis : mNormalPollingIntervalMillis; + + // The refresh attempt delay is applied to a different point in time depending on + // whether a refresh attempt is overdue to ensure the refresh attempt scheduling + // acts correctly / safely, i.e. won't schedule actions for immediate execution or + // in the past. long nextRefreshElapsedRealtimeMillis; - if (latestTimeResultAgeMillis < mNormalPollingIntervalMillis) { - // The latest time result is fresh, use it to determine when next to refresh. + if (latestTimeResultAgeMillis < refreshAttemptDelayMillis) { + // The latestTimeResultAgeMillis and refreshAttemptDelayMillis indicate a + // refresh attempt is not yet due. This branch uses the elapsed realtime of the + // latest time result to calculate when the latest time result will become too + // old and the next refresh attempt will be due. + // + // Possibilities: + // + A refresh was attempted and successful, mTryAgainCounter will be set + // to 0, refreshAttemptDelayMillis == mNormalPollingIntervalMillis, and this + // branch will execute. + // + No refresh was attempted, but something else refreshed the latest time + // result held by the NtpTrustedTime. + // + // If a refresh was attempted but was unsuccessful, latestTimeResultAgeMillis >= + // mNormalPollingIntervalMillis (because otherwise it wouldn't be attempted), + // this branch won't be executed, and the one below will be instead. nextRefreshElapsedRealtimeMillis = - latestTimeResult.getElapsedRealtimeMillis() + refreshDelayMillis; + latestTimeResult.getElapsedRealtimeMillis() + refreshAttemptDelayMillis; } else if (mLastRefreshAttemptElapsedRealtimeMillis != null) { - // The latest time result is missing or old and still needs to be refreshed. - // mLastRefreshAttemptElapsedRealtimeMillis, which should always be set by this - // point because there's no fresh time result, should be very close to - // currentElapsedRealtimeMillis unless the refresh was not allowed. + // This branch is executed when the latest time result is missing, or it's older + // than refreshAttemptDelayMillis. There may already have been attempts to + // refresh the network time that have failed, so the important point for this + // branch is not how old the latest time result is, but when the last refresh + // attempt took place: + // + If a refresh was just attempted (and failed), then + // mLastRefreshAttemptElapsedRealtimeMillis will be close to + // currentElapsedRealtimeMillis. + // + If a refresh was not just attempted, for a refresh not to have been + // attempted EITHER: + // + The latest time result must be < mNormalPollingIntervalMillis ago + // (would be handled by the branch above) + // + A refresh wasn't allowed because {time since last refresh attempt} + // < mShortPollingIntervalMillis, so + // (mLastRefreshAttemptElapsedRealtimeMillis + refreshAttemptDelayMillis) + // would have to be in the future regardless of the + // refreshAttemptDelayMillis value. This ignores the execution time + // between the "current time" used to work out whether a refresh needed to + // happen, and "current time" used to compute the last time result age, + // but a single short interval shouldn't matter. nextRefreshElapsedRealtimeMillis = - mLastRefreshAttemptElapsedRealtimeMillis + refreshDelayMillis; + mLastRefreshAttemptElapsedRealtimeMillis + refreshAttemptDelayMillis; } else { - // This should not happen: mLastRefreshAttemptElapsedRealtimeMillis should - // always be non-null by this point. - logToDebugAndDumpsys( - "mLastRefreshAttemptElapsedRealtimeMillis unexpectedly missing." - + " Scheduling using currentElapsedRealtimeMillis"); + // This branch should never execute: mLastRefreshAttemptElapsedRealtimeMillis + // should always be non-null because a refresh should always be attempted at + // least once above. Regardelss, the calculation below should result in safe + // scheduling behavior. + String logMsg = "mLastRefreshAttemptElapsedRealtimeMillis unexpectedly missing." + + " Scheduling using currentElapsedRealtimeMillis"; + Log.w(TAG, logMsg); + logToDebugAndDumpsys(logMsg); nextRefreshElapsedRealtimeMillis = - currentElapsedRealtimeMillis + refreshDelayMillis; + currentElapsedRealtimeMillis + refreshAttemptDelayMillis; + } + + // Defensive coding to guard against bad scheduling / logic errors above: Try to + // ensure that alarms aren't scheduled in the past. + if (nextRefreshElapsedRealtimeMillis <= currentElapsedRealtimeMillis) { + String logMsg = "nextRefreshElapsedRealtimeMillis is a time in the past." + + " Scheduling using currentElapsedRealtimeMillis instead"; + Log.w(TAG, logMsg); + logToDebugAndDumpsys(logMsg); + nextRefreshElapsedRealtimeMillis = + currentElapsedRealtimeMillis + refreshAttemptDelayMillis; } refreshCallbacks.scheduleNextRefresh(nextRefreshElapsedRealtimeMillis); @@ -562,7 +624,7 @@ public class NetworkTimeUpdateService extends Binder { + formatElapsedRealtimeMillis(currentElapsedRealtimeMillis) + ", latestTimeResult=" + latestTimeResult + ", mTryAgainCounter=" + mTryAgainCounter - + ", refreshDelayMillis=" + refreshDelayMillis + + ", refreshAttemptDelayMillis=" + refreshAttemptDelayMillis + ", nextRefreshElapsedRealtimeMillis=" + formatElapsedRealtimeMillis(nextRefreshElapsedRealtimeMillis)); } @@ -592,6 +654,12 @@ public class NetworkTimeUpdateService extends Binder { return currentElapsedRealtimeMillis >= nextRefreshAllowedElapsedRealtimeMillis; } + /** + * Attempts a network time refresh. Updates {@link + * #mLastRefreshAttemptElapsedRealtimeMillis} regardless of the outcome and returns whether + * the attempt was successful. The latest successful refresh result can be found in {@link + * NtpTrustedTime#getCachedTimeResult()}. + */ private boolean tryRefresh(@NonNull Network network) { long currentElapsedRealtimeMillis = mElapsedRealtimeMillisSupplier.get(); synchronized (this) { @@ -600,15 +668,18 @@ public class NetworkTimeUpdateService extends Binder { return mNtpTrustedTime.forceRefresh(network); } - /** Suggests the time to the time detector. It may choose use it to set the system clock. */ - private void makeNetworkTimeSuggestion(@NonNull TimeResult ntpResult, + /** + * Suggests the network time to the time detector. It may choose use it to set the system + * clock. + */ + private void makeNetworkTimeSuggestion(@NonNull TimeResult timeResult, @NonNull String debugInfo, @NonNull RefreshCallbacks refreshCallbacks) { UnixEpochTime timeSignal = new UnixEpochTime( - ntpResult.getElapsedRealtimeMillis(), ntpResult.getTimeMillis()); + timeResult.getElapsedRealtimeMillis(), timeResult.getTimeMillis()); NetworkTimeSuggestion timeSuggestion = - new NetworkTimeSuggestion(timeSignal, ntpResult.getUncertaintyMillis()); + new NetworkTimeSuggestion(timeSignal, timeResult.getUncertaintyMillis()); timeSuggestion.addDebugInfo(debugInfo); - timeSuggestion.addDebugInfo(ntpResult.toString()); + timeSuggestion.addDebugInfo(timeResult.toString()); refreshCallbacks.submitSuggestion(timeSuggestion); } 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 2f431bd8a4168..d91ee92e005ec 100644 --- a/services/tests/servicestests/src/com/android/server/timedetector/NetworkTimeUpdateServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/timedetector/NetworkTimeUpdateServiceTest.java @@ -111,12 +111,11 @@ public class NetworkTimeUpdateServiceTest { verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); // Check everything happened that was supposed to. + NetworkTimeSuggestion expectedSuggestion = createExpectedSuggestion(timeResult); + verify(mockCallback).submitSuggestion(expectedSuggestion); long expectedDelayMillis = normalPollingIntervalMillis; verify(mockCallback).scheduleNextRefresh( timeResult.getElapsedRealtimeMillis() + expectedDelayMillis); - - NetworkTimeSuggestion expectedSuggestion = createExpectedSuggestion(timeResult); - verify(mockCallback).submitSuggestion(expectedSuggestion); } @Test @@ -148,6 +147,7 @@ public class NetworkTimeUpdateServiceTest { verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); // Check everything happened that was supposed to. + verify(mockCallback, never()).submitSuggestion(any()); long expectedDelayMillis; if (i < tryAgainTimesMax) { expectedDelayMillis = shortPollingIntervalMillis; @@ -156,7 +156,6 @@ public class NetworkTimeUpdateServiceTest { } verify(mockCallback).scheduleNextRefresh( mFakeElapsedRealtimeClock.getElapsedRealtimeMillis() + expectedDelayMillis); - verify(mockCallback, never()).submitSuggestion(any()); reset(mMockNtpTrustedTime); } @@ -196,10 +195,10 @@ public class NetworkTimeUpdateServiceTest { // initially. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); + verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); long expectedDelayMillis = normalPollingIntervalMillis; verify(mockCallback).scheduleNextRefresh( timeResult.getElapsedRealtimeMillis() + expectedDelayMillis); - verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); reset(mMockNtpTrustedTime); } @@ -222,6 +221,9 @@ public class NetworkTimeUpdateServiceTest { // Expect a refresh attempt each time as the cached network time is too old. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); + // No valid time, no suggestion. + verify(mockCallback, never()).submitSuggestion(any()); + // Check the scheduling. long expectedDelayMillis; if (i < tryAgainTimesMax) { @@ -232,9 +234,6 @@ public class NetworkTimeUpdateServiceTest { verify(mockCallback).scheduleNextRefresh( mFakeElapsedRealtimeClock.getElapsedRealtimeMillis() + expectedDelayMillis); - // No valid time, no suggestion. - verify(mockCallback, never()).submitSuggestion(any()); - reset(mMockNtpTrustedTime); // Simulate the passage of time for realism. @@ -276,10 +275,10 @@ public class NetworkTimeUpdateServiceTest { // initially. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); + verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); long expectedDelayMillis = normalPollingIntervalMillis; verify(mockCallback).scheduleNextRefresh( timeResult.getElapsedRealtimeMillis() + expectedDelayMillis); - verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); reset(mMockNtpTrustedTime); } @@ -302,15 +301,14 @@ public class NetworkTimeUpdateServiceTest { // Expect a refresh attempt each time as the cached network time is too old. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); - // Check the scheduling. tryAgainTimesMax == 0, so the algorithm should start with + // No valid time, no suggestion. + verify(mockCallback, never()).submitSuggestion(any()); + // Check the scheduling. tryAgainTimesMax == 0, so the algorithm should start with long expectedDelayMillis = normalPollingIntervalMillis; verify(mockCallback).scheduleNextRefresh( mFakeElapsedRealtimeClock.getElapsedRealtimeMillis() + expectedDelayMillis); - // No valid time, no suggestion. - verify(mockCallback, never()).submitSuggestion(any()); - reset(mMockNtpTrustedTime); // Simulate the passage of time for realism. @@ -352,10 +350,10 @@ public class NetworkTimeUpdateServiceTest { // initially. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); + verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); long expectedDelayMillis = normalPollingIntervalMillis; verify(mockCallback).scheduleNextRefresh( timeResult.getElapsedRealtimeMillis() + expectedDelayMillis); - verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); reset(mMockNtpTrustedTime); } @@ -378,15 +376,15 @@ public class NetworkTimeUpdateServiceTest { // Expect a refresh attempt each time as the cached network time is too old. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); + // No valid time, no suggestion. + verify(mockCallback, never()).submitSuggestion(any()); + // Check the scheduling. tryAgainTimesMax == -1, so it should always be // shortPollingIntervalMillis. long expectedDelayMillis = shortPollingIntervalMillis; verify(mockCallback).scheduleNextRefresh( mFakeElapsedRealtimeClock.getElapsedRealtimeMillis() + expectedDelayMillis); - // No valid time, no suggestion. - verify(mockCallback, never()).submitSuggestion(any()); - reset(mMockNtpTrustedTime); // Simulate the passage of time for realism. @@ -426,11 +424,11 @@ public class NetworkTimeUpdateServiceTest { // initially. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); + NetworkTimeSuggestion expectedSuggestion = createExpectedSuggestion(timeResult1); + verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); long expectedDelayMillis = normalPollingIntervalMillis; verify(mockCallback).scheduleNextRefresh( timeResult1.getElapsedRealtimeMillis() + expectedDelayMillis); - NetworkTimeSuggestion expectedSuggestion = createExpectedSuggestion(timeResult1); - verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); reset(mMockNtpTrustedTime); } @@ -453,12 +451,13 @@ public class NetworkTimeUpdateServiceTest { // Expect the refresh attempt to have been made: the timeResult is too old. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); + // No valid time, no suggestion. + verify(mockCallback, never()).submitSuggestion(any()); + long expectedDelayMillis = shortPollingIntervalMillis; verify(mockCallback).scheduleNextRefresh( mFakeElapsedRealtimeClock.getElapsedRealtimeMillis() + expectedDelayMillis); - // No valid time, no suggestion. - verify(mockCallback, never()).submitSuggestion(any()); reset(mMockNtpTrustedTime); } @@ -485,11 +484,11 @@ public class NetworkTimeUpdateServiceTest { // Expect the refresh attempt to have been made: the timeResult is too old. verify(mMockNtpTrustedTime).forceRefresh(mDummyNetwork); + NetworkTimeSuggestion expectedSuggestion = createExpectedSuggestion(timeResult2); + verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); long expectedDelayMillis = normalPollingIntervalMillis; verify(mockCallback).scheduleNextRefresh( timeResult2.getElapsedRealtimeMillis() + expectedDelayMillis); - NetworkTimeSuggestion expectedSuggestion = createExpectedSuggestion(timeResult2); - verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); reset(mMockNtpTrustedTime); } } @@ -525,16 +524,16 @@ public class NetworkTimeUpdateServiceTest { // Expect no refresh attempt to have been made. verify(mMockNtpTrustedTime, never()).forceRefresh(any()); + // Suggestions must be made every time if the cached time value is not too old in case it + // was refreshed by a different component. + NetworkTimeSuggestion expectedSuggestion = createExpectedSuggestion(timeResult); + verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); + // The next wake-up should be rescheduled for when the cached time value will become too // old. long expectedDelayMillis = normalPollingIntervalMillis; verify(mockCallback).scheduleNextRefresh( timeResult.getElapsedRealtimeMillis() + expectedDelayMillis); - - // Suggestions must be made every time if the cached time value is not too old in case it - // was refreshed by a different component. - NetworkTimeSuggestion expectedSuggestion = createExpectedSuggestion(timeResult); - verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); } /** @@ -568,13 +567,13 @@ public class NetworkTimeUpdateServiceTest { // Expect a refresh attempt to have been made. verify(mMockNtpTrustedTime, times(1)).forceRefresh(mDummyNetwork); + // Suggestions should not be made if the cached time value is too old. + verify(mockCallback, never()).submitSuggestion(any()); + // The next wake-up should be rescheduled using the short polling interval. long expectedDelayMillis = shortPollingIntervalMillis; verify(mockCallback).scheduleNextRefresh( mFakeElapsedRealtimeClock.getElapsedRealtimeMillis() + expectedDelayMillis); - - // Suggestions should not be made if the cached time value is too old. - verify(mockCallback, never()).submitSuggestion(any()); } /** @@ -614,6 +613,9 @@ public class NetworkTimeUpdateServiceTest { verify(mMockNtpTrustedTime, times(1)).forceRefresh(mDummyNetwork); lastRefreshAttemptElapsedMillis = mFakeElapsedRealtimeClock.getElapsedRealtimeMillis(); + // Suggestions should not be made if the cached time value is too old. + verify(mockCallback, never()).submitSuggestion(any()); + // The next wake-up should be rescheduled using the normalPollingIntervalMillis. // Because the time signal age > normalPollingIntervalMillis, the last refresh attempt // time will be used. @@ -622,9 +624,6 @@ public class NetworkTimeUpdateServiceTest { lastRefreshAttemptElapsedMillis + expectedDelayMillis; verify(mockCallback).scheduleNextRefresh(expectedNextRefreshElapsedMillis); - // Suggestions should not be made if the cached time value is too old. - verify(mockCallback, never()).submitSuggestion(any()); - reset(mMockNtpTrustedTime); } @@ -646,6 +645,9 @@ public class NetworkTimeUpdateServiceTest { // Expect no refresh attempt to have been made: time elapsed isn't enough. verify(mMockNtpTrustedTime, never()).forceRefresh(any()); + // Suggestions should not be made if the cached time value is too old. + verify(mockCallback, never()).submitSuggestion(any()); + // The next wake-up should be rescheduled using the normal polling interval and the last // refresh attempt time. long expectedDelayMillis = normalPollingIntervalMillis; @@ -653,11 +655,110 @@ public class NetworkTimeUpdateServiceTest { lastRefreshAttemptElapsedMillis + expectedDelayMillis; verify(mockCallback).scheduleNextRefresh(expectedNextRefreshElapsedMillis); + reset(mMockNtpTrustedTime); + } + } + + /** + * 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_refreshAndRescheduleIfRequired_minimumRefreshTimeEnforced_b269425914() { + mFakeElapsedRealtimeClock.setElapsedRealtimeMillis(ARBITRARY_ELAPSED_REALTIME_MILLIS); + + int normalPollingIntervalMillis = 7777777; + int shortPollingIntervalMillis = 3333; + int tryAgainTimesMax = 3; + NetworkTimeUpdateService.Engine engine = new NetworkTimeUpdateService.EngineImpl( + mFakeElapsedRealtimeClock, + normalPollingIntervalMillis, shortPollingIntervalMillis, tryAgainTimesMax, + mMockNtpTrustedTime); + + NtpTrustedTime.TimeResult timeResult1; + + // Start out with a successful refresh. + long lastRefreshAttemptElapsedMillis; + { + mFakeElapsedRealtimeClock.incrementMillis(normalPollingIntervalMillis); + timeResult1 = createNtpTimeResult(mFakeElapsedRealtimeClock.getElapsedRealtimeMillis()); + when(mMockNtpTrustedTime.getCachedTimeResult()).thenReturn(null, timeResult1); + when(mMockNtpTrustedTime.forceRefresh(mDummyNetwork)).thenReturn(true); + + RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); + // Trigger the engine's logic. + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); + + // Expect a refresh attempt to have been made. + verify(mMockNtpTrustedTime, times(1)).forceRefresh(mDummyNetwork); + lastRefreshAttemptElapsedMillis = mFakeElapsedRealtimeClock.getElapsedRealtimeMillis(); + + NetworkTimeSuggestion expectedSuggestion = createExpectedSuggestion(timeResult1); + verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); + + // The next wake-up should be rescheduled using the normalPollingIntervalMillis. + // Because the time signal age > normalPollingIntervalMillis, the last refresh attempt + // time will be used. + long expectedDelayMillis = normalPollingIntervalMillis; + long expectedNextRefreshElapsedMillis = + lastRefreshAttemptElapsedMillis + expectedDelayMillis; + verify(mockCallback).scheduleNextRefresh(expectedNextRefreshElapsedMillis); + + reset(mMockNtpTrustedTime); + } + + // Now fail: This should result in the next refresh using a short polling interval. + { + mFakeElapsedRealtimeClock.incrementMillis(normalPollingIntervalMillis); + when(mMockNtpTrustedTime.getCachedTimeResult()).thenReturn(timeResult1); + when(mMockNtpTrustedTime.forceRefresh(mDummyNetwork)).thenReturn(false); + + RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); + // Trigger the engine's logic. + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); + + verify(mMockNtpTrustedTime, times(1)).forceRefresh(mDummyNetwork); + lastRefreshAttemptElapsedMillis = mFakeElapsedRealtimeClock.getElapsedRealtimeMillis(); + // Suggestions should not be made if the cached time value is too old. verify(mockCallback, never()).submitSuggestion(any()); + // The next wake-up should be rescheduled using the last refresh attempt time (because + // the latest time result is too old) and the short polling interval. + long expectedDelayMillis = shortPollingIntervalMillis; + long expectedNextRefreshElapsedMillis = + lastRefreshAttemptElapsedMillis + expectedDelayMillis; + verify(mockCallback).scheduleNextRefresh(expectedNextRefreshElapsedMillis); + reset(mMockNtpTrustedTime); } + + // Simulate some other thread successfully refreshing the value held by NtpTrustedTime and + // confirm it is handled correctly. + { + mFakeElapsedRealtimeClock.incrementMillis(shortPollingIntervalMillis / 2); + NtpTrustedTime.TimeResult timeResult2 = createNtpTimeResult( + mFakeElapsedRealtimeClock.getElapsedRealtimeMillis()); + when(mMockNtpTrustedTime.getCachedTimeResult()).thenReturn(timeResult2); + + mFakeElapsedRealtimeClock.incrementMillis(shortPollingIntervalMillis / 2); + + RefreshCallbacks mockCallback = mock(RefreshCallbacks.class); + // Trigger the engine's logic. + engine.refreshAndRescheduleIfRequired(mDummyNetwork, "Test", mockCallback); + + verify(mMockNtpTrustedTime, never()).forceRefresh(any()); + + NetworkTimeSuggestion expectedSuggestion = createExpectedSuggestion(timeResult2); + verify(mockCallback, times(1)).submitSuggestion(expectedSuggestion); + + // The next wake-up should be rescheduled using the normal polling interval and the + // latest time result. + long expectedDelayMillis = normalPollingIntervalMillis; + long expectedNextRefreshElapsedMillis = + timeResult2.getElapsedRealtimeMillis() + expectedDelayMillis; + verify(mockCallback).scheduleNextRefresh(expectedNextRefreshElapsedMillis); + } } private static NetworkTimeSuggestion createExpectedSuggestion(