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

Commit 76df467c authored by Neil Fuller's avatar Neil Fuller
Browse files

Remove geo suggestion clearing 

Remove strategy clearing / ignoring geo suggestions within
TimeZoneDetectorStrategyImpl when geo detection is disabled / the user
is changed.

The LocationTimeZoneManager's ControllerImpl withdraws suggestions in
these situations anyway. Removing the duplicated functionality from the
strategy helps avoid races (i.e. if the TimeZoneDetectorStrategy and
LocationTimeZoneManager receive the same "something has changed" signal
in an ill-defined order it could lead to suggestions being "missed").
This should open up opportunities for future droidfood testing
(i.e. the ability to run geo detection "in the background" and record /
report them in metrics during QA, bug 200279201).

Test: atest services/tests/servicestests/src/com/android/server/timezonedetector/
Test: atest cts/hostsidetests/time/host/src/android/time/cts/host/
Bug: 200279201
Bug: 197624972
Change-Id: I2e5ea8482216ef204cf2e7ad412eeb0f501e36b9
parent bded1d2f

services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java

+17 −29
Original line number Diff line number Diff line
@@ -289,9 +289,11 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat 
        }

        Objects.requireNonNull(suggestion);



        if (currentUserConfig.getGeoDetectionEnabledBehavior()) {

            // Only store a geolocation suggestion if geolocation detection is currently enabled.

            // See also handleConfigChanged(), which can clear mLatestGeoLocationSuggestion.

        // Geolocation suggestions may be stored but not used during time zone detection if the

        // configuration doesn't have geo time zone detection enabled. The caller is expected to

        // withdraw a previous suggestion (i.e. submit an "uncertain" suggestion, when geo time zone

        // detection is disabled.



        // 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);
@@ -301,7 +303,6 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat 
        String reason = "New geolocation time zone suggested. suggestion=" + suggestion;

        doAutoTimeZoneDetection(currentUserConfig, reason);

    }

    }



    @Override

    public synchronized boolean suggestManualTimeZone(
@@ -365,11 +366,9 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat 


        // Now perform auto time zone detection. The new suggestion may be used to modify the time

        // zone setting.

        if (!currentUserConfig.getGeoDetectionEnabledBehavior()) {

        String reason = "New telephony time zone suggested. suggestion=" + suggestion;

        doAutoTimeZoneDetection(currentUserConfig, reason);

    }

    }



    @Override

    @NonNull
@@ -427,7 +426,8 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat 
            return;

        }



        // Use the correct algorithm based on the user's current configuration.

        // Use the correct algorithm based on the user's current configuration. If it changes, then

        // detection will be re-run.

        if (currentUserConfig.getGeoDetectionEnabledBehavior()) {

            doGeolocationTimeZoneDetection(detectionReason);

        } else  {
@@ -605,18 +605,6 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat 
        ConfigurationInternal currentUserConfig =

                mEnvironment.getConfigurationInternal(currentUserId);



        GeolocationTimeZoneSuggestion latestGeoLocationSuggestion =

                mLatestGeoLocationSuggestion.get();

        if (latestGeoLocationSuggestion != null

                && !currentUserConfig.getGeoDetectionEnabledBehavior()) {

            // The current user's config has geodetection disabled, so clear the latest suggestion.

            // This is done to ensure we only ever keep a geolocation suggestion if the user has

            // said it is ok to do so.

            mLatestGeoLocationSuggestion.set(null);

            mTimeZoneChangesLog.log(

                    "handleConfigChanged: Cleared latest Geolocation suggestion.");

        }



        // The configuration change may have changed available suggestions or the way suggestions

        // are used, so re-run detection.

        doAutoTimeZoneDetection(currentUserConfig, "handleConfigChanged()");

services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorStrategyImplTest.java

+8 −40
Original line number Diff line number Diff line
@@ -835,29 +835,6 @@ public class TimeZoneDetectorStrategyImplTest { 
                mTimeZoneDetectorStrategy.getLatestGeolocationSuggestion());

    }



    @Test

    public void testGeoSuggestion_togglingGeoDetectionClearsLastSuggestion() {

        GeolocationTimeZoneSuggestion suggestion =

                createCertainGeolocationSuggestion("Europe/London");



        Script script = new Script()

                .initializeConfig(CONFIG_INT_AUTO_ENABLED_GEO_ENABLED)

                .initializeTimeZoneSetting(ARBITRARY_TIME_ZONE_ID);



        script.simulateGeolocationTimeZoneSuggestion(suggestion)

                .verifyTimeZoneChangedAndReset(suggestion);



        // Assert internal service state.

        assertEquals(suggestion, mTimeZoneDetectorStrategy.getLatestGeolocationSuggestion());



        // Turn off geo detection and verify the latest suggestion is cleared.

        script.simulateUpdateConfiguration(USER_ID, CONFIG_GEO_DETECTION_DISABLED, true)

                .verifyConfigurationChangedAndReset(CONFIG_INT_AUTO_ENABLED_GEO_DISABLED);



        // Assert internal service state.

        assertNull(mTimeZoneDetectorStrategy.getLatestGeolocationSuggestion());

    }



    /**

     * Confirms that changing the geolocation time zone detection enabled setting has the expected

     * behavior, i.e. immediately recompute the detected time zone using different signals.
@@ -878,13 +855,12 @@ public class TimeZoneDetectorStrategyImplTest { 
        script.simulateGeolocationTimeZoneSuggestion(geolocationSuggestion)

                .verifyTimeZoneNotChanged();



        // Geolocation suggestions are only stored when geolocation detection is enabled.

        assertNull(mTimeZoneDetectorStrategy.getLatestGeolocationSuggestion());

        assertEquals(geolocationSuggestion,

                mTimeZoneDetectorStrategy.getLatestGeolocationSuggestion());



        script.simulateTelephonyTimeZoneSuggestion(telephonySuggestion)

                .verifyTimeZoneNotChanged();



        // Telephony suggestions are always stored.

        assertEquals(telephonySuggestion,

                mTimeZoneDetectorStrategy.getLatestTelephonySuggestion(SLOT_INDEX1).suggestion);
@@ -894,12 +870,10 @@ public class TimeZoneDetectorStrategyImplTest { 
        script.simulateUpdateConfiguration(USER_ID, CONFIG_AUTO_ENABLED, true /* expectedResult */)

                .verifyTimeZoneChangedAndReset(telephonySuggestion);



        // Changing the detection to enable geo detection won't cause the device tz setting to

        // change because the geo suggestion is empty.

        // Changing the detection to enable geo detection will cause the device tz setting to

        // change to use the latest geolocation suggestion.

        script.simulateUpdateConfiguration(

                USER_ID, CONFIG_GEO_DETECTION_ENABLED, true /* expectedResult */)

                .verifyTimeZoneNotChanged()

                .simulateGeolocationTimeZoneSuggestion(geolocationSuggestion)

                .verifyTimeZoneChangedAndReset(geolocationSuggestion);



        // Changing the detection to disable geo detection should cause the device tz setting to
@@ -908,7 +882,8 @@ public class TimeZoneDetectorStrategyImplTest { 
                USER_ID, CONFIG_GEO_DETECTION_DISABLED, true /* expectedResult */)

                .verifyTimeZoneChangedAndReset(telephonySuggestion);



        assertNull(mTimeZoneDetectorStrategy.getLatestGeolocationSuggestion());

        assertEquals(geolocationSuggestion,

                mTimeZoneDetectorStrategy.getLatestGeolocationSuggestion());

    }



    @Test
@@ -947,7 +922,7 @@ public class TimeZoneDetectorStrategyImplTest { 
                .verifyTimeZoneNotChanged();



        assertMetricsState(expectedInternalConfig, expectedDeviceTimeZoneId,

                manualSuggestion, telephonySuggestion, null /* expectedGeoSuggestion */,

                manualSuggestion, telephonySuggestion, geolocationTimeZoneSuggestion,

                MetricsTimeZoneDetectorState.DETECTION_MODE_MANUAL);



        // Update the config and confirm that the config metrics state updates also.
@@ -957,16 +932,9 @@ public class TimeZoneDetectorStrategyImplTest { 
                .setAutoDetectionEnabled(true)

                .setGeoDetectionEnabled(true)

                .build();

        script.simulateUpdateConfiguration(USER_ID, configUpdate, true /* expectedResult */)

                .verifyConfigurationChangedAndReset(expectedInternalConfig)

                .verifyTimeZoneNotChanged();

        assertMetricsState(expectedInternalConfig, expectedDeviceTimeZoneId,

                manualSuggestion, telephonySuggestion, null  /* expectedGeoSuggestion */,

                MetricsTimeZoneDetectorState.DETECTION_MODE_GEO);



        // Now simulate a geo suggestion and confirm it is used and reported in the metrics too.

        expectedDeviceTimeZoneId = geolocationTimeZoneSuggestion.getZoneIds().get(0);

        script.simulateGeolocationTimeZoneSuggestion(geolocationTimeZoneSuggestion)

        script.simulateUpdateConfiguration(USER_ID, configUpdate, true /* expectedResult */)

                .verifyTimeZoneChangedAndReset(expectedDeviceTimeZoneId);

        assertMetricsState(expectedInternalConfig, expectedDeviceTimeZoneId,

                manualSuggestion, telephonySuggestion, geolocationTimeZoneSuggestion,