Prepare to make time detection more configurable

Prepare to make time detection more configurable. This change introduces
an idea of origin ordering ("priority") so that different partners and
form factors can modify the previously hardcoded ordering of "telephony
(NITZ) first then network (NTP)".

This commit also modifies the name of the method used to trigger a time
detection cycle after a config change. It's possible partners may want
to make the priority originate from a setting rather than static config
(i.e. so the partner can pick between origins or origin orders). This
would involve more work than currently planned, but it is easy to make
the method name less specific.

Bug: 172230856
Test: treehugger
Test: atest TimeDetectorStrategyImplTest
Change-Id: Id1652be174b7503618e0b802dc7da4d00df4f248

services/core/java/com/android/server/timedetector/TimeDetectorService.java

@@ -132,7 +132,7 @@ public final class TimeDetectorService extends ITimeDetectorService.Stub {
     /** Internal method for handling the auto time setting being changed. */
     @VisibleForTesting
     public void handleAutoTimeDetectionChanged() {
-        mHandler.post(mTimeDetectorStrategy::handleAutoTimeDetectionChanged);
+        mHandler.post(mTimeDetectorStrategy::handleAutoTimeConfigChanged);
     }
 
     @Override

services/core/java/com/android/server/timedetector/TimeDetectorStrategy.java

@@ -16,6 +16,7 @@
 
 package com.android.server.timedetector;
 
+import android.annotation.IntDef;
 import android.annotation.NonNull;
 import android.app.timedetector.ManualTimeSuggestion;
 import android.app.timedetector.NetworkTimeSuggestion;
@@ -25,6 +26,9 @@ import android.util.IndentingPrintWriter;
 
 import com.android.server.timezonedetector.Dumpable;
 
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
 /**
  * The interface for the class that implements the time detection algorithm used by the
  * {@link TimeDetectorService}.
@@ -37,22 +41,41 @@ import com.android.server.timezonedetector.Dumpable;
  */
 public interface TimeDetectorStrategy extends Dumpable {
 
-    /** Process the suggested time from telephony sources. */
+    @IntDef({ ORIGIN_TELEPHONY, ORIGIN_MANUAL, ORIGIN_NETWORK })
+    @Retention(RetentionPolicy.SOURCE)
+    @interface Origin {}
+
+    /** Used when a time value originated from a telephony signal. */
+    @Origin
+    int ORIGIN_TELEPHONY = 1;
+
+    /** Used when a time value originated from a user / manual settings. */
+    @Origin
+    int ORIGIN_MANUAL = 2;
+
+    /** Used when a time value originated from a network signal. */
+    @Origin
+    int ORIGIN_NETWORK = 3;
+
+    /** Processes the suggested time from telephony sources. */
     void suggestTelephonyTime(@NonNull TelephonyTimeSuggestion timeSuggestion);
 
     /**
-     * Process the suggested manually entered time. Returns {@code false} if the suggestion was
+     * Processes the suggested manually entered time. Returns {@code false} if the suggestion was
      * invalid, or the device configuration prevented the suggestion being used, {@code true} if the
      * suggestion was accepted. A suggestion that is valid but does not change the time because it
      * matches the current device time is considered accepted.
      */
     boolean suggestManualTime(@NonNull ManualTimeSuggestion timeSuggestion);
 
-    /** Process the suggested time from network sources. */
+    /** Processes the suggested time from network sources. */
     void suggestNetworkTime(@NonNull NetworkTimeSuggestion timeSuggestion);
 
-    /** Handle the auto-time setting being toggled on or off. */
+    /**
-    void handleAutoTimeDetectionChanged();
+     * Handles the auto-time configuration changing For example, when the auto-time setting is
+     * toggled on or off.
+     */
+    void handleAutoTimeConfigChanged();
 
     // Utility methods below are to be moved to a better home when one becomes more obvious.
 
@@ -64,4 +87,38 @@ public interface TimeDetectorStrategy extends Dumpable {
         return (referenceClockMillisNow - timeValue.getReferenceTimeMillis())
                 + timeValue.getValue();
     }
+
+    /**
+     * Converts one of the {@code ORIGIN_} constants to a human readable string suitable for config
+     * and debug usage. Throws an {@link IllegalArgumentException} if the value is unrecognized.
+     */
+    static String originToString(@Origin int origin) {
+        switch (origin) {
+            case ORIGIN_MANUAL:
+                return "manual";
+            case ORIGIN_NETWORK:
+                return "network";
+            case ORIGIN_TELEPHONY:
+                return "telephony";
+            default:
+                throw new IllegalArgumentException("origin=" + origin);
+        }
+    }
+
+    /**
+     * Converts a human readable config string to one of the {@code ORIGIN_} constants.
+     * Throws an {@link IllegalArgumentException} if the value is unrecognized.
+     */
+    static @Origin int stringToOrigin(String originString) {
+        switch (originString) {
+            case "manual":
+                return ORIGIN_MANUAL;
+            case "network":
+                return ORIGIN_NETWORK;
+            case "telephony":
+                return ORIGIN_TELEPHONY;
+            default:
+                throw new IllegalArgumentException("originString=" + originString);
+        }
+    }
 }

services/core/java/com/android/server/timedetector/TimeDetectorStrategyCallbackImpl.java

@@ -16,6 +16,8 @@
 
 package com.android.server.timedetector;
 
+import static com.android.server.timedetector.TimeDetectorStrategy.stringToOrigin;
+
 import android.annotation.NonNull;
 import android.app.AlarmManager;
 import android.content.ContentResolver;
@@ -58,6 +60,7 @@ public final class TimeDetectorStrategyCallbackImpl implements TimeDetectorStrat
     @NonNull private final ContentResolver mContentResolver;
     @NonNull private final PowerManager.WakeLock mWakeLock;
     @NonNull private final AlarmManager mAlarmManager;
+    @NonNull private final int[] mOriginPriorities;
 
     public TimeDetectorStrategyCallbackImpl(@NonNull Context context) {
         mContext = Objects.requireNonNull(context);
@@ -72,6 +75,15 @@ public final class TimeDetectorStrategyCallbackImpl implements TimeDetectorStrat
         mSystemClockUpdateThresholdMillis =
                 SystemProperties.getInt("ro.sys.time_detector_update_diff",
                         SYSTEM_CLOCK_UPDATE_THRESHOLD_MILLIS_DEFAULT);
+
+        // TODO(b/172230856): Obtain these values from configuration.
+        String[] originStrings = { "telephony", "network" };
+        int[] origins = new int[originStrings.length];
+        for (int i = 0; i < originStrings.length; i++) {
+            int origin = stringToOrigin(originStrings[i]);
+            origins[i] = origin;
+        }
+        mOriginPriorities = origins;
     }
 
     @Override
@@ -93,6 +105,11 @@ public final class TimeDetectorStrategyCallbackImpl implements TimeDetectorStrat
         return TIME_LOWER_BOUND;
     }
 
+    @Override
+    public int[] getAutoOriginPriorities() {
+        return mOriginPriorities;
+    }
+
     @Override
     public void acquireWakeLock() {
         if (mWakeLock.isHeld()) {

services/core/java/com/android/server/timedetector/TimeDetectorStrategyImpl.java

@@ -16,7 +16,8 @@
 
 package com.android.server.timedetector;
 
-import android.annotation.IntDef;
+import static com.android.server.timedetector.TimeDetectorStrategy.originToString;
+
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.app.AlarmManager;
@@ -33,9 +34,8 @@ import com.android.internal.annotations.VisibleForTesting;
 import com.android.server.timezonedetector.ArrayMapWithHistory;
 import com.android.server.timezonedetector.ReferenceWithHistory;
 
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
 import java.time.Instant;
+import java.util.Arrays;
 
 /**
  * An implementation of {@link TimeDetectorStrategy} that passes telephony and manual suggestions to
@@ -63,22 +63,6 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
     static final long MAX_UTC_TIME_AGE_MILLIS =
             TELEPHONY_BUCKET_COUNT * TELEPHONY_BUCKET_SIZE_MILLIS;
 
-    @IntDef({ ORIGIN_TELEPHONY, ORIGIN_MANUAL, ORIGIN_NETWORK })
-    @Retention(RetentionPolicy.SOURCE)
-    public @interface Origin {}
-
-    /** Used when a time value originated from a telephony signal. */
-    @Origin
-    private static final int ORIGIN_TELEPHONY = 1;
-
-    /** Used when a time value originated from a user / manual settings. */
-    @Origin
-    private static final int ORIGIN_MANUAL = 2;
-
-    /** Used when a time value originated from a network signal. */
-    @Origin
-    private static final int ORIGIN_NETWORK = 3;
-
     /**
      * CLOCK_PARANOIA: The maximum difference allowed between the expected system clock time and the
      * actual system clock time before a warning is logged. Used to help identify situations where
@@ -152,6 +136,12 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
         @NonNull
         Instant autoTimeLowerBound();
 
+        /**
+         * Returns the order to look at time suggestions when automatically detecting time.
+         * See {@code #ORIGIN_} constants
+         */
+        @Origin int[] getAutoOriginPriorities();
+
         /** Acquire a suitable wake lock. Must be followed by {@link #releaseWakeLock()} */
         void acquireWakeLock();
 
@@ -236,12 +226,12 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
     }
 
     @Override
-    public synchronized void handleAutoTimeDetectionChanged() {
+    public synchronized void handleAutoTimeConfigChanged() {
         boolean enabled = mCallback.isAutoTimeDetectionEnabled();
         // When automatic time detection is enabled we update the system clock instantly if we can.
         // Conversely, when automatic time detection is disabled we leave the clock as it is.
         if (enabled) {
-            String reason = "Auto time zone detection setting enabled.";
+            String reason = "Auto time zone detection config changed.";
             doAutoTimeDetection(reason);
         } else {
             // CLOCK_PARANOIA: We are losing "control" of the system clock so we cannot predict what
@@ -362,33 +352,44 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
             return;
         }
 
-        // Android devices currently prioritize any telephony over network signals. There are
+        // Try the different origins one at a time.
-        // carrier compliance tests that would need to be changed before we could ignore NITZ or
+        int[] originPriorities = mCallback.getAutoOriginPriorities();
-        // prefer NTP generally. This check is cheap on devices without telephony hardware.
+        for (int origin : originPriorities) {
+            TimestampedValue<Long> newUtcTime = null;
+            String cause = null;
+            if (origin == ORIGIN_TELEPHONY) {
                 TelephonyTimeSuggestion bestTelephonySuggestion = findBestTelephonySuggestion();
                 if (bestTelephonySuggestion != null) {
-            final TimestampedValue<Long> newUtcTime = bestTelephonySuggestion.getUtcTime();
+                    newUtcTime = bestTelephonySuggestion.getUtcTime();
-            String cause = "Found good telephony suggestion."
+                    cause = "Found good telephony suggestion."
                             + ", bestTelephonySuggestion=" + bestTelephonySuggestion
                             + ", detectionReason=" + detectionReason;
-            setSystemClockIfRequired(ORIGIN_TELEPHONY, newUtcTime, cause);
-            return;
                 }
-
+            } else if (origin == ORIGIN_NETWORK) {
-        // There is no good telephony suggestion, try network.
                 NetworkTimeSuggestion networkSuggestion = findLatestValidNetworkSuggestion();
                 if (networkSuggestion != null) {
-            final TimestampedValue<Long> newUtcTime = networkSuggestion.getUtcTime();
+                    newUtcTime = networkSuggestion.getUtcTime();
-            String cause = "Found good network suggestion."
+                    cause = "Found good network suggestion."
                             + ", networkSuggestion=" + networkSuggestion
                             + ", detectionReason=" + detectionReason;
-            setSystemClockIfRequired(ORIGIN_NETWORK, newUtcTime, cause);
+                }
+            } else {
+                Slog.w(LOG_TAG, "Unknown or unsupported origin=" + origin
+                        + " in " + Arrays.toString(originPriorities)
+                        + ": Skipping");
+            }
+
+            // Update the system clock if a good suggestion has been found.
+            if (newUtcTime != null) {
+                setSystemClockIfRequired(origin, newUtcTime, cause);
                 return;
             }
+        }
 
         if (DBG) {
-            Slog.d(LOG_TAG, "Could not determine time: No best telephony or network suggestion."
+            Slog.d(LOG_TAG, "Could not determine time: No suggestion found in"
-                    + " detectionReason=" + detectionReason);
+                    + " originPriorities=" + Arrays.toString(originPriorities)
+                    + ", detectionReason=" + detectionReason);
         }
     }
 
@@ -473,7 +474,7 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
         // Validate first.
         TimestampedValue<Long> utcTime = timeSuggestion.getUtcTime();
         if (!validateSuggestionUtcTime(elapsedRealtimeMillis, utcTime)) {
-            Slog.w(LOG_TAG, "Existing suggestion found to be invalid "
+            Slog.w(LOG_TAG, "Existing suggestion found to be invalid"
                     + " elapsedRealtimeMillis=" + elapsedRealtimeMillis
                     + ", timeSuggestion=" + timeSuggestion);
             return TELEPHONY_INVALID_SCORE;
@@ -522,7 +523,7 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
             if (!mCallback.isAutoTimeDetectionEnabled()) {
                 if (DBG) {
                     Slog.d(LOG_TAG, "Auto time detection is not enabled."
-                            + " origin=" + origin
+                            + " origin=" + originToString(origin)
                             + ", time=" + time
                             + ", cause=" + cause);
                 }
@@ -532,7 +533,7 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
             if (mCallback.isAutoTimeDetectionEnabled()) {
                 if (DBG) {
                     Slog.d(LOG_TAG, "Auto time detection is enabled."
-                            + " origin=" + origin
+                            + " origin=" + originToString(origin)
                             + ", time=" + time
                             + ", cause=" + cause);
                 }
@@ -554,7 +555,7 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
 
     @GuardedBy("this")
     private boolean setSystemClockUnderWakeLock(
-            int origin, @NonNull TimestampedValue<Long> newTime, @NonNull Object cause) {
+            @Origin int origin, @NonNull TimestampedValue<Long> newTime, @NonNull String cause) {
 
         long elapsedRealtimeMillis = mCallback.elapsedRealtimeMillis();
         boolean isOriginAutomatic = isOriginAutomatic(origin);

services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorServiceTest.java

@@ -245,7 +245,7 @@ public class TimeDetectorServiceTest {
         }
 
         @Override
-        public void handleAutoTimeDetectionChanged() {
+        public void handleAutoTimeConfigChanged() {
             mHandleAutoTimeDetectionChangedCalled = true;
         }
 

services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorStrategyImplTest.java

@@ -16,6 +16,9 @@
 
 package com.android.server.timedetector;
 
+import static com.android.server.timedetector.TimeDetectorStrategy.ORIGIN_NETWORK;
+import static com.android.server.timedetector.TimeDetectorStrategy.ORIGIN_TELEPHONY;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
@@ -29,6 +32,8 @@ import android.os.TimestampedValue;
 
 import androidx.test.runner.AndroidJUnit4;
 
+import com.android.server.timedetector.TimeDetectorStrategy.Origin;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -48,6 +53,9 @@ public class TimeDetectorStrategyImplTest {
                     123456789L /* realtimeClockMillis */,
                     createUtcTime(2010, 5, 23, 12, 0, 0));
 
+    // This is the traditional ordering for time detection on Android.
+    private static final @Origin int [] PROVIDERS_PRIORITY = { ORIGIN_TELEPHONY, ORIGIN_NETWORK };
+
     /**
      * An arbitrary time, very different from the {@link #ARBITRARY_CLOCK_INITIALIZATION_INFO}
      * time. Can be used as the basis for time suggestions.
@@ -488,11 +496,8 @@ public class TimeDetectorStrategyImplTest {
                 .assertLatestTelephonySuggestion(slotIndex, telephonyTimeSuggestion);
     }
 
-    /**
-     * Manual suggestions should be ignored if auto time is enabled.
-     */
     @Test
-    public void testSuggestManualTime_autoTimeEnabled() {
+    public void manualTimeSuggestion_isIgnored_whenAutoTimeEnabled() {
         mScript.pokeFakeClocks(ARBITRARY_CLOCK_INITIALIZATION_INFO)
                 .pokeAutoTimeDetectionEnabled(true);
 
@@ -505,7 +510,7 @@ public class TimeDetectorStrategyImplTest {
     }
 
     @Test
-    public void suggestManualTime_ignoresTimeLowerBound() {
+    public void manualTimeSuggestion_ignoresTimeLowerBound() {
         mScript.pokeFakeClocks(ARBITRARY_CLOCK_INITIALIZATION_INFO)
                 .pokeAutoTimeDetectionEnabled(false);
         Instant suggestedTime = TIME_LOWER_BOUND.minus(Duration.ofDays(1));
@@ -658,6 +663,73 @@ public class TimeDetectorStrategyImplTest {
                 .assertLatestNetworkSuggestion(null);
     }
 
+    @Test
+    public void whenAllTimeSuggestionsAreAvailable_higherPriorityWins_lowerPriorityComesFirst() {
+        mScript.pokeFakeClocks(ARBITRARY_CLOCK_INITIALIZATION_INFO)
+                .pokeAutoTimeDetectionEnabled(true);
+
+        Instant networkTime = ARBITRARY_TEST_TIME;
+        Instant telephonyTime = ARBITRARY_TEST_TIME.plus(Duration.ofDays(30));
+
+        NetworkTimeSuggestion networkTimeSuggestion =
+                mScript.generateNetworkTimeSuggestion(networkTime);
+        TelephonyTimeSuggestion telephonyTimeSuggestion =
+                mScript.generateTelephonyTimeSuggestion(ARBITRARY_SLOT_INDEX, telephonyTime);
+
+        mScript.simulateNetworkTimeSuggestion(networkTimeSuggestion)
+                .simulateTelephonyTimeSuggestion(telephonyTimeSuggestion)
+                .assertLatestNetworkSuggestion(networkTimeSuggestion)
+                .assertLatestTelephonySuggestion(ARBITRARY_SLOT_INDEX, telephonyTimeSuggestion)
+                .verifySystemClockWasSetAndResetCallTracking(telephonyTime.toEpochMilli());
+    }
+
+    @Test
+    public void whenAllTimeSuggestionsAreAvailable_higherPriorityWins_higherPriorityComesFirst() {
+        mScript.pokeFakeClocks(ARBITRARY_CLOCK_INITIALIZATION_INFO)
+                .pokeAutoTimeDetectionEnabled(true);
+
+        Instant networkTime = ARBITRARY_TEST_TIME;
+        Instant telephonyTime = ARBITRARY_TEST_TIME.plus(Duration.ofDays(30));
+
+        NetworkTimeSuggestion networkTimeSuggestion =
+                mScript.generateNetworkTimeSuggestion(networkTime);
+        TelephonyTimeSuggestion telephonyTimeSuggestion =
+                mScript.generateTelephonyTimeSuggestion(ARBITRARY_SLOT_INDEX, telephonyTime);
+
+        mScript.simulateTelephonyTimeSuggestion(telephonyTimeSuggestion)
+                .simulateNetworkTimeSuggestion(networkTimeSuggestion)
+                .assertLatestNetworkSuggestion(networkTimeSuggestion)
+                .assertLatestTelephonySuggestion(ARBITRARY_SLOT_INDEX, telephonyTimeSuggestion)
+                .verifySystemClockWasSetAndResetCallTracking(telephonyTime.toEpochMilli());
+    }
+
+    @Test
+    public void whenHighestPrioritySuggestionIsNotAvailable_fallbacksToNext() {
+        mScript.pokeFakeClocks(ARBITRARY_CLOCK_INITIALIZATION_INFO)
+                .pokeAutoTimeDetectionEnabled(true);
+
+        NetworkTimeSuggestion timeSuggestion =
+                mScript.generateNetworkTimeSuggestion(ARBITRARY_TEST_TIME);
+
+        mScript.simulateNetworkTimeSuggestion(timeSuggestion)
+                .assertLatestNetworkSuggestion(timeSuggestion)
+                .verifySystemClockWasSetAndResetCallTracking(ARBITRARY_TEST_TIME.toEpochMilli());
+    }
+
+    @Test
+    public void suggestionsFromSourceNotListedInPrioritiesList_areIgnored() {
+        mScript.pokeFakeClocks(ARBITRARY_CLOCK_INITIALIZATION_INFO)
+                .pokeAutoTimeDetectionEnabled(true)
+                .pokeAutoOriginPriorities(new int[]{ORIGIN_TELEPHONY});
+
+        NetworkTimeSuggestion timeSuggestion = mScript.generateNetworkTimeSuggestion(
+                ARBITRARY_TEST_TIME);
+
+        mScript.simulateNetworkTimeSuggestion(timeSuggestion)
+                .assertLatestNetworkSuggestion(timeSuggestion)
+                .verifySystemClockWasNotSetAndResetCallTracking();
+    }
+
     /**
      * A fake implementation of TimeDetectorStrategy.Callback. Besides tracking changes and behaving
      * like the real thing should, it also asserts preconditions.
@@ -668,6 +740,7 @@ public class TimeDetectorStrategyImplTest {
         private long mElapsedRealtimeMillis;
         private long mSystemClockMillis;
         private int mSystemClockUpdateThresholdMillis = 2000;
+        private int[] mAutoOriginPriorities = PROVIDERS_PRIORITY;
 
         // Tracking operations.
         private boolean mSystemClockWasSet;
@@ -687,6 +760,11 @@ public class TimeDetectorStrategyImplTest {
             return TIME_LOWER_BOUND;
         }
 
+        @Override
+        public int[] getAutoOriginPriorities() {
+            return mAutoOriginPriorities;
+        }
+
         @Override
         public void acquireWakeLock() {
             if (mWakeLockAcquired) {
@@ -736,6 +814,10 @@ public class TimeDetectorStrategyImplTest {
             mAutoTimeDetectionEnabled = enabled;
         }
 
+        void pokeAutoOriginPriorities(@Origin int[] autoOriginPriorities) {
+            mAutoOriginPriorities = autoOriginPriorities;
+        }
+
         long peekElapsedRealtimeMillis() {
             return mElapsedRealtimeMillis;
         }
@@ -804,6 +886,11 @@ public class TimeDetectorStrategyImplTest {
             return this;
         }
 
+        Script pokeAutoOriginPriorities(@Origin int[] autoOriginPriorites) {
+            mFakeCallback.pokeAutoOriginPriorities(autoOriginPriorites);
+            return this;
+        }
+
         long peekElapsedRealtimeMillis() {
             return mFakeCallback.peekElapsedRealtimeMillis();
         }
@@ -836,7 +923,7 @@ public class TimeDetectorStrategyImplTest {
 
         Script simulateAutoTimeDetectionToggle() {
             mFakeCallback.simulateAutoTimeZoneDetectionToggle();
-            mTimeDetectorStrategy.handleAutoTimeDetectionChanged();
+            mTimeDetectorStrategy.handleAutoTimeConfigChanged();
             return this;
         }