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

Commit 53196bbe authored by Neil Fuller's avatar Neil Fuller
Browse files

Withdraw suggestion when current user changes 

This commit changes ControllerImpl so that it replaces previous
"certain" suggestions when the user changes. Before this commit, the
previous suggestion would be left until the providers made a new
suggestion. Generally, the ControllerImpl tries to avoid carrying state
between users so replacing the suggestion is a more consistent approach.

This commit also adds an assertion to an existing test. Before this
change, the test fails. It passes after.

It also adds a test for destroy(), which is another path where we expect
previous suggestions to be overridden, is potentially influenced by the
ControllerImpl change, and where test coverage was previously lacking.

Bug: 193410893
Test: atest com.android.server.timezonedetector.location
Change-Id: I9b1b61bdf0d7c269bd0e2252f8ba0663464eb9b8
parent 36e2c34e

services/core/java/com/android/server/timezonedetector/location/ControllerImpl.java

+10 −21
Original line number Original line Diff line number Diff line
@@ -166,12 +166,6 @@ class ControllerImpl extends LocationTimeZoneProviderController { 
            stopProviders();

            stopProviders();

            mPrimaryProvider.destroy();

            mPrimaryProvider.destroy();

            mSecondaryProvider.destroy();

            mSecondaryProvider.destroy();



            // If the controller has made a "certain" suggestion, it should make an uncertain

            // suggestion to cancel it.

            if (mLastSuggestion != null && mLastSuggestion.getZoneIds() != null) {

                makeSuggestion(createUncertainSuggestion("Controller is destroyed"));

            }

        }

        }

    }

    }
@@ -182,6 +176,16 @@ class ControllerImpl extends LocationTimeZoneProviderController { 




        // By definition, if both providers are stopped, the controller is uncertain.

        // By definition, if both providers are stopped, the controller is uncertain.

        cancelUncertaintyTimeout();

        cancelUncertaintyTimeout();



        // If a previous "certain" suggestion has been made, then a new "uncertain"

        // suggestion must now be made to indicate the controller {does not / no longer has}

        // an opinion and will not be sending further updates (until at least the providers are

        // re-started).

        if (mLastSuggestion != null && mLastSuggestion.getZoneIds() != null) {

            GeolocationTimeZoneSuggestion suggestion = createUncertainSuggestion(

                    "Providers are stopping");

            makeSuggestion(suggestion);

        }

    }

    }





    @GuardedBy("mSharedLock")

    @GuardedBy("mSharedLock")
@@ -275,21 +279,6 @@ class ControllerImpl extends LocationTimeZoneProviderController { 
            }

            }

        } else {

        } else {

            stopProviders();

            stopProviders();



            // There can be an uncertainty timeout set if the controller most recently received

            // an uncertain event. This is a no-op if there isn't a timeout set.

            cancelUncertaintyTimeout();



            // If a previous "certain" suggestion has been made, then a new "uncertain"

            // suggestion must now be made to indicate the controller {does not / no longer has}

            // an opinion and will not be sending further updates (until at least the config

            // changes again and providers are re-started).

            if (mLastSuggestion != null && mLastSuggestion.getZoneIds() != null) {

                GeolocationTimeZoneSuggestion suggestion = createUncertainSuggestion(

                        "Provider is stopped:"

                                + " primary=" + mPrimaryProvider.getCurrentState());

                makeSuggestion(suggestion);

            }

        }

        }

    }

    }

services/tests/servicestests/src/com/android/server/timezonedetector/location/ControllerImplTest.java

+47 −2
Original line number Original line Diff line number Diff line
@@ -15,6 +15,7 @@ 
 */

 */

package com.android.server.timezonedetector.location;

package com.android.server.timezonedetector.location;





import static com.android.server.timezonedetector.location.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_DESTROYED;

import static com.android.server.timezonedetector.location.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_PERM_FAILED;

import static com.android.server.timezonedetector.location.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_PERM_FAILED;

import static com.android.server.timezonedetector.location.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_STARTED_CERTAIN;

import static com.android.server.timezonedetector.location.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_STARTED_CERTAIN;

import static com.android.server.timezonedetector.location.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_STARTED_INITIALIZING;

import static com.android.server.timezonedetector.location.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_STARTED_INITIALIZING;
@@ -728,6 +729,9 @@ public class ControllerImplTest { 
        // Simulate the user change (but geo detection still enabled).

        // Simulate the user change (but geo detection still enabled).

        testEnvironment.simulateConfigChange(USER2_CONFIG_GEO_DETECTION_ENABLED);

        testEnvironment.simulateConfigChange(USER2_CONFIG_GEO_DETECTION_ENABLED);





        // Confirm that the previous suggestion was overridden.

        mTestCallback.assertUncertainSuggestionMadeAndCommit();



        // We expect the provider to end up in PROVIDER_STATE_STARTED_INITIALIZING, but it should

        // We expect the provider to end up in PROVIDER_STATE_STARTED_INITIALIZING, but it should

        // have been stopped when the user changed.

        // have been stopped when the user changed.

        int[] expectedStateTransitions =

        int[] expectedStateTransitions =
@@ -1068,6 +1072,48 @@ public class ControllerImplTest { 
        }

        }

    }

    }





    @Test

    public void destroy() {

        ControllerImpl controllerImpl = new ControllerImpl(mTestThreadingDomain,

                mTestPrimaryLocationTimeZoneProvider, mTestSecondaryLocationTimeZoneProvider);

        TestEnvironment testEnvironment = new TestEnvironment(

                mTestThreadingDomain, controllerImpl, USER1_CONFIG_GEO_DETECTION_ENABLED);



        // Initialize and check initial state.

        controllerImpl.initialize(testEnvironment, mTestCallback);



        mTestPrimaryLocationTimeZoneProvider.assertStateEnumAndConfigAndCommit(

                PROVIDER_STATE_STARTED_INITIALIZING, USER1_CONFIG_GEO_DETECTION_ENABLED);

        mTestSecondaryLocationTimeZoneProvider.assertIsStoppedAndCommit();

        mTestCallback.assertNoSuggestionMade();

        assertFalse(controllerImpl.isUncertaintyTimeoutSet());



        // Simulate the primary provider suggesting a time zone.

        mTestPrimaryLocationTimeZoneProvider.simulateTimeZoneProviderEvent(

                USER1_SUCCESS_LOCATION_TIME_ZONE_EVENT1);



        // Receiving a "success" provider event should cause a suggestion to be made synchronously,

        // and also clear the scheduled uncertainty suggestion.

        mTestPrimaryLocationTimeZoneProvider.assertStateEnumAndConfigAndCommit(

                PROVIDER_STATE_STARTED_CERTAIN, USER1_CONFIG_GEO_DETECTION_ENABLED);

        mTestSecondaryLocationTimeZoneProvider.assertIsStoppedAndCommit();

        mTestCallback.assertSuggestionMadeAndCommit(

                USER1_SUCCESS_LOCATION_TIME_ZONE_EVENT1.getSuggestion().getTimeZoneIds());

        assertFalse(controllerImpl.isUncertaintyTimeoutSet());



        // Trigger destroy().

        controllerImpl.destroy();



        // Confirm that the previous suggestion was overridden.

        mTestCallback.assertUncertainSuggestionMadeAndCommit();



        mTestPrimaryLocationTimeZoneProvider.assertStateChangesAndCommit(

                PROVIDER_STATE_STOPPED, PROVIDER_STATE_DESTROYED);

        mTestSecondaryLocationTimeZoneProvider.assertStateChangesAndCommit(

                PROVIDER_STATE_DESTROYED);

        assertFalse(controllerImpl.isUncertaintyTimeoutSet());

    }



    private static void assertUncertaintyTimeoutSet(

    private static void assertUncertaintyTimeoutSet(

            LocationTimeZoneProviderController.Environment environment,

            LocationTimeZoneProviderController.Environment environment,

            LocationTimeZoneProviderController controller) {

            LocationTimeZoneProviderController controller) {
@@ -1175,7 +1221,6 @@ public class ControllerImplTest { 
        private final TestState<ProviderState> mTestProviderState = new TestState<>();

        private final TestState<ProviderState> mTestProviderState = new TestState<>();

        private boolean mFailDuringInitialization;

        private boolean mFailDuringInitialization;

        private boolean mInitialized;

        private boolean mInitialized;

        private boolean mDestroyed;





        /**

        /**

         * Creates the instance.

         * Creates the instance.
@@ -1200,7 +1245,7 @@ public class ControllerImplTest { 




        @Override

        @Override

        void onDestroy() {

        void onDestroy() {

            mDestroyed = true;

            // No behavior needed.

        }

        }





        @Override

        @Override