From caae804b086d7d7a8e1da9195efd246df9ea1644 Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Fri, 9 Oct 2020 16:27:15 +0100 Subject: [PATCH] Fix getConfigureGeoDetectionEnabledCapability() Fix TimeZoneCapabilities.getConfigureGeoDetectionEnabledCapability() on devices without geo detection enabled. It was previously only checking whether auto detection was supported, which is incorrect and was leading to the wrong capability being returned. The rest is plumbing and tests. Bug: 152746236 Test: atest services/tests/servicestests/src/com/android/server/timezonedetector/ Change-Id: Ic3a6cab69dcd32dbe99d8cd1236fe0150faa4632 --- .../ConfigurationInternal.java | 47 ++++++++++--- .../TimeZoneDetectorCallbackImpl.java | 16 +++-- .../server/location/timezone/TestSupport.java | 1 + .../ConfigurationInternalTest.java | 66 +++++++++++++++++++ .../TimeZoneDetectorServiceTest.java | 1 + .../TimeZoneDetectorStrategyImplTest.java | 63 ++++++++++++++---- 6 files changed, 168 insertions(+), 26 deletions(-) diff --git a/services/core/java/com/android/server/timezonedetector/ConfigurationInternal.java b/services/core/java/com/android/server/timezonedetector/ConfigurationInternal.java index 08eaa29253085..1f73977444f81 100644 --- a/services/core/java/com/android/server/timezonedetector/ConfigurationInternal.java +++ b/services/core/java/com/android/server/timezonedetector/ConfigurationInternal.java @@ -28,6 +28,8 @@ import android.app.time.TimeZoneCapabilitiesAndConfig; import android.app.time.TimeZoneConfiguration; import android.os.UserHandle; +import com.android.internal.util.Preconditions; + import java.util.Objects; /** @@ -40,6 +42,7 @@ public final class ConfigurationInternal { private final @UserIdInt int mUserId; private final boolean mUserConfigAllowed; private final boolean mAutoDetectionSupported; + private final boolean mGeoDetectionSupported; private final boolean mAutoDetectionEnabled; private final boolean mLocationEnabled; private final boolean mGeoDetectionEnabled; @@ -48,9 +51,13 @@ public final class ConfigurationInternal { mUserId = builder.mUserId; mUserConfigAllowed = builder.mUserConfigAllowed; mAutoDetectionSupported = builder.mAutoDetectionSupported; + mGeoDetectionSupported = builder.mGeoDetectionSupported; mAutoDetectionEnabled = builder.mAutoDetectionEnabled; mLocationEnabled = builder.mLocationEnabled; mGeoDetectionEnabled = builder.mGeoDetectionEnabled; + // if mGeoDetectionSupported then mAutoDetectionSupported, i.e. mGeoDetectionSupported + // cannot be true if mAutoDetectionSupported == false + Preconditions.checkState(mAutoDetectionSupported || !mGeoDetectionSupported); } /** Returns the ID of the user this configuration is associated with. */ @@ -69,11 +76,16 @@ public final class ConfigurationInternal { return mUserConfigAllowed; } - /** Returns true if the device supports some form of auto time zone detection. */ + /** Returns true if the device supports any form of auto time zone detection. */ public boolean isAutoDetectionSupported() { return mAutoDetectionSupported; } + /** Returns true if the device supports geolocation time zone detection. */ + public boolean isGeoDetectionSupported() { + return mGeoDetectionSupported; + } + /** Returns the value of the auto time zone detection enabled setting. */ public boolean getAutoDetectionEnabledSetting() { return mAutoDetectionEnabled; @@ -101,10 +113,10 @@ public final class ConfigurationInternal { * distinct from the raw setting value. */ public boolean getGeoDetectionEnabledBehavior() { - if (getAutoDetectionEnabledBehavior()) { - return mLocationEnabled && mGeoDetectionEnabled; - } - return false; + return getAutoDetectionEnabledBehavior() + && isGeoDetectionSupported() + && isLocationEnabled() + && getGeoDetectionEnabledSetting(); } /** Creates a {@link TimeZoneCapabilitiesAndConfig} object using the configuration values. */ @@ -121,10 +133,10 @@ public final class ConfigurationInternal { // Automatic time zone detection is only supported on devices if there is a telephony // network available or geolocation time zone detection is possible. - boolean deviceHasTimeZoneDetection = isAutoDetectionSupported(); + boolean deviceHasAutoTimeZoneDetection = isAutoDetectionSupported(); final int configureAutoDetectionEnabledCapability; - if (!deviceHasTimeZoneDetection) { + if (!deviceHasAutoTimeZoneDetection) { configureAutoDetectionEnabledCapability = CAPABILITY_NOT_SUPPORTED; } else if (!allowConfigDateTime) { configureAutoDetectionEnabledCapability = CAPABILITY_NOT_ALLOWED; @@ -133,8 +145,9 @@ public final class ConfigurationInternal { } builder.setConfigureAutoDetectionEnabledCapability(configureAutoDetectionEnabledCapability); + boolean deviceHasLocationTimeZoneDetection = isGeoDetectionSupported(); final int configureGeolocationDetectionEnabledCapability; - if (!deviceHasTimeZoneDetection) { + if (!deviceHasLocationTimeZoneDetection) { configureGeolocationDetectionEnabledCapability = CAPABILITY_NOT_SUPPORTED; } else if (!allowConfigDateTime) { configureGeolocationDetectionEnabledCapability = CAPABILITY_NOT_ALLOWED; @@ -199,6 +212,7 @@ public final class ConfigurationInternal { return mUserId == that.mUserId && mUserConfigAllowed == that.mUserConfigAllowed && mAutoDetectionSupported == that.mAutoDetectionSupported + && mGeoDetectionSupported == that.mGeoDetectionSupported && mAutoDetectionEnabled == that.mAutoDetectionEnabled && mLocationEnabled == that.mLocationEnabled && mGeoDetectionEnabled == that.mGeoDetectionEnabled; @@ -207,7 +221,8 @@ public final class ConfigurationInternal { @Override public int hashCode() { return Objects.hash(mUserId, mUserConfigAllowed, mAutoDetectionSupported, - mAutoDetectionEnabled, mLocationEnabled, mGeoDetectionEnabled); + mGeoDetectionSupported, mAutoDetectionEnabled, mLocationEnabled, + mGeoDetectionEnabled); } @Override @@ -216,6 +231,7 @@ public final class ConfigurationInternal { + "mUserId=" + mUserId + ", mUserConfigAllowed=" + mUserConfigAllowed + ", mAutoDetectionSupported=" + mAutoDetectionSupported + + ", mGeoDetectionSupported=" + mGeoDetectionSupported + ", mAutoDetectionEnabled=" + mAutoDetectionEnabled + ", mLocationEnabled=" + mLocationEnabled + ", mGeoDetectionEnabled=" + mGeoDetectionEnabled @@ -228,8 +244,10 @@ public final class ConfigurationInternal { public static class Builder { private final @UserIdInt int mUserId; + private boolean mUserConfigAllowed; private boolean mAutoDetectionSupported; + private boolean mGeoDetectionSupported; private boolean mAutoDetectionEnabled; private boolean mLocationEnabled; private boolean mGeoDetectionEnabled; @@ -248,6 +266,7 @@ public final class ConfigurationInternal { this.mUserId = toCopy.mUserId; this.mUserConfigAllowed = toCopy.mUserConfigAllowed; this.mAutoDetectionSupported = toCopy.mAutoDetectionSupported; + this.mGeoDetectionSupported = toCopy.mGeoDetectionSupported; this.mAutoDetectionEnabled = toCopy.mAutoDetectionEnabled; this.mLocationEnabled = toCopy.mLocationEnabled; this.mGeoDetectionEnabled = toCopy.mGeoDetectionEnabled; @@ -262,13 +281,21 @@ public final class ConfigurationInternal { } /** - * Sets whether automatic time zone detection is supported on this device. + * Sets whether any form of automatic time zone detection is supported on this device. */ public Builder setAutoDetectionSupported(boolean supported) { mAutoDetectionSupported = supported; return this; } + /** + * Sets whether geolocation time zone detection is supported on this device. + */ + public Builder setGeoDetectionSupported(boolean supported) { + mGeoDetectionSupported = supported; + return this; + } + /** * Sets the value of the automatic time zone detection enabled setting for this device. */ diff --git a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorCallbackImpl.java b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorCallbackImpl.java index 964dbecf705c3..6ee0af7d61e9d 100644 --- a/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorCallbackImpl.java +++ b/services/core/java/com/android/server/timezonedetector/TimeZoneDetectorCallbackImpl.java @@ -119,13 +119,13 @@ public final class TimeZoneDetectorCallbackImpl implements TimeZoneDetectorStrat @Override public ConfigurationInternal getConfigurationInternal(@UserIdInt int userId) { - boolean geoDetectionEnabled = mGeoDetectionFeatureEnabled && isGeoDetectionEnabled(userId); return new ConfigurationInternal.Builder(userId) .setUserConfigAllowed(isUserConfigAllowed(userId)) .setAutoDetectionSupported(isAutoDetectionSupported()) + .setGeoDetectionSupported(isGeoDetectionSupported()) .setAutoDetectionEnabled(isAutoDetectionEnabled()) .setLocationEnabled(isLocationEnabled(userId)) - .setGeoDetectionEnabled(geoDetectionEnabled) + .setGeoDetectionEnabled(isGeoDetectionEnabled(userId)) .build(); } @@ -170,7 +170,11 @@ public final class TimeZoneDetectorCallbackImpl implements TimeZoneDetectorStrat final boolean autoDetectionEnabled = configuration.isAutoDetectionEnabled(); setAutoDetectionEnabled(autoDetectionEnabled); - if (mGeoDetectionFeatureEnabled) { + // Avoid writing the geo detection enabled setting for devices that do not support geo + // time zone detection: if we wrote it down then we'd set the value explicitly, which + // would prevent detecting "default" later. That might influence what happens on later + // releases that support geo detection on the same hardware. + if (isGeoDetectionSupported()) { final boolean geoTzDetectionEnabled = configuration.isGeoDetectionEnabled(); setGeoDetectionEnabled(userId, geoTzDetectionEnabled); } @@ -183,7 +187,11 @@ public final class TimeZoneDetectorCallbackImpl implements TimeZoneDetectorStrat } private boolean isAutoDetectionSupported() { - return deviceHasTelephonyNetwork() || mGeoDetectionFeatureEnabled; + return deviceHasTelephonyNetwork() || isGeoDetectionSupported(); + } + + private boolean isGeoDetectionSupported() { + return mGeoDetectionFeatureEnabled; } private boolean isAutoDetectionEnabled() { diff --git a/services/tests/servicestests/src/com/android/server/location/timezone/TestSupport.java b/services/tests/servicestests/src/com/android/server/location/timezone/TestSupport.java index 192ade7847b0e..48105634c69df 100644 --- a/services/tests/servicestests/src/com/android/server/location/timezone/TestSupport.java +++ b/services/tests/servicestests/src/com/android/server/location/timezone/TestSupport.java @@ -45,6 +45,7 @@ final class TestSupport { return new ConfigurationInternal.Builder(userId) .setUserConfigAllowed(true) .setAutoDetectionSupported(true) + .setGeoDetectionSupported(true) .setAutoDetectionEnabled(true) .setLocationEnabled(true) .setGeoDetectionEnabled(geoDetectionEnabled) diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/ConfigurationInternalTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/ConfigurationInternalTest.java index 3fc294d6ea897..682a80c9b2975 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/ConfigurationInternalTest.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/ConfigurationInternalTest.java @@ -47,6 +47,7 @@ public class ConfigurationInternalTest { ConfigurationInternal baseConfig = new ConfigurationInternal.Builder(ARBITRARY_USER_ID) .setUserConfigAllowed(true) .setAutoDetectionSupported(true) + .setGeoDetectionSupported(true) .setAutoDetectionEnabled(true) .setLocationEnabled(true) .setGeoDetectionEnabled(true) @@ -108,6 +109,7 @@ public class ConfigurationInternalTest { ConfigurationInternal baseConfig = new ConfigurationInternal.Builder(ARBITRARY_USER_ID) .setUserConfigAllowed(false) .setAutoDetectionSupported(true) + .setGeoDetectionSupported(true) .setAutoDetectionEnabled(true) .setLocationEnabled(true) .setGeoDetectionEnabled(true) @@ -169,6 +171,7 @@ public class ConfigurationInternalTest { ConfigurationInternal baseConfig = new ConfigurationInternal.Builder(ARBITRARY_USER_ID) .setUserConfigAllowed(true) .setAutoDetectionSupported(false) + .setGeoDetectionSupported(false) .setAutoDetectionEnabled(true) .setLocationEnabled(true) .setGeoDetectionEnabled(true) @@ -220,4 +223,67 @@ public class ConfigurationInternalTest { assertTrue(configuration.isGeoDetectionEnabled()); } } + + /** + * Tests when {@link ConfigurationInternal#isAutoDetectionSupported()} is true, but + * {@link ConfigurationInternal#isGeoDetectionSupported()} is false. + */ + @Test + public void test_geoDetectNotSupported() { + ConfigurationInternal baseConfig = new ConfigurationInternal.Builder(ARBITRARY_USER_ID) + .setUserConfigAllowed(true) + .setAutoDetectionSupported(true) + .setGeoDetectionSupported(false) + .setAutoDetectionEnabled(true) + .setLocationEnabled(true) + .setGeoDetectionEnabled(true) + .build(); + { + ConfigurationInternal autoOnConfig = new ConfigurationInternal.Builder(baseConfig) + .setAutoDetectionEnabled(true) + .build(); + assertTrue(autoOnConfig.getAutoDetectionEnabledSetting()); + assertTrue(autoOnConfig.getGeoDetectionEnabledSetting()); + assertTrue(autoOnConfig.getAutoDetectionEnabledBehavior()); + assertFalse(autoOnConfig.getGeoDetectionEnabledBehavior()); + + TimeZoneCapabilitiesAndConfig capabilitiesAndConfig = + autoOnConfig.createCapabilitiesAndConfig(); + + TimeZoneCapabilities capabilities = capabilitiesAndConfig.getCapabilities(); + assertEquals(CAPABILITY_POSSESSED, + capabilities.getConfigureAutoDetectionEnabledCapability()); + assertEquals(CAPABILITY_NOT_SUPPORTED, + capabilities.getConfigureGeoDetectionEnabledCapability()); + assertEquals(CAPABILITY_NOT_APPLICABLE, + capabilities.getSuggestManualTimeZoneCapability()); + + TimeZoneConfiguration configuration = capabilitiesAndConfig.getConfiguration(); + assertTrue(configuration.isAutoDetectionEnabled()); + assertTrue(configuration.isGeoDetectionEnabled()); + } + { + ConfigurationInternal autoOffConfig = new ConfigurationInternal.Builder(baseConfig) + .setAutoDetectionEnabled(false) + .build(); + assertFalse(autoOffConfig.getAutoDetectionEnabledSetting()); + assertTrue(autoOffConfig.getGeoDetectionEnabledSetting()); + assertFalse(autoOffConfig.getAutoDetectionEnabledBehavior()); + assertFalse(autoOffConfig.getGeoDetectionEnabledBehavior()); + + TimeZoneCapabilitiesAndConfig capabilitiesAndConfig = + autoOffConfig.createCapabilitiesAndConfig(); + + TimeZoneCapabilities capabilities = capabilitiesAndConfig.getCapabilities(); + assertEquals(CAPABILITY_POSSESSED, + capabilities.getConfigureAutoDetectionEnabledCapability()); + assertEquals(CAPABILITY_NOT_SUPPORTED, + capabilities.getConfigureGeoDetectionEnabledCapability()); + assertEquals(CAPABILITY_POSSESSED, capabilities.getSuggestManualTimeZoneCapability()); + + TimeZoneConfiguration configuration = capabilitiesAndConfig.getConfiguration(); + assertFalse(configuration.isAutoDetectionEnabled()); + assertTrue(configuration.isGeoDetectionEnabled()); + } + } } 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 cb27657f8364c..d2452eaee6570 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorServiceTest.java @@ -365,6 +365,7 @@ public class TimeZoneDetectorServiceTest { final boolean geoDetectionEnabled = autoDetectionEnabled; return new ConfigurationInternal.Builder(ARBITRARY_USER_ID) .setAutoDetectionSupported(true) + .setGeoDetectionSupported(true) .setUserConfigAllowed(true) .setAutoDetectionEnabled(autoDetectionEnabled) .setLocationEnabled(geoDetectionEnabled) 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 296aa73f9c453..a6ffd201c16eb 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorStrategyImplTest.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/TimeZoneDetectorStrategyImplTest.java @@ -91,6 +91,7 @@ public class TimeZoneDetectorStrategyImplTest { new ConfigurationInternal.Builder(USER_ID) .setUserConfigAllowed(false) .setAutoDetectionSupported(true) + .setGeoDetectionSupported(true) .setAutoDetectionEnabled(false) .setLocationEnabled(true) .setGeoDetectionEnabled(false) @@ -100,6 +101,7 @@ public class TimeZoneDetectorStrategyImplTest { new ConfigurationInternal.Builder(USER_ID) .setUserConfigAllowed(false) .setAutoDetectionSupported(true) + .setGeoDetectionSupported(true) .setAutoDetectionEnabled(true) .setLocationEnabled(true) .setGeoDetectionEnabled(true) @@ -109,32 +111,36 @@ public class TimeZoneDetectorStrategyImplTest { new ConfigurationInternal.Builder(USER_ID) .setUserConfigAllowed(true) .setAutoDetectionSupported(false) + .setGeoDetectionSupported(false) .setAutoDetectionEnabled(false) .setLocationEnabled(true) .setGeoDetectionEnabled(false) .build(); + private static final ConfigurationInternal CONFIG_INT_AUTO_SUPPORTED_GEO_NOT_SUPPORTED = + new ConfigurationInternal.Builder(USER_ID) + .setUserConfigAllowed(true) + .setAutoDetectionSupported(true) + .setGeoDetectionSupported(false) + .setAutoDetectionEnabled(true) + .setLocationEnabled(true) + .setGeoDetectionEnabled(true) + .build(); + private static final ConfigurationInternal CONFIG_INT_AUTO_DISABLED_GEO_DISABLED = new ConfigurationInternal.Builder(USER_ID) .setUserConfigAllowed(true) .setAutoDetectionSupported(true) + .setGeoDetectionSupported(true) .setAutoDetectionEnabled(false) .setLocationEnabled(true) .setGeoDetectionEnabled(false) .build(); - private static final ConfigurationInternal CONFIG_INT_AUTO_DISABLED_GEO_ENABLED = - new ConfigurationInternal.Builder(USER_ID) - .setUserConfigAllowed(true) - .setAutoDetectionSupported(true) - .setAutoDetectionEnabled(false) - .setLocationEnabled(true) - .setGeoDetectionEnabled(true) - .build(); - private static final ConfigurationInternal CONFIG_INT_AUTO_ENABLED_GEO_DISABLED = new ConfigurationInternal.Builder(USER_ID) .setAutoDetectionSupported(true) + .setGeoDetectionSupported(true) .setUserConfigAllowed(true) .setAutoDetectionEnabled(true) .setLocationEnabled(true) @@ -144,6 +150,7 @@ public class TimeZoneDetectorStrategyImplTest { private static final ConfigurationInternal CONFIG_INT_AUTO_ENABLED_GEO_ENABLED = new ConfigurationInternal.Builder(USER_ID) .setAutoDetectionSupported(true) + .setGeoDetectionSupported(true) .setUserConfigAllowed(true) .setAutoDetectionEnabled(true) .setLocationEnabled(true) @@ -223,14 +230,14 @@ public class TimeZoneDetectorStrategyImplTest { // The settings should not have been changed: user shouldn't have the capabilities. script.verifyConfigurationNotChanged(); - // Update the configuration with auto detection enabled. + // Try to update the configuration with auto detection enabled. script.simulateUpdateConfiguration( USER_ID, CONFIG_AUTO_ENABLED, false /* expectedResult */); // The settings should not have been changed: user shouldn't have the capabilities. script.verifyConfigurationNotChanged(); - // Try to update the configuration to enable geolocation time zone detection. + // Try to update the configuration to enable geolocation time zone detection. script.simulateUpdateConfiguration( USER_ID, CONFIG_GEO_DETECTION_ENABLED, false /* expectedResult */); @@ -249,7 +256,7 @@ public class TimeZoneDetectorStrategyImplTest { // The settings should not have been changed: user shouldn't have the capabilities. script.verifyConfigurationNotChanged(); - // Update the configuration with auto detection enabled. + // Try to update the configuration with auto detection enabled. script.simulateUpdateConfiguration( USER_ID, CONFIG_AUTO_ENABLED, false /* expectedResult */); @@ -257,6 +264,38 @@ public class TimeZoneDetectorStrategyImplTest { script.verifyConfigurationNotChanged(); } + @Test + public void testUpdateConfiguration_autoDetectSupportedGeoNotSupported() { + Script script = new Script().initializeConfig(CONFIG_INT_AUTO_SUPPORTED_GEO_NOT_SUPPORTED); + + // Update the configuration with auto detection disabled. + script.simulateUpdateConfiguration( + USER_ID, CONFIG_AUTO_DISABLED, true /* expectedResult */); + + // The settings should have been changed and the StrategyListener onChange() called. + ConfigurationInternal expectedConfig = + new ConfigurationInternal.Builder(CONFIG_INT_AUTO_SUPPORTED_GEO_NOT_SUPPORTED) + .setAutoDetectionEnabled(false) + .build(); + script.verifyConfigurationChangedAndReset(expectedConfig); + + // Try to update the configuration with geo detection disabled. + script.simulateUpdateConfiguration( + USER_ID, CONFIG_GEO_DETECTION_DISABLED, false /* expectedResult */); + + // The settings should not have been changed: user shouldn't have the capability to modify + // the setting when the feature is disabled. + script.verifyConfigurationNotChanged(); + + // Try to update the configuration with geo detection enabled. + script.simulateUpdateConfiguration( + USER_ID, CONFIG_GEO_DETECTION_ENABLED, false /* expectedResult */); + + // The settings should not have been changed: user shouldn't have the capability to modify + // the setting when the feature is disabled. + script.verifyConfigurationNotChanged(); + } + @Test public void testEmptyTelephonySuggestions() { TelephonyTimeZoneSuggestion slotIndex1TimeZoneSuggestion =