Merge "Remove geo suggestion clearing"

This commit is contained in:
Neil Fuller
2021-11-17 15:03:06 +00:00
committed by Android (Google) Code Review
2 changed files with 24 additions and 68 deletions

View File

@@ -289,18 +289,19 @@ 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.
// 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);
// 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.
// Now perform auto time zone detection. The new suggestion may be used to modify the
// time zone setting.
String reason = "New geolocation time zone suggested. suggestion=" + suggestion;
doAutoTimeZoneDetection(currentUserConfig, reason);
}
// 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
// time zone setting.
String reason = "New geolocation time zone suggested. suggestion=" + suggestion;
doAutoTimeZoneDetection(currentUserConfig, reason);
}
@Override
@@ -365,10 +366,8 @@ 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);
}
String reason = "New telephony time zone suggested. suggestion=" + suggestion;
doAutoTimeZoneDetection(currentUserConfig, reason);
}
@Override
@@ -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()");

View File

@@ -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,