diff options
author | 2023-09-05 08:48:28 +0000 | |
---|---|---|
committer | 2023-09-05 08:48:28 +0000 | |
commit | 0266a4dc61b9ff6576a57fc1ba5b20d0ac62b236 (patch) | |
tree | 5571bfd0237b0ca41242057dec95d48cc3529aa8 | |
parent | 8c3ef35b51d2125fff3843f696d65c03bb9ef4d4 (diff) | |
parent | 0f9db7e4d29c382c73b72a45b06bd6a2171ca431 (diff) |
Merge "Time service / strategy config refactoring" into udc-qpr-dev am: 0f9db7e4d2
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/24552768
Change-Id: I5747d8593233b7d4cb1c6b032e59e00ca4a44a78
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
10 files changed, 385 insertions, 146 deletions
diff --git a/services/core/java/com/android/server/timedetector/ConfigurationInternal.java b/services/core/java/com/android/server/timedetector/ConfigurationInternal.java index 4f221b532b75..97181948352b 100644 --- a/services/core/java/com/android/server/timedetector/ConfigurationInternal.java +++ b/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; diff --git a/services/core/java/com/android/server/timedetector/EnvironmentImpl.java b/services/core/java/com/android/server/timedetector/EnvironmentImpl.java index fc960d83dc3b..c52f8f887463 100644 --- a/services/core/java/com/android/server/timedetector/EnvironmentImpl.java +++ b/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( @@ -59,19 +57,6 @@ final class EnvironmentImpl implements TimeDetectorStrategyImpl.Environment { } @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()) { Slog.wtf(LOG_TAG, "WakeLock " + mWakeLock + " already held"); @@ -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 diff --git a/services/core/java/com/android/server/timedetector/TimeDetectorService.java b/services/core/java/com/android/server/timedetector/TimeDetectorService.java index 22f096b11f18..d88f4268434b 100644 --- a/services/core/java/com/android/server/timedetector/TimeDetectorService.java +++ b/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. diff --git a/services/core/java/com/android/server/timedetector/TimeDetectorStrategy.java b/services/core/java/com/android/server/timedetector/TimeDetectorStrategy.java index 11cec6663a37..15c0a809cde8 100644 --- a/services/core/java/com/android/server/timedetector/TimeDetectorStrategy.java +++ b/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); diff --git a/services/core/java/com/android/server/timedetector/TimeDetectorStrategyImpl.java b/services/core/java/com/android/server/timedetector/TimeDetectorStrategyImpl.java index b293bacfdc0d..fd35df61c952 100644 --- a/services/core/java/com/android/server/timedetector/TimeDetectorStrategyImpl.java +++ b/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; - - 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; + /** + * Handles a configuration change notification. + */ + private synchronized void handleConfigurationInternalMaybeChanged() { + String logMsg = "handleConfigurationInternalMaybeChanged:"; + updateCurrentConfigurationInternalIfRequired(logMsg); + } + + @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(); diff --git a/services/tests/servicestests/src/com/android/server/timedetector/FakeServiceConfigAccessor.java b/services/tests/servicestests/src/com/android/server/timedetector/FakeServiceConfigAccessor.java index 93464cdd13d0..d9bc74dfb1cb 100644 --- a/services/tests/servicestests/src/com/android/server/timedetector/FakeServiceConfigAccessor.java +++ b/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(); + } } } diff --git a/services/tests/servicestests/src/com/android/server/timedetector/FakeTimeDetectorStrategy.java b/services/tests/servicestests/src/com/android/server/timedetector/FakeTimeDetectorStrategy.java index 87aa2725491b..a7a9c0cd86f2 100644 --- a/services/tests/servicestests/src/com/android/server/timedetector/FakeTimeDetectorStrategy.java +++ b/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; @@ -51,6 +63,26 @@ public class FakeTimeDetectorStrategy implements TimeDetectorStrategy { } @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) { } diff --git a/services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorInternalImplTest.java b/services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorInternalImplTest.java index a0845a64757e..de5a37b43df8 100644 --- a/services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorInternalImplTest.java +++ b/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) diff --git a/services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorServiceTest.java b/services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorServiceTest.java index daa682342836..6b2d4b01dd08 100644 --- a/services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorServiceTest.java +++ b/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); diff --git a/services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorStrategyImplTest.java b/services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorStrategyImplTest.java index 4df21e00d61c..dd58135a8e87 100644 --- a/services/tests/servicestests/src/com/android/server/timedetector/TimeDetectorStrategyImplTest.java +++ b/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,20 +2040,14 @@ 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()); @@ -1960,16 +2055,6 @@ public class TimeDetectorStrategyImplTest { } @Override - public void setConfigurationInternalChangeListener(StateChangeListener listener) { - mConfigurationInternalChangeListener = Objects.requireNonNull(listener); - } - - @Override - public ConfigurationInternal getCurrentUserConfigurationInternal() { - return mConfigurationInternal; - } - - @Override public void acquireWakeLock() { if (mWakeLockAcquired) { fail("Wake lock already acquired"); @@ -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); + } } |