Merge "Time service / strategy config refactoring"

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

@@ -244,6 +244,8 @@ public final class ConfigurationInternal {
                 && mAutoDetectionEnabledSetting == that.mAutoDetectionEnabledSetting
                 && mUserId == that.mUserId && mUserConfigAllowed == that.mUserConfigAllowed
                 && mSystemClockUpdateThresholdMillis == that.mSystemClockUpdateThresholdMillis
+                && mSystemClockConfidenceThresholdMillis
+                == that.mSystemClockConfidenceThresholdMillis
                 && mAutoSuggestionLowerBound.equals(that.mAutoSuggestionLowerBound)
                 && mManualSuggestionLowerBound.equals(that.mManualSuggestionLowerBound)
                 && mSuggestionUpperBound.equals(that.mSuggestionUpperBound)
@@ -253,7 +255,8 @@ public final class ConfigurationInternal {
     @Override
     public int hashCode() {
         int result = Objects.hash(mAutoDetectionSupported, mAutoDetectionEnabledSetting, mUserId,
-                mUserConfigAllowed, mSystemClockUpdateThresholdMillis, mAutoSuggestionLowerBound,
+                mUserConfigAllowed, mSystemClockUpdateThresholdMillis,
+                mSystemClockConfidenceThresholdMillis, mAutoSuggestionLowerBound,
                 mManualSuggestionLowerBound, mSuggestionUpperBound);
         result = 31 * result + Arrays.hashCode(mOriginPriorities);
         return result;

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

@@ -22,15 +22,16 @@ import android.content.Context;
 import android.os.Handler;
 import android.os.PowerManager;
 import android.os.SystemClock;
+import android.util.IndentingPrintWriter;
 import android.util.Slog;
 
 import com.android.server.AlarmManagerInternal;
 import com.android.server.LocalServices;
 import com.android.server.SystemClockTime;
 import com.android.server.SystemClockTime.TimeConfidence;
-import com.android.server.timezonedetector.StateChangeListener;
 
-import java.io.PrintWriter;
+import java.time.Duration;
+import java.time.Instant;
 import java.util.Objects;
 
 /**
@@ -41,14 +42,11 @@ final class EnvironmentImpl implements TimeDetectorStrategyImpl.Environment {
     private static final String LOG_TAG = TimeDetectorService.TAG;
 
     @NonNull private final Handler mHandler;
-    @NonNull private final ServiceConfigAccessor mServiceConfigAccessor;
     @NonNull private final PowerManager.WakeLock mWakeLock;
     @NonNull private final AlarmManagerInternal mAlarmManagerInternal;
 
-    EnvironmentImpl(@NonNull Context context, @NonNull Handler handler,
-            @NonNull ServiceConfigAccessor serviceConfigAccessor) {
+    EnvironmentImpl(@NonNull Context context, @NonNull Handler handler) {
         mHandler = Objects.requireNonNull(handler);
-        mServiceConfigAccessor = Objects.requireNonNull(serviceConfigAccessor);
 
         PowerManager powerManager = context.getSystemService(PowerManager.class);
         mWakeLock = Objects.requireNonNull(
@@ -58,19 +56,6 @@ final class EnvironmentImpl implements TimeDetectorStrategyImpl.Environment {
                 LocalServices.getService(AlarmManagerInternal.class));
     }
 
-    @Override
-    public void setConfigurationInternalChangeListener(
-            @NonNull StateChangeListener listener) {
-        StateChangeListener stateChangeListener =
-                () -> mHandler.post(listener::onChange);
-        mServiceConfigAccessor.addConfigurationInternalChangeListener(stateChangeListener);
-    }
-
-    @Override
-    public ConfigurationInternal getCurrentUserConfigurationInternal() {
-        return mServiceConfigAccessor.getCurrentUserConfigurationInternal();
-    }
-
     @Override
     public void acquireWakeLock() {
         if (mWakeLock.isHeld()) {
@@ -126,8 +111,19 @@ final class EnvironmentImpl implements TimeDetectorStrategyImpl.Environment {
     }
 
     @Override
-    public void dumpDebugLog(@NonNull PrintWriter printWriter) {
-        SystemClockTime.dump(printWriter);
+    public void dumpDebugLog(@NonNull IndentingPrintWriter pw) {
+        long elapsedRealtimeMillis = elapsedRealtimeMillis();
+        pw.printf("elapsedRealtimeMillis()=%s (%s)\n",
+                Duration.ofMillis(elapsedRealtimeMillis), elapsedRealtimeMillis);
+        long systemClockMillis = systemClockMillis();
+        pw.printf("systemClockMillis()=%s (%s)\n",
+                Instant.ofEpochMilli(systemClockMillis), systemClockMillis);
+        pw.println("systemClockConfidence()=" + systemClockConfidence());
+
+        pw.println("SystemClockTime debug log:");
+        pw.increaseIndent();
+        SystemClockTime.dump(pw);
+        pw.decreaseIndent();
     }
 
     @Override

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

@@ -96,8 +96,8 @@ public final class TimeDetectorService extends ITimeDetectorService.Stub
 
             CallerIdentityInjector callerIdentityInjector = CallerIdentityInjector.REAL;
             TimeDetectorService service = new TimeDetectorService(
-                    context, handler, callerIdentityInjector, serviceConfigAccessor,
-                    timeDetectorStrategy, NtpTrustedTime.getInstance(context));
+                    context, handler, callerIdentityInjector, timeDetectorStrategy,
+                    NtpTrustedTime.getInstance(context));
 
             // Publish the binder service so it can be accessed from other (appropriately
             // permissioned) processes.
@@ -108,7 +108,6 @@ public final class TimeDetectorService extends ITimeDetectorService.Stub
     @NonNull private final Handler mHandler;
     @NonNull private final Context mContext;
     @NonNull private final CallerIdentityInjector mCallerIdentityInjector;
-    @NonNull private final ServiceConfigAccessor mServiceConfigAccessor;
     @NonNull private final TimeDetectorStrategy mTimeDetectorStrategy;
     @NonNull private final NtpTrustedTime mNtpTrustedTime;
 
@@ -123,20 +122,18 @@ public final class TimeDetectorService extends ITimeDetectorService.Stub
     @VisibleForTesting
     public TimeDetectorService(@NonNull Context context, @NonNull Handler handler,
             @NonNull CallerIdentityInjector callerIdentityInjector,
-            @NonNull ServiceConfigAccessor serviceConfigAccessor,
             @NonNull TimeDetectorStrategy timeDetectorStrategy,
             @NonNull NtpTrustedTime ntpTrustedTime) {
         mContext = Objects.requireNonNull(context);
         mHandler = Objects.requireNonNull(handler);
         mCallerIdentityInjector = Objects.requireNonNull(callerIdentityInjector);
-        mServiceConfigAccessor = Objects.requireNonNull(serviceConfigAccessor);
         mTimeDetectorStrategy = Objects.requireNonNull(timeDetectorStrategy);
         mNtpTrustedTime = Objects.requireNonNull(ntpTrustedTime);
 
-        // Wire up a change listener so that ITimeZoneDetectorListeners can be notified when
-        // the configuration changes for any reason.
-        mServiceConfigAccessor.addConfigurationInternalChangeListener(
-                () -> mHandler.post(this::handleConfigurationInternalChangedOnHandlerThread));
+        // Wire up a change listener so that ITimeDetectorListeners can be notified when the
+        // detector state changes for any reason.
+        mTimeDetectorStrategy.addChangeListener(
+                () -> mHandler.post(this::handleChangeOnHandlerThread));
     }
 
     @Override
@@ -151,10 +148,8 @@ public final class TimeDetectorService extends ITimeDetectorService.Stub
 
         final long token = mCallerIdentityInjector.clearCallingIdentity();
         try {
-            ConfigurationInternal configurationInternal =
-                    mServiceConfigAccessor.getConfigurationInternal(userId);
-            final boolean bypassUserPolicyCheck = false;
-            return configurationInternal.createCapabilitiesAndConfig(bypassUserPolicyCheck);
+            final boolean bypassUserPolicyChecks = false;
+            return mTimeDetectorStrategy.getCapabilitiesAndConfig(userId, bypassUserPolicyChecks);
         } finally {
             mCallerIdentityInjector.restoreCallingIdentity(token);
         }
@@ -180,9 +175,9 @@ public final class TimeDetectorService extends ITimeDetectorService.Stub
 
         final long token = mCallerIdentityInjector.clearCallingIdentity();
         try {
-            final boolean bypassUserPolicyCheck = false;
-            return mServiceConfigAccessor.updateConfiguration(
-                    resolvedUserId, configuration, bypassUserPolicyCheck);
+            final boolean bypassUserPolicyChecks = false;
+            return mTimeDetectorStrategy.updateConfiguration(
+                    resolvedUserId, configuration, bypassUserPolicyChecks);
         } finally {
             mCallerIdentityInjector.restoreCallingIdentity(token);
         }
@@ -262,7 +257,7 @@ public final class TimeDetectorService extends ITimeDetectorService.Stub
         }
     }
 
-    private void handleConfigurationInternalChangedOnHandlerThread() {
+    private void handleChangeOnHandlerThread() {
         // Configuration has changed, but each user may have a different view of the configuration.
         // It's possible that this will cause unnecessary notifications but that shouldn't be a
         // problem.

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

@@ -21,6 +21,8 @@ import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.UserIdInt;
 import android.app.time.ExternalTimeSuggestion;
+import android.app.time.TimeCapabilitiesAndConfig;
+import android.app.time.TimeConfiguration;
 import android.app.time.TimeState;
 import android.app.time.UnixEpochTime;
 import android.app.timedetector.ManualTimeSuggestion;
@@ -87,6 +89,48 @@ public interface TimeDetectorStrategy extends Dumpable {
      */
     boolean confirmTime(@NonNull UnixEpochTime confirmationTime);
 
+    /**
+     * Adds a listener that will be triggered when something changes that could affect the result
+     * of the {@link #getCapabilitiesAndConfig} call for the <em>current user only</em>. This
+     * includes the current user changing. This is exposed so that (indirect) users like SettingsUI
+     * can monitor for changes to data derived from {@link TimeCapabilitiesAndConfig} and update
+     * the UI accordingly.
+     */
+    void addChangeListener(@NonNull StateChangeListener listener);
+
+    /**
+     * Returns a {@link TimeCapabilitiesAndConfig} object for the specified user.
+     *
+     * <p>The strategy is dependent on device state like current user, settings and device config.
+     * These updates are usually handled asynchronously, so callers should expect some delay between
+     * a change being made directly to services like settings and the strategy becoming aware of
+     * them. Changes made via {@link #updateConfiguration} will be visible immediately.
+     *
+     * @param userId the user ID to retrieve the information for
+     * @param bypassUserPolicyChecks {@code true} for device policy manager use cases where device
+     *   policy restrictions that should apply to actual users can be ignored
+     */
+    TimeCapabilitiesAndConfig getCapabilitiesAndConfig(
+            @UserIdInt int userId, boolean bypassUserPolicyChecks);
+
+    /**
+     * Updates the configuration properties that control a device's time behavior.
+     *
+     * <p>This method returns {@code true} if the configuration was changed, {@code false}
+     * otherwise.
+     *
+     * <p>See {@link #getCapabilitiesAndConfig} for guarantees about visibility of updates to
+     * subsequent calls.
+     *
+     * @param userId the current user ID, supplied to make sure that the asynchronous process
+     *   that happens when users switch is completed when the call is made
+     * @param configuration the configuration changes
+     * @param bypassUserPolicyChecks {@code true} for device policy manager use cases where device
+     *   policy restrictions that should apply to actual users can be ignored
+     */
+    boolean updateConfiguration(@UserIdInt int userId,
+            @NonNull TimeConfiguration configuration, boolean bypassUserPolicyChecks);
+
     /** Processes the suggested time from telephony sources. */
     void suggestTelephonyTime(@NonNull TelephonyTimeSuggestion suggestion);
 

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

@@ -30,6 +30,7 @@ import android.annotation.UserIdInt;
 import android.app.time.ExternalTimeSuggestion;
 import android.app.time.TimeCapabilities;
 import android.app.time.TimeCapabilitiesAndConfig;
+import android.app.time.TimeConfiguration;
 import android.app.time.TimeState;
 import android.app.time.UnixEpochTime;
 import android.app.timedetector.ManualTimeSuggestion;
@@ -48,10 +49,10 @@ import com.android.server.timezonedetector.ArrayMapWithHistory;
 import com.android.server.timezonedetector.ReferenceWithHistory;
 import com.android.server.timezonedetector.StateChangeListener;
 
-import java.io.PrintWriter;
-import java.time.Duration;
 import java.time.Instant;
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Objects;
 
 /**
@@ -94,6 +95,12 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
     @NonNull
     private final Environment mEnvironment;
 
+    @NonNull
+    private final ServiceConfigAccessor mServiceConfigAccessor;
+
+    @GuardedBy("this")
+    @NonNull private final List<StateChangeListener> mStateChangeListeners = new ArrayList<>();
+
     @GuardedBy("this")
     @NonNull
     private ConfigurationInternal mCurrentConfigurationInternal;
@@ -139,16 +146,6 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
      */
     public interface Environment {
 
-        /**
-         * Sets a {@link StateChangeListener} that will be invoked when there are any changes that
-         * could affect the content of {@link ConfigurationInternal}.
-         * This is invoked during system server setup.
-         */
-        void setConfigurationInternalChangeListener(@NonNull StateChangeListener listener);
-
-        /** Returns the {@link ConfigurationInternal} for the current user. */
-        @NonNull ConfigurationInternal getCurrentUserConfigurationInternal();
-
         /** Acquire a suitable wake lock. Must be followed by {@link #releaseWakeLock()} */
         void acquireWakeLock();
 
@@ -174,16 +171,15 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
         /** Release the wake lock acquired by a call to {@link #acquireWakeLock()}. */
         void releaseWakeLock();
 
-
         /**
          * Adds a standalone entry to the time debug log.
          */
         void addDebugLogEntry(@NonNull String logMsg);
 
         /**
-         * Dumps the time debug log to the supplied {@link PrintWriter}.
+         * Dumps the time debug log to the supplied {@link IndentingPrintWriter}.
          */
-        void dumpDebugLog(PrintWriter printWriter);
+        void dumpDebugLog(IndentingPrintWriter ipw);
 
         /**
          * Requests that the supplied runnable is invoked asynchronously.
@@ -195,19 +191,23 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
             @NonNull Context context, @NonNull Handler handler,
             @NonNull ServiceConfigAccessor serviceConfigAccessor) {
 
-        TimeDetectorStrategyImpl.Environment environment =
-                new EnvironmentImpl(context, handler, serviceConfigAccessor);
-        return new TimeDetectorStrategyImpl(environment);
+        TimeDetectorStrategyImpl.Environment environment = new EnvironmentImpl(context, handler);
+        return new TimeDetectorStrategyImpl(environment, serviceConfigAccessor);
     }
 
     @VisibleForTesting
-    TimeDetectorStrategyImpl(@NonNull Environment environment) {
+    TimeDetectorStrategyImpl(@NonNull Environment environment,
+            @NonNull ServiceConfigAccessor serviceConfigAccessor) {
         mEnvironment = Objects.requireNonNull(environment);
+        mServiceConfigAccessor = Objects.requireNonNull(serviceConfigAccessor);
 
         synchronized (this) {
-            mEnvironment.setConfigurationInternalChangeListener(
-                    this::handleConfigurationInternalChanged);
-            mCurrentConfigurationInternal = mEnvironment.getCurrentUserConfigurationInternal();
+            // Listen for config and user changes and get an initial snapshot of configuration.
+            StateChangeListener stateChangeListener = this::handleConfigurationInternalMaybeChanged;
+            mServiceConfigAccessor.addConfigurationInternalChangeListener(stateChangeListener);
+
+            // Initialize mCurrentConfigurationInternal with a starting value.
+            updateCurrentConfigurationInternalIfRequired("TimeDetectorStrategyImpl:");
         }
     }
 
@@ -421,6 +421,57 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
         }
     }
 
+    @GuardedBy("this")
+    private void notifyStateChangeListenersAsynchronously() {
+        for (StateChangeListener listener : mStateChangeListeners) {
+            // This is queuing asynchronous notification, so no need to surrender the "this" lock.
+            mEnvironment.runAsync(listener::onChange);
+        }
+    }
+
+    @Override
+    public synchronized void addChangeListener(@NonNull StateChangeListener listener) {
+        mStateChangeListeners.add(listener);
+    }
+
+    @Override
+    public synchronized TimeCapabilitiesAndConfig getCapabilitiesAndConfig(@UserIdInt int userId,
+            boolean bypassUserPolicyChecks) {
+        ConfigurationInternal configurationInternal;
+        if (mCurrentConfigurationInternal.getUserId() == userId) {
+            // Use the cached snapshot we have.
+            configurationInternal = mCurrentConfigurationInternal;
+        } else {
+            // This is not a common case: It would be unusual to want the configuration for a user
+            // other than the "current" user, but it is supported because it is trivial to do so.
+            // Unlike the current user config, there's no cached copy to worry about so read it
+            // directly from mServiceConfigAccessor.
+            configurationInternal = mServiceConfigAccessor.getConfigurationInternal(userId);
+        }
+        return configurationInternal.createCapabilitiesAndConfig(bypassUserPolicyChecks);
+    }
+
+    @Override
+    public synchronized boolean updateConfiguration(@UserIdInt int userId,
+            @NonNull TimeConfiguration configuration, boolean bypassUserPolicyChecks) {
+        // Write-through
+        boolean updateSuccessful = mServiceConfigAccessor.updateConfiguration(
+                userId, configuration, bypassUserPolicyChecks);
+
+        // The update above will trigger config update listeners asynchronously if they are needed,
+        // but that could mean an immediate call to getCapabilitiesAndConfig() for the current user
+        // wouldn't see the update. So, handle the cache update and notifications here. When the
+        // async update listener triggers it will find everything already up to date and do nothing.
+        if (updateSuccessful) {
+            String logMsg = "updateConfiguration:"
+                    + " userId=" + userId
+                    + ", configuration=" + configuration
+                    + ", bypassUserPolicyChecks=" + bypassUserPolicyChecks;
+            updateCurrentConfigurationInternalIfRequired(logMsg);
+        }
+        return updateSuccessful;
+    }
+
     @Override
     public synchronized void suggestTelephonyTime(@NonNull TelephonyTimeSuggestion suggestion) {
         // Empty time suggestion means that telephony network connectivity has been lost.
@@ -448,26 +499,49 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
         doAutoTimeDetection(reason);
     }
 
-    private synchronized void handleConfigurationInternalChanged() {
-        ConfigurationInternal currentUserConfig =
-                mEnvironment.getCurrentUserConfigurationInternal();
-        String logMsg = "handleConfigurationInternalChanged:"
-                + " oldConfiguration=" + mCurrentConfigurationInternal
-                + ", newConfiguration=" + currentUserConfig;
-        addDebugLogEntry(logMsg);
-        mCurrentConfigurationInternal = currentUserConfig;
+    /**
+     * Handles a configuration change notification.
+     */
+    private synchronized void handleConfigurationInternalMaybeChanged() {
+        String logMsg = "handleConfigurationInternalMaybeChanged:";
+        updateCurrentConfigurationInternalIfRequired(logMsg);
+    }
 
-        boolean autoDetectionEnabled =
-                mCurrentConfigurationInternal.getAutoDetectionEnabledBehavior();
-        // 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 (autoDetectionEnabled) {
-            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
-            // it should be in future.
-            mLastAutoSystemClockTimeSet = null;
+    @GuardedBy("this")
+    private void updateCurrentConfigurationInternalIfRequired(@NonNull String logMsg) {
+        ConfigurationInternal newCurrentConfigurationInternal =
+                mServiceConfigAccessor.getCurrentUserConfigurationInternal();
+        // mCurrentConfigurationInternal is null the first time this method is called.
+        ConfigurationInternal oldCurrentConfigurationInternal = mCurrentConfigurationInternal;
+
+        // If the configuration actually changed, update the cached copy synchronously and do
+        // other necessary house-keeping / (async) listener notifications.
+        if (!newCurrentConfigurationInternal.equals(oldCurrentConfigurationInternal)) {
+            mCurrentConfigurationInternal = newCurrentConfigurationInternal;
+
+            logMsg = new StringBuilder(logMsg)
+                    .append(" [oldConfiguration=").append(oldCurrentConfigurationInternal)
+                    .append(", newConfiguration=").append(newCurrentConfigurationInternal)
+                    .append("]")
+                    .toString();
+            addDebugLogEntry(logMsg);
+
+            // The configuration and maybe the status changed so notify listeners.
+            notifyStateChangeListenersAsynchronously();
+
+            boolean autoDetectionEnabled =
+                    mCurrentConfigurationInternal.getAutoDetectionEnabledBehavior();
+            // 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 (autoDetectionEnabled) {
+                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 it should be in future.
+                mLastAutoSystemClockTimeSet = null;
+            }
         }
     }
 
@@ -489,13 +563,11 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
         ipw.println("[Capabilities="
                 + mCurrentConfigurationInternal.createCapabilitiesAndConfig(bypassUserPolicyChecks)
                 + "]");
-        long elapsedRealtimeMillis = mEnvironment.elapsedRealtimeMillis();
-        ipw.printf("mEnvironment.elapsedRealtimeMillis()=%s (%s)\n",
-                Duration.ofMillis(elapsedRealtimeMillis), elapsedRealtimeMillis);
-        long systemClockMillis = mEnvironment.systemClockMillis();
-        ipw.printf("mEnvironment.systemClockMillis()=%s (%s)\n",
-                Instant.ofEpochMilli(systemClockMillis), systemClockMillis);
-        ipw.println("mEnvironment.systemClockConfidence()=" + mEnvironment.systemClockConfidence());
+
+        ipw.println("mEnvironment:");
+        ipw.increaseIndent();
+        mEnvironment.dumpDebugLog(ipw);
+        ipw.decreaseIndent();
 
         ipw.println("Time change log:");
         ipw.increaseIndent(); // level 2
@@ -525,6 +597,11 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy {
         ipw.decreaseIndent(); // level 1
     }
 
+    @VisibleForTesting
+    public synchronized ConfigurationInternal getCachedCapabilitiesAndConfigForTests() {
+        return mCurrentConfigurationInternal;
+    }
+
     @GuardedBy("this")
     private boolean storeTelephonySuggestion(@NonNull TelephonyTimeSuggestion suggestion) {
         UnixEpochTime newUnixEpochTime = suggestion.getUnixEpochTime();

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

@@ -35,7 +35,7 @@ public class FakeServiceConfigAccessor implements ServiceConfigAccessor {
 
     private final List<StateChangeListener> mConfigurationInternalChangeListeners =
             new ArrayList<>();
-    private ConfigurationInternal mConfigurationInternal;
+    private ConfigurationInternal mCurrentUserConfigurationInternal;
 
     @Override
     public void addConfigurationInternalChangeListener(StateChangeListener listener) {
@@ -49,21 +49,23 @@ public class FakeServiceConfigAccessor implements ServiceConfigAccessor {
 
     @Override
     public ConfigurationInternal getCurrentUserConfigurationInternal() {
-        return mConfigurationInternal;
+        return mCurrentUserConfigurationInternal;
     }
 
     @Override
     public boolean updateConfiguration(
-            @UserIdInt int userID, @NonNull TimeConfiguration requestedChanges,
+            @UserIdInt int userId, @NonNull TimeConfiguration requestedChanges,
             boolean bypassUserPolicyChecks) {
-        assertNotNull(mConfigurationInternal);
+        assertNotNull(mCurrentUserConfigurationInternal);
         assertNotNull(requestedChanges);
 
+        ConfigurationInternal toUpdate = getConfigurationInternal(userId);
+
         // Simulate the real strategy's behavior: the new configuration will be updated to be the
         // old configuration merged with the new if the user has the capability to up the settings.
         // Then, if the configuration changed, the change listener is invoked.
         TimeCapabilitiesAndConfig capabilitiesAndConfig =
-                mConfigurationInternal.createCapabilitiesAndConfig(bypassUserPolicyChecks);
+                toUpdate.createCapabilitiesAndConfig(bypassUserPolicyChecks);
         TimeCapabilities capabilities = capabilitiesAndConfig.getCapabilities();
         TimeConfiguration configuration = capabilitiesAndConfig.getConfiguration();
         TimeConfiguration newConfiguration =
@@ -73,28 +75,36 @@ public class FakeServiceConfigAccessor implements ServiceConfigAccessor {
         }
 
         if (!newConfiguration.equals(capabilitiesAndConfig.getConfiguration())) {
-            mConfigurationInternal = mConfigurationInternal.merge(newConfiguration);
+            mCurrentUserConfigurationInternal = toUpdate.merge(newConfiguration);
 
             // Note: Unlike the real strategy, the listeners are invoked synchronously.
-            simulateConfigurationChangeForTests();
+            notifyConfigurationChange();
         }
         return true;
     }
 
-    void initializeConfiguration(ConfigurationInternal configurationInternal) {
-        mConfigurationInternal = configurationInternal;
+
+    void initializeCurrentUserConfiguration(ConfigurationInternal configurationInternal) {
+        mCurrentUserConfigurationInternal = configurationInternal;
     }
 
-    void simulateConfigurationChangeForTests() {
-        for (StateChangeListener listener : mConfigurationInternalChangeListeners) {
-            listener.onChange();
-        }
+    void simulateCurrentUserConfigurationInternalChange(
+            ConfigurationInternal configurationInternal) {
+        mCurrentUserConfigurationInternal = configurationInternal;
+        // Note: Unlike the real strategy, the listeners are invoked synchronously.
+        notifyConfigurationChange();
     }
 
     @Override
     public ConfigurationInternal getConfigurationInternal(int userId) {
         assertEquals("Multi-user testing not supported currently",
-                userId, mConfigurationInternal.getUserId());
-        return mConfigurationInternal;
+                userId, mCurrentUserConfigurationInternal.getUserId());
+        return mCurrentUserConfigurationInternal;
+    }
+
+    private void notifyConfigurationChange() {
+        for (StateChangeListener listener : mConfigurationInternalChangeListeners) {
+            listener.onChange();
+        }
     }
 }

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

@@ -18,6 +18,8 @@ package com.android.server.timedetector;
 
 import android.annotation.UserIdInt;
 import android.app.time.ExternalTimeSuggestion;
+import android.app.time.TimeCapabilitiesAndConfig;
+import android.app.time.TimeConfiguration;
 import android.app.time.TimeState;
 import android.app.time.UnixEpochTime;
 import android.app.timedetector.ManualTimeSuggestion;
@@ -31,10 +33,20 @@ import com.android.server.timezonedetector.StateChangeListener;
  * in tests.
  */
 public class FakeTimeDetectorStrategy implements TimeDetectorStrategy {
+    private final FakeServiceConfigAccessor mFakeServiceConfigAccessor;
+
     // State
     private TimeState mTimeState;
     private NetworkTimeSuggestion mLatestNetworkTimeSuggestion;
 
+    FakeTimeDetectorStrategy() {
+        mFakeServiceConfigAccessor = new FakeServiceConfigAccessor();
+    }
+
+    void initializeConfiguration(ConfigurationInternal configuration) {
+        mFakeServiceConfigAccessor.initializeCurrentUserConfiguration(configuration);
+    }
+
     @Override
     public TimeState getTimeState() {
         return mTimeState;
@@ -50,6 +62,26 @@ public class FakeTimeDetectorStrategy implements TimeDetectorStrategy {
         return false;
     }
 
+    @Override
+    public void addChangeListener(StateChangeListener listener) {
+        mFakeServiceConfigAccessor.addConfigurationInternalChangeListener(listener);
+    }
+
+    @Override
+    public TimeCapabilitiesAndConfig getCapabilitiesAndConfig(int userId,
+            boolean bypassUserPolicyChecks) {
+        ConfigurationInternal configurationInternal =
+                mFakeServiceConfigAccessor.getConfigurationInternal(userId);
+        return configurationInternal.createCapabilitiesAndConfig(bypassUserPolicyChecks);
+    }
+
+    @Override
+    public boolean updateConfiguration(int userId, TimeConfiguration configuration,
+            boolean bypassUserPolicyChecks) {
+        return mFakeServiceConfigAccessor.updateConfiguration(
+                userId, configuration, bypassUserPolicyChecks);
+    }
+
     @Override
     public void suggestTelephonyTime(TelephonyTimeSuggestion suggestion) {
     }

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

@@ -89,7 +89,7 @@ public class TimeDetectorInternalImplTest {
     public void testGetCapabilitiesAndConfigForDpm() throws Exception {
         final boolean autoDetectionEnabled = true;
         ConfigurationInternal testConfig = createConfigurationInternal(autoDetectionEnabled);
-        mFakeServiceConfigAccessorSpy.initializeConfiguration(testConfig);
+        mFakeServiceConfigAccessorSpy.initializeCurrentUserConfiguration(testConfig);
 
         TimeCapabilitiesAndConfig actualCapabilitiesAndConfig =
                 mTimeDetectorInternal.getCapabilitiesAndConfigForDpm();
@@ -108,7 +108,8 @@ public class TimeDetectorInternalImplTest {
         final boolean autoDetectionEnabled = false;
         ConfigurationInternal initialConfigurationInternal =
                 createConfigurationInternal(autoDetectionEnabled);
-        mFakeServiceConfigAccessorSpy.initializeConfiguration(initialConfigurationInternal);
+        mFakeServiceConfigAccessorSpy.initializeCurrentUserConfiguration(
+                initialConfigurationInternal);
 
         TimeConfiguration timeConfiguration = new TimeConfiguration.Builder()
                 .setAutoDetectionEnabled(true)

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

@@ -83,7 +83,6 @@ public class TimeDetectorServiceTest {
     private HandlerThread mHandlerThread;
     private TestHandler mTestHandler;
     private TestCallerIdentityInjector mTestCallerIdentityInjector;
-    private FakeServiceConfigAccessor mFakeServiceConfigAccessorSpy;
     private FakeTimeDetectorStrategy mFakeTimeDetectorStrategySpy;
 
     private NtpTrustedTime mMockNtpTrustedTime;
@@ -101,13 +100,12 @@ public class TimeDetectorServiceTest {
         mTestCallerIdentityInjector = new TestCallerIdentityInjector();
         mTestCallerIdentityInjector.initializeCallingUserId(ARBITRARY_USER_ID);
 
-        mFakeServiceConfigAccessorSpy = spy(new FakeServiceConfigAccessor());
         mFakeTimeDetectorStrategySpy = spy(new FakeTimeDetectorStrategy());
         mMockNtpTrustedTime = mock(NtpTrustedTime.class);
 
         mTimeDetectorService = new TimeDetectorService(
                 mMockContext, mTestHandler, mTestCallerIdentityInjector,
-                mFakeServiceConfigAccessorSpy, mFakeTimeDetectorStrategySpy, mMockNtpTrustedTime);
+                mFakeTimeDetectorStrategySpy, mMockNtpTrustedTime);
     }
 
     @After
@@ -132,14 +130,14 @@ public class TimeDetectorServiceTest {
 
         ConfigurationInternal configuration =
                 createConfigurationInternal(true /* autoDetectionEnabled*/);
-        mFakeServiceConfigAccessorSpy.initializeConfiguration(configuration);
+        mFakeTimeDetectorStrategySpy.initializeConfiguration(configuration);
 
         TimeCapabilitiesAndConfig actualCapabilitiesAndConfig =
                 mTimeDetectorService.getCapabilitiesAndConfig();
         verify(mMockContext).enforceCallingPermission(
                 eq(android.Manifest.permission.MANAGE_TIME_AND_ZONE_DETECTION), anyString());
         int expectedUserId = mTestCallerIdentityInjector.getCallingUserId();
-        verify(mFakeServiceConfigAccessorSpy).getConfigurationInternal(expectedUserId);
+        verify(mFakeTimeDetectorStrategySpy).getCapabilitiesAndConfig(expectedUserId, false);
 
         boolean bypassUserPolicyChecks = false;
         TimeCapabilitiesAndConfig expectedCapabilitiesAndConfig =
@@ -174,7 +172,7 @@ public class TimeDetectorServiceTest {
     public void testListenerRegistrationAndCallbacks() throws Exception {
         ConfigurationInternal initialConfiguration =
                 createConfigurationInternal(false /* autoDetectionEnabled */);
-        mFakeServiceConfigAccessorSpy.initializeConfiguration(initialConfiguration);
+        mFakeTimeDetectorStrategySpy.initializeConfiguration(initialConfiguration);
 
         IBinder mockListenerBinder = mock(IBinder.class);
         ITimeDetectorListener mockListener = mock(ITimeDetectorListener.class);

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

@@ -29,14 +29,22 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 
 import android.annotation.UserIdInt;
 import android.app.time.ExternalTimeSuggestion;
+import android.app.time.TimeCapabilitiesAndConfig;
+import android.app.time.TimeConfiguration;
 import android.app.time.TimeState;
 import android.app.time.UnixEpochTime;
 import android.app.timedetector.ManualTimeSuggestion;
 import android.app.timedetector.TelephonyTimeSuggestion;
 import android.os.TimestampedValue;
+import android.util.IndentingPrintWriter;
 
 import com.android.server.SystemClockTime.TimeConfidence;
 import com.android.server.timedetector.TimeDetectorStrategy.Origin;
@@ -47,14 +55,12 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
-import java.io.PrintWriter;
 import java.time.Duration;
 import java.time.Instant;
 import java.time.LocalDateTime;
 import java.time.ZoneOffset;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Objects;
 
 import junitparams.JUnitParamsRunner;
 import junitparams.Parameters;
@@ -120,13 +126,108 @@ public class TimeDetectorStrategyImplTest {
                     .build();
 
     private FakeEnvironment mFakeEnvironment;
+    private FakeServiceConfigAccessor mFakeServiceConfigAccessorSpy;
+    private TimeDetectorStrategyImpl mTimeDetectorStrategy;
 
     @Before
     public void setUp() {
         mFakeEnvironment = new FakeEnvironment();
-        mFakeEnvironment.initializeConfig(CONFIG_AUTO_DISABLED);
         mFakeEnvironment.initializeFakeClocks(
                 ARBITRARY_CLOCK_INITIALIZATION_INFO, TIME_CONFIDENCE_LOW);
+
+        mFakeServiceConfigAccessorSpy = spy(new FakeServiceConfigAccessor());
+        mFakeServiceConfigAccessorSpy.initializeCurrentUserConfiguration(CONFIG_AUTO_DISABLED);
+
+        mTimeDetectorStrategy = new TimeDetectorStrategyImpl(
+                mFakeEnvironment, mFakeServiceConfigAccessorSpy);
+    }
+
+    @Test
+    public void testChangeListenerBehavior() throws Exception {
+        TestStateChangeListener stateChangeListener = new TestStateChangeListener();
+        mTimeDetectorStrategy.addChangeListener(stateChangeListener);
+
+        boolean bypassUserPolicyChecks = false;
+
+        // Report a config change, but not one that actually changes anything.
+        {
+            mFakeServiceConfigAccessorSpy.simulateCurrentUserConfigurationInternalChange(
+                    CONFIG_AUTO_DISABLED);
+            assertStateChangeNotificationsSent(stateChangeListener, 0);
+            assertEquals(CONFIG_AUTO_DISABLED,
+                    mTimeDetectorStrategy.getCachedCapabilitiesAndConfigForTests());
+        }
+
+        // Report a config change that actually changes something.
+        {
+            mFakeServiceConfigAccessorSpy.simulateCurrentUserConfigurationInternalChange(
+                    CONFIG_AUTO_ENABLED);
+            assertStateChangeNotificationsSent(stateChangeListener, 1);
+            assertEquals(CONFIG_AUTO_ENABLED,
+                    mTimeDetectorStrategy.getCachedCapabilitiesAndConfigForTests());
+        }
+
+        // Perform a (current user) update via the strategy.
+        {
+            TimeConfiguration requestedChanges =
+                    new TimeConfiguration.Builder().setAutoDetectionEnabled(false).build();
+            mTimeDetectorStrategy.updateConfiguration(
+                    ARBITRARY_USER_ID, requestedChanges, bypassUserPolicyChecks);
+            assertStateChangeNotificationsSent(stateChangeListener, 1);
+        }
+    }
+
+    // Current user behavior: the strategy caches and returns the latest configuration.
+    @Test
+    public void testReadAndWriteConfiguration() throws Exception {
+        ConfigurationInternal currentUserConfig = CONFIG_AUTO_ENABLED;
+        mFakeServiceConfigAccessorSpy.simulateCurrentUserConfigurationInternalChange(
+                currentUserConfig);
+
+        final boolean bypassUserPolicyChecks = false;
+
+        ConfigurationInternal cachedConfigurationInternal =
+                mTimeDetectorStrategy.getCachedCapabilitiesAndConfigForTests();
+        assertEquals(currentUserConfig, cachedConfigurationInternal);
+
+        // Confirm getCapabilitiesAndConfig() does not call through to the ServiceConfigAccessor.
+        {
+            reset(mFakeServiceConfigAccessorSpy);
+            TimeCapabilitiesAndConfig actualCapabilitiesAndConfig =
+                    mTimeDetectorStrategy.getCapabilitiesAndConfig(
+                            currentUserConfig.getUserId(), bypassUserPolicyChecks);
+            verify(mFakeServiceConfigAccessorSpy, never()).getConfigurationInternal(
+                    currentUserConfig.getUserId());
+
+            TimeCapabilitiesAndConfig expectedCapabilitiesAndConfig =
+                    currentUserConfig.createCapabilitiesAndConfig(bypassUserPolicyChecks);
+            assertEquals(expectedCapabilitiesAndConfig.getCapabilities(),
+                    actualCapabilitiesAndConfig.getCapabilities());
+            assertEquals(expectedCapabilitiesAndConfig.getConfiguration(),
+                    actualCapabilitiesAndConfig.getConfiguration());
+        }
+
+        // Confirm updateConfiguration() calls through to the ServiceConfigAccessor and updates
+        // the cached copy.
+        {
+            boolean newAutoDetectionEnabled =
+                    !cachedConfigurationInternal.getAutoDetectionEnabledBehavior();
+            TimeConfiguration requestedChanges = new TimeConfiguration.Builder()
+                    .setAutoDetectionEnabled(newAutoDetectionEnabled)
+                    .build();
+            ConfigurationInternal expectedConfigAfterChange =
+                    new ConfigurationInternal.Builder(cachedConfigurationInternal)
+                            .setAutoDetectionEnabledSetting(newAutoDetectionEnabled)
+                            .build();
+
+            reset(mFakeServiceConfigAccessorSpy);
+            mTimeDetectorStrategy.updateConfiguration(
+                    currentUserConfig.getUserId(), requestedChanges, bypassUserPolicyChecks);
+            verify(mFakeServiceConfigAccessorSpy, times(1)).updateConfiguration(
+                    currentUserConfig.getUserId(), requestedChanges, bypassUserPolicyChecks);
+            assertEquals(expectedConfigAfterChange,
+                    mTimeDetectorStrategy.getCachedCapabilitiesAndConfigForTests());
+        }
     }
 
     @Test
@@ -1939,36 +2040,20 @@ public class TimeDetectorStrategyImplTest {
 
         private final List<Runnable> mAsyncRunnables = new ArrayList<>();
 
-        private ConfigurationInternal mConfigurationInternal;
         private boolean mWakeLockAcquired;
         private long mElapsedRealtimeMillis;
         private long mSystemClockMillis;
         private int mSystemClockConfidence = TIME_CONFIDENCE_LOW;
-        private StateChangeListener mConfigurationInternalChangeListener;
 
         // Tracking operations.
         private boolean mSystemClockWasSet;
 
-        void initializeConfig(ConfigurationInternal configurationInternal) {
-            mConfigurationInternal = configurationInternal;
-        }
-
         public void initializeFakeClocks(
                 TimestampedValue<Instant> timeInfo, @TimeConfidence int timeConfidence) {
             pokeElapsedRealtimeMillis(timeInfo.getReferenceTimeMillis());
             pokeSystemClockMillis(timeInfo.getValue().toEpochMilli(), timeConfidence);
         }
 
-        @Override
-        public void setConfigurationInternalChangeListener(StateChangeListener listener) {
-            mConfigurationInternalChangeListener = Objects.requireNonNull(listener);
-        }
-
-        @Override
-        public ConfigurationInternal getCurrentUserConfigurationInternal() {
-            return mConfigurationInternal;
-        }
-
         @Override
         public void acquireWakeLock() {
             if (mWakeLockAcquired) {
@@ -2019,7 +2104,7 @@ public class TimeDetectorStrategyImplTest {
         }
 
         @Override
-        public void dumpDebugLog(PrintWriter printWriter) {
+        public void dumpDebugLog(IndentingPrintWriter pw) {
             // No-op for tests
         }
 
@@ -2037,11 +2122,6 @@ public class TimeDetectorStrategyImplTest {
             mAsyncRunnables.clear();
         }
 
-        void simulateConfigurationInternalChange(ConfigurationInternal configurationInternal) {
-            mConfigurationInternal = configurationInternal;
-            mConfigurationInternalChangeListener.onChange();
-        }
-
         void pokeElapsedRealtimeMillis(long elapsedRealtimeMillis) {
             mElapsedRealtimeMillis = elapsedRealtimeMillis;
         }
@@ -2095,13 +2175,6 @@ public class TimeDetectorStrategyImplTest {
      */
     private class Script {
 
-        private final TimeDetectorStrategyImpl mTimeDetectorStrategy;
-
-        Script() {
-            mFakeEnvironment = new FakeEnvironment();
-            mTimeDetectorStrategy = new TimeDetectorStrategyImpl(mFakeEnvironment);
-        }
-
         Script pokeFakeClocks(TimestampedValue<Instant> initialClockTime,
                 @TimeConfidence int timeConfidence) {
             mFakeEnvironment.pokeElapsedRealtimeMillis(initialClockTime.getReferenceTimeMillis());
@@ -2122,7 +2195,8 @@ public class TimeDetectorStrategyImplTest {
          * Simulates the user / user's configuration changing.
          */
         Script simulateConfigurationInternalChange(ConfigurationInternal configurationInternal) {
-            mFakeEnvironment.simulateConfigurationInternalChange(configurationInternal);
+            mFakeServiceConfigAccessorSpy.simulateCurrentUserConfigurationInternalChange(
+                    configurationInternal);
             return this;
         }
 
@@ -2167,14 +2241,15 @@ public class TimeDetectorStrategyImplTest {
 
         Script simulateAutoTimeDetectionToggle() {
             ConfigurationInternal configurationInternal =
-                    mFakeEnvironment.getCurrentUserConfigurationInternal();
+                    mFakeServiceConfigAccessorSpy.getCurrentUserConfigurationInternal();
             boolean autoDetectionEnabledSetting =
                     !configurationInternal.getAutoDetectionEnabledSetting();
             ConfigurationInternal newConfigurationInternal =
                     new ConfigurationInternal.Builder(configurationInternal)
                             .setAutoDetectionEnabledSetting(autoDetectionEnabledSetting)
                             .build();
-            mFakeEnvironment.simulateConfigurationInternalChange(newConfigurationInternal);
+            mFakeServiceConfigAccessorSpy.simulateCurrentUserConfigurationInternalChange(
+                    newConfigurationInternal);
             return this;
         }
 
@@ -2389,4 +2464,12 @@ public class TimeDetectorStrategyImplTest {
         return LocalDateTime.of(year, monthInYear, day, hourOfDay, minute, second)
                 .toInstant(ZoneOffset.UTC);
     }
+
+    private void assertStateChangeNotificationsSent(
+            TestStateChangeListener stateChangeListener, int expectedCount) {
+        // The fake environment needs to be told to run posted work.
+        mFakeEnvironment.runAsyncRunnables();
+
+        stateChangeListener.assertNotificationsReceivedAndReset(expectedCount);
+    }
 }