Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 35f7d0a3 authored by Neil Fuller's avatar Neil Fuller
Browse files

Use ServiceConfigAccessor directly 

Architectural change: Make ServiceConfigAccessor a direct dependency of
TimeZoneDetectorStrategyImpl.

Previously, TimeZoneDetectorInternalImpl and TimeZoneDetectorService
used ServiceConfigAccessor directly for reading / writing config, and
TimeZoneDetectorStrategyImpl read it via it's "Environment"
facade and relied on async updates to follow along.

Upcoming changes that are likely to add more information to the cached
status information that downstream components will be interested in
(i.e. will want to listen to). This extra information will be held by
TimeZoneDetectorStrategyImpl so it will be easier if it is (only)
TimeZoneDetectorStrategyImpl that manages the ServiceConfigAccessor of
the three.

The dependency on ServiceConfigAccessor is removed from the classes
higher in the stack (TimeZoneDetectorService /
TimeZoneDetectorInternalImpl), which now call through to the Strategy.
The ServiceConfigAccessor functionality has been removed from the
Environment, and TimeZoneDetectorStrategyImpl uses ServiceConfigAccessor
directly.

Now that the TimeZoneDetectorStrategyImpl is in the direct config read /
update flow for binder calls, some care has been taken to ensure the
cached ConfigurationInternal is kept up to date. Previously, the
TimeZoneDetectorStrategyImpl updates happened asynchronously.  There's
some complexity around caching / eventual consistency here that might be
detectable but should not have any impact on behavior.

Notes for people following along: We cannot assume that the only thing
affecting config / status happens via TimeZoneDetectorStrategyImpl:
Android's settings and device config are mutatable via all sorts of
routes (e.g. command line, backup&restore, random partner apps). This is
not likely to change. Therefore, the TimeZoneDetectorStrategyImpl still
listens for ServiceConfigAccessor updates, but will (now) only forward
them on if something changes Vs the last one it notified about.

This commit changes the time zone strategy code. A follow-up will make
an equivalent change for the time code at a later time for consistency.

Bug: 253015306
Test: atest services/tests/servicestests/src/com/android/server/timedetector/ services/tests/servicestests/src/com/android/server/timezonedetector/
Test: atest ./tests/tests/time/src/android/app/time/cts/
Change-Id: Ie3fe60a624be8a5bb3e161a3e88f0a257cd96c3d
parent c91eb23d

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

+4 −4
Original line number Diff line number Diff line
@@ -28,7 +28,7 @@ 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.ConfigurationChangeListener;

import com.android.server.timezonedetector.StateChangeListener;



import java.io.PrintWriter;

import java.util.Objects;
@@ -60,10 +60,10 @@ final class EnvironmentImpl implements TimeDetectorStrategyImpl.Environment { 


    @Override

    public void setConfigurationInternalChangeListener(

            @NonNull ConfigurationChangeListener listener) {

        ConfigurationChangeListener configurationChangeListener =

            @NonNull StateChangeListener listener) {

        StateChangeListener stateChangeListener =

                () -> mHandler.post(listener::onChange);

        mServiceConfigAccessor.addConfigurationInternalChangeListener(configurationChangeListener);

        mServiceConfigAccessor.addConfigurationInternalChangeListener(stateChangeListener);

    }



    @Override

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

+4 −5
Original line number Diff line number Diff line
@@ -25,8 +25,8 @@ import android.provider.DeviceConfig; 
import android.util.ArrayMap;



import com.android.internal.annotations.GuardedBy;

import com.android.server.timezonedetector.ConfigurationChangeListener;

import com.android.server.timezonedetector.ServiceConfigAccessor;

import com.android.server.timezonedetector.StateChangeListener;



import java.lang.annotation.ElementType;

import java.lang.annotation.Retention;
@@ -185,8 +185,7 @@ public final class ServerFlags { 
     * ensure O(1) lookup performance when working out whether a listener should trigger.

     */

    @GuardedBy("mListeners")

    private final ArrayMap<ConfigurationChangeListener, HashSet<String>> mListeners =

            new ArrayMap<>();

    private final ArrayMap<StateChangeListener, HashSet<String>> mListeners = new ArrayMap<>();



    private static final Object SLOCK = new Object();
@@ -213,7 +212,7 @@ public final class ServerFlags { 


    private void handlePropertiesChanged(@NonNull DeviceConfig.Properties properties) {

        synchronized (mListeners) {

            for (Map.Entry<ConfigurationChangeListener, HashSet<String>> listenerEntry

            for (Map.Entry<StateChangeListener, HashSet<String>> listenerEntry

                    : mListeners.entrySet()) {

                // It's unclear which set of the following two Sets is going to be larger in the

                // average case: monitoredKeys will be a subset of the set of possible keys, but
@@ -249,7 +248,7 @@ public final class ServerFlags { 
     * <p>Note: Only for use by long-lived objects like other singletons. There is deliberately no

     * associated remove method.

     */

    public void addListener(@NonNull ConfigurationChangeListener listener,

    public void addListener(@NonNull StateChangeListener listener,

            @NonNull Set<String> keys) {

        Objects.requireNonNull(listener);

        Objects.requireNonNull(keys);

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

+5 −5
Original line number Diff line number Diff line
@@ -19,7 +19,7 @@ import android.annotation.NonNull; 
import android.annotation.UserIdInt;

import android.app.time.TimeConfiguration;



import com.android.server.timezonedetector.ConfigurationChangeListener;

import com.android.server.timezonedetector.StateChangeListener;



/**

 * An interface that provides access to service configuration for time detection. This hides
@@ -33,18 +33,18 @@ public interface ServiceConfigAccessor { 
     * Adds a listener that will be invoked when {@link ConfigurationInternal} may have changed.

     * The listener is invoked on the main thread.

     */

    void addConfigurationInternalChangeListener(@NonNull ConfigurationChangeListener listener);

    void addConfigurationInternalChangeListener(@NonNull StateChangeListener listener);



    /**

     * Removes a listener previously added via {@link

     * #addConfigurationInternalChangeListener(ConfigurationChangeListener)}.

     * #addConfigurationInternalChangeListener(StateChangeListener)}.

     */

    void removeConfigurationInternalChangeListener(@NonNull ConfigurationChangeListener listener);

    void removeConfigurationInternalChangeListener(@NonNull StateChangeListener listener);



    /**

     * Returns a snapshot of the {@link ConfigurationInternal} for the current user. This is only a

     * snapshot so callers must use {@link

     * #addConfigurationInternalChangeListener(ConfigurationChangeListener)} to be notified when it

     * #addConfigurationInternalChangeListener(StateChangeListener)} to be notified when it

     * changes.

     */

    @NonNull

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

+6 −6
Original line number Diff line number Diff line
@@ -49,7 +49,7 @@ import com.android.internal.annotations.GuardedBy; 
import com.android.internal.util.Preconditions;

import com.android.server.LocalServices;

import com.android.server.timedetector.TimeDetectorStrategy.Origin;

import com.android.server.timezonedetector.ConfigurationChangeListener;

import com.android.server.timezonedetector.StateChangeListener;



import java.time.Instant;

import java.util.ArrayList;
@@ -104,8 +104,8 @@ final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { 
    @NonNull private final ServerFlagsOriginPrioritiesSupplier mServerFlagsOriginPrioritiesSupplier;



    @GuardedBy("this")

    @NonNull private final List<ConfigurationChangeListener> mConfigurationInternalListeners =

            new ArrayList<>();

    @NonNull

    private final List<StateChangeListener> mConfigurationInternalListeners = new ArrayList<>();



    /**

     * If a newly calculated system clock time and the current system clock time differs by this or
@@ -167,20 +167,20 @@ final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { 
    }



    private synchronized void handleConfigurationInternalChangeOnMainThread() {

        for (ConfigurationChangeListener changeListener : mConfigurationInternalListeners) {

        for (StateChangeListener changeListener : mConfigurationInternalListeners) {

            changeListener.onChange();

        }

    }



    @Override

    public synchronized void addConfigurationInternalChangeListener(

            @NonNull ConfigurationChangeListener listener) {

            @NonNull StateChangeListener listener) {

        mConfigurationInternalListeners.add(Objects.requireNonNull(listener));

    }



    @Override

    public synchronized void removeConfigurationInternalChangeListener(

            @NonNull ConfigurationChangeListener listener) {

            @NonNull StateChangeListener listener) {

        mConfigurationInternalListeners.remove(Objects.requireNonNull(listener));

    }

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

+4 −4
Original line number Diff line number Diff line
@@ -44,8 +44,8 @@ import com.android.internal.annotations.VisibleForTesting; 
import com.android.server.SystemClockTime;

import com.android.server.SystemClockTime.TimeConfidence;

import com.android.server.timezonedetector.ArrayMapWithHistory;

import com.android.server.timezonedetector.ConfigurationChangeListener;

import com.android.server.timezonedetector.ReferenceWithHistory;

import com.android.server.timezonedetector.StateChangeListener;



import java.io.PrintWriter;

import java.time.Duration;
@@ -136,11 +136,11 @@ public final class TimeDetectorStrategyImpl implements TimeDetectorStrategy { 
    public interface Environment {



        /**

         * Sets a {@link ConfigurationChangeListener} that will be invoked when there are any

         * changes that could affect the content of {@link ConfigurationInternal}.

         * 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 ConfigurationChangeListener listener);

        void setConfigurationInternalChangeListener(@NonNull StateChangeListener listener);



        /** Returns the {@link ConfigurationInternal} for the current user. */

        @NonNull ConfigurationInternal getCurrentUserConfigurationInternal();