From bded1d2f5d0bfaf6224cbca74590b899e56b61a9 Mon Sep 17 00:00:00 2001
From: Neil Fuller <nfuller@google.com>
Date: Fri, 12 Nov 2021 13:47:06 +0000
Subject: [PATCH] A mixed bag of tidy ups / refactoring

A mixed bag of tidy ups / refactoring to make the following commits
smaller. This change is not intended to have any user-facing functional
impact.

Summary of changes that aren't obvious comment / code / string tidy-ups:

1) Support for geolocation time zone detection support configuration via
the system property
"persist.sys.location_time_zone_detection_feature_supported" has been
removed. I haven't used this recently and "real" devices get to use
server flags to turn the feature off now. Turning the feature on will
have to be done in .xml config after this.

2) Remove Dumpable.Container from TimeZoneDetectorInternal and
TimeZoneDetectorStrategy as it is not used.

3) Add support for adding Dumpable to TimeZoneDetectorService, because
it will be used in a following commit. Removed Dumpable.Container
interface as it's not currently pulling its weight, with only one user
where nothing refers to the interface.

4) Put all debugging logging under control of a constant in
TimeZoneDetectorService.

Bug: 197624972
Test: treehugger
Test: atest services/tests/servicestests/src/com/android/server/timezonedetector/
Change-Id: I3c3895b3ec7b0ad88fc3776221616a973d54863b
---
 .../server/timezonedetector/Dumpable.java     | 14 ------
 .../timezonedetector/EnvironmentImpl.java     | 26 ++++------
 .../GeolocationTimeZoneSuggestion.java        | 11 ++--
 .../ServiceConfigAccessor.java                | 50 ++++++++-----------
 .../TimeZoneDetectorInternal.java             |  2 +-
 .../TimeZoneDetectorInternalImpl.java         |  5 --
 .../TimeZoneDetectorService.java              | 38 +++++++++++---
 .../TimeZoneDetectorStrategy.java             | 11 +++-
 .../TimeZoneDetectorStrategyImpl.java         | 45 +++++++----------
 .../LocationTimeZoneManagerService.java       |  3 +-
 .../FakeTimeZoneDetectorStrategy.java         | 13 -----
 .../TimeZoneDetectorInternalImplTest.java     | 10 ----
 .../TimeZoneDetectorServiceTest.java          |  6 ++-
 .../TimeZoneDetectorStrategyImplTest.java     | 45 ++++++-----------
 14 files changed, 115 insertions(+), 164 deletions(-)

diff --git a/services/core/java/com/android/server/timezonedetector/Dumpable.java b/services/core/java/com/android/server/timezonedetector/Dumpable.java
index 5603c38bd0aee..1dd963795f6d0 100644
--- a/services/core/java/com/android/server/timezonedetector/Dumpable.java
+++ b/services/core/java/com/android/server/timezonedetector/Dumpable.java
@@ -24,18 +24,4 @@ public interface Dumpable {
 
     /** Dump internal state. */
     void dump(@NonNull IndentingPrintWriter pw, @Nullable String[] args);
-
-    /**
-     * An interface that can be used expose when one component allows another to be registered so
-     * that it is dumped at the same time.
-     */
-    interface Container {
-
-        /**
-         * Registers the supplied {@link Dumpable}. When the implementation is dumped
-         * {@link Dumpable#dump(IndentingPrintWriter, String[])} should be called on the
-         * {@code dumpable}.
-         */
-        void addDumpable(@NonNull Dumpable dumpable);
-    }
 }
diff --git a/services/core/java/com/android/server/timezonedetector/EnvironmentImpl.java b/services/core/java/com/android/server/timezonedetector/EnvironmentImpl.java
index b84f8a850ba76..fc6e372b80414 100644
--- a/services/core/java/com/android/server/timezonedetector/EnvironmentImpl.java
+++ b/services/core/java/com/android/server/timezonedetector/EnvironmentImpl.java
@@ -72,8 +72,8 @@ final class EnvironmentImpl implements TimeZoneDetectorStrategyImpl.Environment
         mLocationManager = context.getSystemService(LocationManager.class);
         mServiceConfigAccessor = Objects.requireNonNull(serviceConfigAccessor);
 
-        // Wire up the config change listeners. All invocations are performed on the mHandler
-        // thread.
+        // Wire up the config change listeners for anything that could affect the return values from
+        // this object. All listener invocations are performed on the mHandler thread.
 
         // Listen for the user changing / the user's location mode changing.
         IntentFilter filter = new IntentFilter();
@@ -88,25 +88,19 @@ final class EnvironmentImpl implements TimeZoneDetectorStrategyImpl.Environment
 
         // Add async callbacks for global settings being changed.
         ContentResolver contentResolver = mContext.getContentResolver();
+        ContentObserver contentObserver = new ContentObserver(mHandler) {
+            @Override
+            public void onChange(boolean selfChange) {
+                handleConfigChangeOnHandlerThread();
+            }
+        };
         contentResolver.registerContentObserver(
-                Settings.Global.getUriFor(Settings.Global.AUTO_TIME_ZONE), true,
-                new ContentObserver(mHandler) {
-                    @Override
-                    public void onChange(boolean selfChange) {
-                        handleConfigChangeOnHandlerThread();
-                    }
-                });
+                Settings.Global.getUriFor(Settings.Global.AUTO_TIME_ZONE), true, contentObserver);
 
         // Add async callbacks for user scoped location settings being changed.
         contentResolver.registerContentObserver(
                 Settings.Secure.getUriFor(Settings.Secure.LOCATION_TIME_ZONE_DETECTION_ENABLED),
-                true,
-                new ContentObserver(mHandler) {
-                    @Override
-                    public void onChange(boolean selfChange) {
-                        handleConfigChangeOnHandlerThread();
-                    }
-                }, UserHandle.USER_ALL);
+                true, contentObserver, UserHandle.USER_ALL);
     }
 
     private void handleConfigChangeOnHandlerThread() {
diff --git a/services/core/java/com/android/server/timezonedetector/GeolocationTimeZoneSuggestion.java b/services/core/java/com/android/server/timezonedetector/GeolocationTimeZoneSuggestion.java
index 3b3254985abca..f4a6ef0bf25c0 100644
--- a/services/core/java/com/android/server/timezonedetector/GeolocationTimeZoneSuggestion.java
+++ b/services/core/java/com/android/server/timezonedetector/GeolocationTimeZoneSuggestion.java
@@ -39,12 +39,11 @@ import java.util.StringTokenizer;
  * <ul>
  *     <li>{@code effectiveFromElapsedMillis}: The time according to the elapsed realtime clock
  *     after which the suggestion should be considered in effect. For example, when a location fix
- *     used to establish the time zone is old, then the suggestion
- *     {@code effectiveFromElapsedMillis} should reflect this and indicates the time zone that was
- *     detected / correct at that time. The time_zone_detector is only expected to use the latest
- *     suggestion it has received, and so later suggestions always counteract previous suggestions.
- *     The inclusion of this information means that the time_zone_detector can take into account
- *     ordering when comparing suggestions from different sources.
+ *     used to establish the time zone is old, then the suggestion's {@code
+ *     effectiveFromElapsedMillis} should reflect this and indicates the time zone that was
+ *     detected / correct at that time. The inclusion of this information means that the
+ *     time_zone_detector <em>may</em> take this into account if comparing suggestions or signals
+ *     from different sources.
  *     <br />Note: Because the times can be back-dated, time_zone_detector can be sent a sequence of
  *     suggestions where the {@code effectiveFromElapsedMillis} of later suggestions is before
  *     the {@code effectiveFromElapsedMillis} of an earlier one.</li>
diff --git a/services/core/java/com/android/server/timezonedetector/ServiceConfigAccessor.java b/services/core/java/com/android/server/timezonedetector/ServiceConfigAccessor.java
index 20f4fa17ad67e..f7ac9b66bb02c 100644
--- a/services/core/java/com/android/server/timezonedetector/ServiceConfigAccessor.java
+++ b/services/core/java/com/android/server/timezonedetector/ServiceConfigAccessor.java
@@ -21,7 +21,6 @@ import android.annotation.StringDef;
 import android.content.Context;
 import android.content.pm.PackageManager;
 import android.content.res.Resources;
-import android.os.SystemProperties;
 import android.util.ArraySet;
 
 import com.android.internal.R;
@@ -64,11 +63,11 @@ public final class ServiceConfigAccessor {
     public static final @ProviderMode String PROVIDER_MODE_ENABLED = "enabled";
 
     /**
-     * Device config keys that affect the {@link TimeZoneDetectorService} service and {@link
-     * com.android.server.timezonedetector.location.LocationTimeZoneManagerService}.
+     * Device config keys that can affect {@link
+     * com.android.server.timezonedetector.location.LocationTimeZoneManagerService} behavior.
      */
-    private static final Set<String> SERVER_FLAGS_KEYS_TO_WATCH = Collections.unmodifiableSet(
-            new ArraySet<>(new String[] {
+    private static final Set<String> LOCATION_TIME_ZONE_MANAGER_SERVER_FLAGS_KEYS_TO_WATCH =
+            Collections.unmodifiableSet(new ArraySet<>(new String[] {
                     ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_FEATURE_SUPPORTED,
                     ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_SETTING_ENABLED_DEFAULT,
                     ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_SETTING_ENABLED_OVERRIDE,
@@ -94,14 +93,6 @@ public final class ServiceConfigAccessor {
 
     @NonNull private final Context mContext;
 
-    /**
-     * An ultimate "feature switch" for location-based time zone detection. If this is
-     * {@code false}, the device cannot support the feature without a config change or a reboot:
-     * This affects what services are started on boot to minimize expense when the feature is not
-     * wanted.
-     */
-    private final boolean mGeoDetectionFeatureSupportedInConfig;
-
     @NonNull private final ServerFlags mServerFlags;
 
     /**
@@ -148,14 +139,6 @@ public final class ServiceConfigAccessor {
     private ServiceConfigAccessor(@NonNull Context context) {
         mContext = Objects.requireNonNull(context);
 
-        // The config value is expected to be the main feature flag. Platform developers can also
-        // force enable the feature using a persistent system property. Because system properties
-        // can change, this value is cached and only changes on reboot.
-        mGeoDetectionFeatureSupportedInConfig = context.getResources().getBoolean(
-                com.android.internal.R.bool.config_enableGeolocationTimeZoneDetection)
-                || SystemProperties.getBoolean(
-                "persist.sys.location_time_zone_detection_feature_supported", false);
-
         mServerFlags = ServerFlags.getInstance(mContext);
     }
 
@@ -170,14 +153,15 @@ public final class ServiceConfigAccessor {
     }
 
     /**
-     * Adds a listener that will be called when server flags related to this class change. The
-     * callbacks are delivered on the main looper thread.
+     * Adds a listener that will be called when server flags related to location_time_zone_manager
+     * change. The callbacks are delivered on the main looper thread.
      *
      * <p>Note: Only for use by long-lived objects. There is deliberately no associated remove
      * method.
      */
-    public void addListener(@NonNull ConfigurationChangeListener listener) {
-        mServerFlags.addListener(listener, SERVER_FLAGS_KEYS_TO_WATCH);
+    public void addLocationTimeZoneManagerConfigListener(
+            @NonNull ConfigurationChangeListener listener) {
+        mServerFlags.addListener(listener, LOCATION_TIME_ZONE_MANAGER_SERVER_FLAGS_KEYS_TO_WATCH);
     }
 
     /** Returns {@code true} if any form of automatic time zone detection is supported. */
@@ -197,11 +181,19 @@ public final class ServiceConfigAccessor {
     /**
      * Returns {@code true} if the location-based time zone detection feature can be supported on
      * this device at all according to config. When {@code false}, implies that various other
-     * location-based settings will be turned off or rendered meaningless. Typically {@link
-     * #isGeoTimeZoneDetectionFeatureSupported()} should be used instead.
+     * location-based services and settings will be turned off or rendered meaningless.
+     *
+     * <p>This is the ultimate "feature switch" for location-based time zone detection. If this is
+     * {@code false}, the device cannot support the feature without a config change or a reboot:
+     * This affects what services are started on boot to minimize expense when the feature is not
+     * wanted.
+     *
+     * Typically {@link #isGeoTimeZoneDetectionFeatureSupported()} should be used except during
+     * boot.
      */
     public boolean isGeoTimeZoneDetectionFeatureSupportedInConfig() {
-        return mGeoDetectionFeatureSupportedInConfig;
+        return mContext.getResources().getBoolean(
+                com.android.internal.R.bool.config_enableGeolocationTimeZoneDetection);
     }
 
     /**
@@ -213,7 +205,7 @@ public final class ServiceConfigAccessor {
         // 1) Be turned on in config.
         // 2) Not be turned off via a server flag.
         // 3) There must be at least one location time zone provider enabled / configured.
-        return mGeoDetectionFeatureSupportedInConfig
+        return isGeoTimeZoneDetectionFeatureSupportedInConfig()
                 && isGeoTimeZoneDetectionFeatureSupportedInternal()
                 && atLeastOneProviderIsEnabled();
     }
diff --git a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorInternal.java b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorInternal.java
index d429b8762a7ca..41611772bc9ff 100644
--- a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorInternal.java
+++ b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorInternal.java
@@ -25,7 +25,7 @@ import android.annotation.NonNull;
  * <p>The methods on this class can be called from any thread.
  * @hide
  */
-public interface TimeZoneDetectorInternal extends Dumpable.Container {
+public interface TimeZoneDetectorInternal {
 
     /** Adds a listener that will be invoked when {@link ConfigurationInternal} may have changed. */
     void addConfigurationListener(@NonNull ConfigurationChangeListener listener);
diff --git a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorInternalImpl.java b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorInternalImpl.java
index 4e78f5aa444c3..ca87811ce34d9 100644
--- a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorInternalImpl.java
+++ b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorInternalImpl.java
@@ -56,11 +56,6 @@ public final class TimeZoneDetectorInternalImpl implements TimeZoneDetectorInter
         }
     }
 
-    @Override
-    public void addDumpable(@NonNull Dumpable dumpable) {
-        mTimeZoneDetectorStrategy.addDumpable(dumpable);
-    }
-
     @Override
     public void addConfigurationListener(ConfigurationChangeListener listener) {
         synchronized (mConfigurationListeners) {
diff --git a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorService.java b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorService.java
index b1b537be1d295..e6a58a1d2c429 100644
--- a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorService.java
+++ b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorService.java
@@ -45,6 +45,8 @@ import com.android.server.SystemService;
 
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Objects;
 
 /**
@@ -59,6 +61,7 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub
         implements IBinder.DeathRecipient {
 
     static final String TAG = "time_zone_detector";
+    static final boolean DBG = false;
 
     /**
      * Handles the service lifecycle for {@link TimeZoneDetectorService} and
@@ -112,17 +115,22 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub
      */
     @GuardedBy("mListeners")
     @NonNull
-    private final ArrayMap<IBinder, ITimeZoneDetectorListener> mListeners =
-            new ArrayMap<>();
+    private final ArrayMap<IBinder, ITimeZoneDetectorListener> mListeners = new ArrayMap<>();
+
+    /**
+     * References to components that should be dumped when {@link
+     * #dump(FileDescriptor, PrintWriter, String[])} is called on the service.
+     */
+    @GuardedBy("mDumpables")
+    private final List<Dumpable> mDumpables = new ArrayList<>();
 
     private static TimeZoneDetectorService create(
             @NonNull Context context, @NonNull Handler handler,
             @NonNull TimeZoneDetectorStrategy timeZoneDetectorStrategy) {
 
         CallerIdentityInjector callerIdentityInjector = CallerIdentityInjector.REAL;
-        TimeZoneDetectorService service = new TimeZoneDetectorService(
+        return new TimeZoneDetectorService(
                 context, handler, callerIdentityInjector, timeZoneDetectorStrategy);
-        return service;
     }
 
     @VisibleForTesting
@@ -251,7 +259,7 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub
             if (!removedListener) {
                 Slog.w(TAG, "Notified of binder death for who=" + who
                         + ", but did not remove any listeners."
-                        + " mConfigurationListeners=" + mListeners);
+                        + " mListeners=" + mListeners);
             }
         }
     }
@@ -314,8 +322,17 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub
     boolean isGeoTimeZoneDetectionSupported() {
         enforceManageTimeZoneDetectorPermission();
 
-        return ServiceConfigAccessor.getInstance(mContext)
-                .isGeoTimeZoneDetectionFeatureSupported();
+        return ServiceConfigAccessor.getInstance(mContext).isGeoTimeZoneDetectionFeatureSupported();
+    }
+
+    /**
+     * Registers the supplied {@link Dumpable} for dumping. When the service is dumped
+     * {@link Dumpable#dump(IndentingPrintWriter, String[])} will be called on the {@code dumpable}.
+     */
+    void addDumpable(@NonNull Dumpable dumpable) {
+        synchronized (mDumpables) {
+            mDumpables.add(dumpable);
+        }
     }
 
     @Override
@@ -325,6 +342,13 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub
 
         IndentingPrintWriter ipw = new IndentingPrintWriter(pw);
         mTimeZoneDetectorStrategy.dump(ipw, args);
+
+        synchronized (mDumpables) {
+            for (Dumpable dumpable : mDumpables) {
+                dumpable.dump(ipw, args);
+            }
+        }
+
         ipw.flush();
     }
 
diff --git a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategy.java b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategy.java
index e3f31b6aa3265..e2cc679197008 100644
--- a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategy.java
+++ b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategy.java
@@ -30,7 +30,7 @@ import android.util.IndentingPrintWriter;
  * Suggestions are acted on or ignored as needed, depending on previously received suggestions and
  * the current user's configuration (see {@link ConfigurationInternal}).
  *
- * <p>Devices can have zero, one or two automatic time zone detection algorithm available at any
+ * <p>Devices can have zero, one or two automatic time zone detection algorithms available at any
  * point in time.
  *
  * <p>The two automatic detection algorithms supported are "telephony" and "geolocation". Algorithm
@@ -63,6 +63,13 @@ import android.util.IndentingPrintWriter;
  * have an empty suggestion submitted in order to "withdraw" their previous suggestion otherwise it
  * will remain in use.
  *
+ * <p>The strategy uses only one algorithm at a time and does not attempt consensus even when
+ * more than one is available on a device. This "use only one" behavior is deliberate as different
+ * algorithms have edge cases and blind spots that lead to incorrect answers or uncertainty;
+ * different algorithms aren't guaranteed to agree, and algorithms may frequently lose certainty as
+ * users enter areas without the necessary signals. Ultimately, with no perfect algorithm available,
+ * the user is left to choose which algorithm works best for their circumstances.
+ *
  * <p>Threading:
  *
  * <p>Suggestion calls with a void return type may be handed off to a separate thread and handled
@@ -73,7 +80,7 @@ import android.util.IndentingPrintWriter;
  *
  * @hide
  */
-public interface TimeZoneDetectorStrategy extends Dumpable, Dumpable.Container {
+public interface TimeZoneDetectorStrategy extends Dumpable {
 
     /**
      * Adds a listener that will be triggered whenever {@link ConfigurationInternal} may have
diff --git a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java
index ab2a88b44a87b..1ebb983eef576 100644
--- a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java
+++ b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java
@@ -98,7 +98,7 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat
     }
 
     private static final String LOG_TAG = TimeZoneDetectorService.TAG;
-    private static final boolean DBG = false;
+    private static final boolean DBG = TimeZoneDetectorService.DBG;
 
     /**
      * The abstract score for an empty or invalid telephony suggestion.
@@ -168,7 +168,7 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat
 
     @GuardedBy("this")
     @NonNull
-    private List<ConfigurationChangeListener> mConfigChangeListeners = new ArrayList<>();
+    private final List<ConfigurationChangeListener> mConfigChangeListeners = new ArrayList<>();
 
     /**
      * A log that records the decisions / decision metadata that affected the device's time zone.
@@ -183,7 +183,7 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat
      * to be stable.
      */
     @GuardedBy("this")
-    private ArrayMapWithHistory<Integer, QualifiedTelephonyTimeZoneSuggestion>
+    private final ArrayMapWithHistory<Integer, QualifiedTelephonyTimeZoneSuggestion>
             mTelephonySuggestionsBySlotIndex =
             new ArrayMapWithHistory<>(KEEP_SUGGESTION_HISTORY_SIZE);
 
@@ -192,18 +192,16 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat
      * detection then the latest suggestion is cleared.
      */
     @GuardedBy("this")
-    private ReferenceWithHistory<GeolocationTimeZoneSuggestion> mLatestGeoLocationSuggestion =
+    private final ReferenceWithHistory<GeolocationTimeZoneSuggestion> mLatestGeoLocationSuggestion =
             new ReferenceWithHistory<>(KEEP_SUGGESTION_HISTORY_SIZE);
 
     /**
      * The latest manual suggestion received.
      */
     @GuardedBy("this")
-    private ReferenceWithHistory<ManualTimeZoneSuggestion> mLatestManualSuggestion =
+    private final ReferenceWithHistory<ManualTimeZoneSuggestion> mLatestManualSuggestion =
             new ReferenceWithHistory<>(KEEP_SUGGESTION_HISTORY_SIZE);
 
-    @GuardedBy("this")
-    private final List<Dumpable> mDumpables = new ArrayList<>();
 
     /**
      * Creates a new instance of {@link TimeZoneDetectorStrategyImpl}.
@@ -293,7 +291,9 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat
 
         if (currentUserConfig.getGeoDetectionEnabledBehavior()) {
             // Only store a geolocation suggestion if geolocation detection is currently enabled.
-            // See also clearGeolocationSuggestionIfNeeded().
+            // See also handleConfigChanged(), which can clear mLatestGeoLocationSuggestion.
+            // The suggestion's "effective from" time is ignored: we currently assume suggestions
+            // are made in a sensible order and the most recent is always the best one to use.
             mLatestGeoLocationSuggestion.set(suggestion);
 
             // Now perform auto time zone detection. The new suggestion may be used to modify the
@@ -427,7 +427,7 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat
             return;
         }
 
-        // Use the right suggestions based on the current configuration.
+        // Use the correct algorithm based on the user's current configuration.
         if (currentUserConfig.getGeoDetectionEnabledBehavior()) {
             doGeolocationTimeZoneDetection(detectionReason);
         } else  {
@@ -598,15 +598,6 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat
             Slog.d(LOG_TAG, "handleConfigChanged()");
         }
 
-        clearGeolocationSuggestionIfNeeded();
-
-        for (ConfigurationChangeListener listener : mConfigChangeListeners) {
-            listener.onChange();
-        }
-    }
-
-    @GuardedBy("this")
-    private void clearGeolocationSuggestionIfNeeded() {
         // This method is called whenever the user changes or the config for any user changes. We
         // don't know what happened, so we capture the current user's config, check to see if we
         // need to clear state associated with a previous user, and rerun detection.
@@ -623,15 +614,17 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat
             // said it is ok to do so.
             mLatestGeoLocationSuggestion.set(null);
             mTimeZoneChangesLog.log(
-                    "clearGeolocationSuggestionIfNeeded: Cleared latest Geolocation suggestion.");
+                    "handleConfigChanged: Cleared latest Geolocation suggestion.");
         }
 
-        doAutoTimeZoneDetection(currentUserConfig, "clearGeolocationSuggestionIfNeeded()");
-    }
+        // The configuration change may have changed available suggestions or the way suggestions
+        // are used, so re-run detection.
+        doAutoTimeZoneDetection(currentUserConfig, "handleConfigChanged()");
 
-    @Override
-    public synchronized void addDumpable(@NonNull Dumpable dumpable) {
-        mDumpables.add(dumpable);
+        // Pass on the signal to sub-components.
+        for (ConfigurationChangeListener listener : mConfigChangeListeners) {
+            listener.onChange();
+        }
     }
 
     /**
@@ -671,10 +664,6 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat
         mTelephonySuggestionsBySlotIndex.dump(ipw);
         ipw.decreaseIndent(); // level 2
         ipw.decreaseIndent(); // level 1
-
-        for (Dumpable dumpable : mDumpables) {
-            dumpable.dump(ipw, args);
-        }
     }
 
     /**
diff --git a/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneManagerService.java b/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneManagerService.java
index c5c59cee17688..942df532d612a 100644
--- a/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneManagerService.java
+++ b/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneManagerService.java
@@ -168,7 +168,8 @@ public class LocationTimeZoneManagerService extends Binder {
     // According to the SystemService docs: All lifecycle methods are called from the system
     // server's main looper thread.
     void onSystemReady() {
-        mServiceConfigAccessor.addListener(this::handleServiceConfigurationChangedOnMainThread);
+        mServiceConfigAccessor.addLocationTimeZoneManagerConfigListener(
+                this::handleServiceConfigurationChangedOnMainThread);
     }
 
     private void handleServiceConfigurationChangedOnMainThread() {
diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/FakeTimeZoneDetectorStrategy.java b/services/tests/servicestests/src/com/android/server/timezonedetector/FakeTimeZoneDetectorStrategy.java
index 51f627ab415c8..11d5f3f2b9542 100644
--- a/services/tests/servicestests/src/com/android/server/timezonedetector/FakeTimeZoneDetectorStrategy.java
+++ b/services/tests/servicestests/src/com/android/server/timezonedetector/FakeTimeZoneDetectorStrategy.java
@@ -29,9 +29,6 @@ import android.app.timezonedetector.ManualTimeZoneSuggestion;
 import android.app.timezonedetector.TelephonyTimeZoneSuggestion;
 import android.util.IndentingPrintWriter;
 
-import java.util.ArrayList;
-import java.util.List;
-
 class FakeTimeZoneDetectorStrategy implements TimeZoneDetectorStrategy {
 
     private ConfigurationChangeListener mConfigurationChangeListener;
@@ -44,7 +41,6 @@ class FakeTimeZoneDetectorStrategy implements TimeZoneDetectorStrategy {
     private ManualTimeZoneSuggestion mLastManualSuggestion;
     private TelephonyTimeZoneSuggestion mLastTelephonySuggestion;
     private boolean mDumpCalled;
-    private final List<Dumpable> mDumpables = new ArrayList<>();
 
     @Override
     public void addConfigChangeListener(@NonNull ConfigurationChangeListener listener) {
@@ -122,11 +118,6 @@ class FakeTimeZoneDetectorStrategy implements TimeZoneDetectorStrategy {
         throw new UnsupportedOperationException();
     }
 
-    @Override
-    public void addDumpable(Dumpable dumpable) {
-        mDumpables.add(dumpable);
-    }
-
     @Override
     public void dump(IndentingPrintWriter pw, String[] args) {
         mDumpCalled = true;
@@ -159,8 +150,4 @@ class FakeTimeZoneDetectorStrategy implements TimeZoneDetectorStrategy {
     void verifyDumpCalled() {
         assertTrue(mDumpCalled);
     }
-
-    void verifyHasDumpable(Dumpable expected) {
-        assertTrue(mDumpables.contains(expected));
-    }
 }
diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorInternalImplTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorInternalImplTest.java
index 586462006ee28..45c5b6c35b5fa 100644
--- a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorInternalImplTest.java
+++ b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorInternalImplTest.java
@@ -77,16 +77,6 @@ public class TimeZoneDetectorInternalImplTest {
         mFakeTimeZoneDetectorStrategy.verifySuggestGeolocationTimeZoneCalled(timeZoneSuggestion);
     }
 
-    @Test
-    public void testAddDumpable() throws Exception {
-        Dumpable stubbedDumpable = mock(Dumpable.class);
-
-        mTimeZoneDetectorInternal.addDumpable(stubbedDumpable);
-        mTestHandler.assertTotalMessagesEnqueued(0);
-
-        mFakeTimeZoneDetectorStrategy.verifyHasDumpable(stubbedDumpable);
-    }
-
     @Test
     public void testAddConfigurationListener() throws Exception {
         boolean[] changeCalled = new boolean[2];
diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorServiceTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorServiceTest.java
index 773abf86e18ab..1e55c7bdc7754 100644
--- a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorServiceTest.java
@@ -60,12 +60,12 @@ public class TimeZoneDetectorServiceTest {
     private static final long ARBITRARY_ELAPSED_REALTIME_MILLIS = 1234L;
 
     private Context mMockContext;
-    private FakeTimeZoneDetectorStrategy mFakeTimeZoneDetectorStrategy;
 
     private TimeZoneDetectorService mTimeZoneDetectorService;
     private HandlerThread mHandlerThread;
     private TestHandler mTestHandler;
     private TestCallerIdentityInjector mTestCallerIdentityInjector;
+    private FakeTimeZoneDetectorStrategy mFakeTimeZoneDetectorStrategy;
 
 
     @Before
@@ -349,11 +349,15 @@ public class TimeZoneDetectorServiceTest {
         when(mMockContext.checkCallingOrSelfPermission(android.Manifest.permission.DUMP))
                 .thenReturn(PackageManager.PERMISSION_GRANTED);
 
+        Dumpable dumpable = mock(Dumpable.class);
+        mTimeZoneDetectorService.addDumpable(dumpable);
+
         PrintWriter pw = new PrintWriter(new StringWriter());
         mTimeZoneDetectorService.dump(null, pw, null);
 
         verify(mMockContext).checkCallingOrSelfPermission(eq(android.Manifest.permission.DUMP));
         mFakeTimeZoneDetectorStrategy.verifyDumpCalled();
+        verify(dumpable).dump(any(), any());
     }
 
     private static TimeZoneConfiguration createTimeZoneConfiguration(boolean autoDetectionEnabled) {
diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorStrategyImplTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorStrategyImplTest.java
index e2e875507c93a..e8af41d1cfded 100644
--- a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorStrategyImplTest.java
+++ b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorStrategyImplTest.java
@@ -47,18 +47,15 @@ import android.app.timezonedetector.ManualTimeZoneSuggestion;
 import android.app.timezonedetector.TelephonyTimeZoneSuggestion;
 import android.app.timezonedetector.TelephonyTimeZoneSuggestion.MatchType;
 import android.app.timezonedetector.TelephonyTimeZoneSuggestion.Quality;
-import android.util.IndentingPrintWriter;
 
 import com.android.server.timezonedetector.TimeZoneDetectorStrategyImpl.QualifiedTelephonyTimeZoneSuggestion;
 
 import org.junit.Before;
 import org.junit.Test;
 
-import java.io.StringWriter;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
-import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Function;
 
 /**
@@ -66,8 +63,8 @@ import java.util.function.Function;
  */
 public class TimeZoneDetectorStrategyImplTest {
 
-    /** A time zone used for initialization that does not occur elsewhere in tests. */
     private static final @UserIdInt int USER_ID = 9876;
+    /** A time zone used for initialization that does not occur elsewhere in tests. */
     private static final String ARBITRARY_TIME_ZONE_ID = "Etc/UTC";
     private static final int SLOT_INDEX1 = 10000;
     private static final int SLOT_INDEX2 = 20000;
@@ -790,7 +787,7 @@ public class TimeZoneDetectorStrategyImplTest {
                 .initializeTimeZoneSetting(ARBITRARY_TIME_ZONE_ID);
 
         script.simulateGeolocationTimeZoneSuggestion(suggestion)
-                .verifyTimeZoneChangedAndReset("Europe/London");
+                .verifyTimeZoneChangedAndReset(suggestion);
 
         // Assert internal service state.
         assertEquals(suggestion, mTimeZoneDetectorStrategy.getLatestGeolocationSuggestion());
@@ -815,7 +812,7 @@ public class TimeZoneDetectorStrategyImplTest {
                 .initializeTimeZoneSetting(ARBITRARY_TIME_ZONE_ID);
 
         script.simulateGeolocationTimeZoneSuggestion(londonOnlySuggestion)
-                .verifyTimeZoneChangedAndReset("Europe/London");
+                .verifyTimeZoneChangedAndReset(londonOnlySuggestion);
         assertEquals(londonOnlySuggestion,
                 mTimeZoneDetectorStrategy.getLatestGeolocationSuggestion());
 
@@ -826,7 +823,7 @@ public class TimeZoneDetectorStrategyImplTest {
                 mTimeZoneDetectorStrategy.getLatestGeolocationSuggestion());
 
         script.simulateGeolocationTimeZoneSuggestion(parisOnlySuggestion)
-                .verifyTimeZoneChangedAndReset("Europe/Paris");
+                .verifyTimeZoneChangedAndReset(parisOnlySuggestion);
         assertEquals(parisOnlySuggestion,
                 mTimeZoneDetectorStrategy.getLatestGeolocationSuggestion());
 
@@ -848,7 +845,7 @@ public class TimeZoneDetectorStrategyImplTest {
                 .initializeTimeZoneSetting(ARBITRARY_TIME_ZONE_ID);
 
         script.simulateGeolocationTimeZoneSuggestion(suggestion)
-                .verifyTimeZoneChangedAndReset("Europe/London");
+                .verifyTimeZoneChangedAndReset(suggestion);
 
         // Assert internal service state.
         assertEquals(suggestion, mTimeZoneDetectorStrategy.getLatestGeolocationSuggestion());
@@ -903,7 +900,7 @@ public class TimeZoneDetectorStrategyImplTest {
                 USER_ID, CONFIG_GEO_DETECTION_ENABLED, true /* expectedResult */)
                 .verifyTimeZoneNotChanged()
                 .simulateGeolocationTimeZoneSuggestion(geolocationSuggestion)
-                .verifyTimeZoneChangedAndReset(geolocationSuggestion.getZoneIds().get(0));
+                .verifyTimeZoneChangedAndReset(geolocationSuggestion);
 
         // Changing the detection to disable geo detection should cause the device tz setting to
         // change to the telephony suggestion.
@@ -914,28 +911,6 @@ public class TimeZoneDetectorStrategyImplTest {
         assertNull(mTimeZoneDetectorStrategy.getLatestGeolocationSuggestion());
     }
 
-    @Test
-    public void testAddDumpable() {
-        new Script()
-                .initializeConfig(CONFIG_INT_AUTO_DISABLED_GEO_DISABLED)
-                .initializeTimeZoneSetting(ARBITRARY_TIME_ZONE_ID);
-
-        AtomicBoolean dumpCalled = new AtomicBoolean(false);
-        class FakeDumpable implements Dumpable {
-            @Override
-            public void dump(IndentingPrintWriter pw, String[] args) {
-                dumpCalled.set(true);
-            }
-        }
-
-        mTimeZoneDetectorStrategy.addDumpable(new FakeDumpable());
-        IndentingPrintWriter ipw = new IndentingPrintWriter(new StringWriter());
-        String[] args = {"ArgOne", "ArgTwo"};
-        mTimeZoneDetectorStrategy.dump(ipw, args);
-
-        assertTrue(dumpCalled.get());
-    }
-
     @Test
     public void testGenerateMetricsState() {
         ConfigurationInternal expectedInternalConfig = CONFIG_INT_AUTO_DISABLED_GEO_DISABLED;
@@ -1254,6 +1229,14 @@ public class TimeZoneDetectorStrategyImplTest {
             return this;
         }
 
+        Script verifyTimeZoneChangedAndReset(GeolocationTimeZoneSuggestion suggestion) {
+            assertEquals("Only use this method with unambiguous geo suggestions",
+                    1, suggestion.getZoneIds().size());
+            mFakeEnvironment.assertTimeZoneChangedTo(suggestion.getZoneIds().get(0));
+            mFakeEnvironment.commitAllChanges();
+            return this;
+        }
+
         /**
          * Verifies that the configuration has been changed to the expected value.
          */