From 321b621ee0e48efd6edff25b361aa7118a90e770 Mon Sep 17 00:00:00 2001 From: Kate Montgomery Date: Fri, 25 Mar 2022 20:33:20 +0000 Subject: [PATCH] Update show system experiment behavior. When the experiment flag is flipped to on, toggle the "show system accesses" setting to on one time and then do not modify the value again unless the experiment is turned off and then on again. Bug: 191503437 Test: atest LocationControllerImplTest Change-Id: I25e647bc3ccfb911719b2047a5096f9a9e62a3ad --- core/java/android/provider/Settings.java | 7 +++ .../policy/LocationControllerImpl.java | 43 +++++++++++++-- .../policy/LocationControllerImplTest.java | 54 ++++++++++++++++--- 3 files changed, 94 insertions(+), 10 deletions(-) diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 5bd77abc277f1..48fa06a1fbe1c 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -7229,6 +7229,13 @@ public final class Settings { */ public static final String LOCATION_SHOW_SYSTEM_OPS = "locationShowSystemOps"; + + /** + * Whether or not an indicator experiment has started. + * @hide + */ + public static final String LOCATION_INDICATOR_EXPERIMENT_STARTED = + "locationIndicatorExperimentStarted"; /** * A flag containing settings used for biometric weak * @hide diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/LocationControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/LocationControllerImpl.java index f52c6ed0a2328..3e50acb75401e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/LocationControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/LocationControllerImpl.java @@ -86,6 +86,7 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio private boolean mShouldDisplayAllAccesses; private boolean mShowSystemAccessesFlag; private boolean mShowSystemAccessesSetting; + private boolean mExperimentStarted; @Inject public LocationControllerImpl(Context context, AppOpsController appOpsController, @@ -107,6 +108,9 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio mShouldDisplayAllAccesses = getAllAccessesSetting(); mShowSystemAccessesFlag = getShowSystemFlag(); mShowSystemAccessesSetting = getShowSystemSetting(); + mExperimentStarted = getExperimentStarted(); + toggleSystemSettingIfExperimentJustStarted(); + mContentObserver = new ContentObserver(mBackgroundHandler) { @Override public void onChange(boolean selfChange) { @@ -123,8 +127,15 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio DeviceConfig.NAMESPACE_PRIVACY, backgroundHandler::post, properties -> { + // Update the Device Config flag which controls the experiment to display + // location accesses. mShouldDisplayAllAccesses = getAllAccessesSetting(); - mShowSystemAccessesFlag = getShowSystemSetting(); + // Update the Device Config flag which controls the experiment to display + // system location accesses. + mShowSystemAccessesFlag = getShowSystemFlag(); + // Update the local flag for the system accesses experiment and potentially + // update the behavior based on the flag value. + toggleSystemSettingIfExperimentJustStarted(); updateActiveLocationRequests(); }); @@ -222,6 +233,33 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio return mSecureSettings.getInt(Settings.Secure.LOCATION_SHOW_SYSTEM_OPS, 0) == 1; } + private boolean getExperimentStarted() { + return mSecureSettings + .getInt(Settings.Secure.LOCATION_INDICATOR_EXPERIMENT_STARTED, 0) == 1; + } + + private void toggleSystemSettingIfExperimentJustStarted() { + // mShowSystemAccessesFlag indicates whether the Device Config flag is flipped + // by an experiment. mExperimentStarted is the local device value which indicates the last + // value the device has seen for the Device Config flag. + // The local device value is needed to determine that the Device Config flag was just + // flipped, as the experiment behavior should only happen once after the experiment is + // enabled. + if (mShowSystemAccessesFlag && !mExperimentStarted) { + // If the Device Config flag is enabled, but the local device setting is not then the + // experiment just started. Update the local flag to match and enable the experiment + // behavior by flipping the show system setting value. + mSecureSettings.putInt(Settings.Secure.LOCATION_INDICATOR_EXPERIMENT_STARTED, 1); + mSecureSettings.putInt(Settings.Secure.LOCATION_SHOW_SYSTEM_OPS, 1); + mExperimentStarted = true; + } else if (!mShowSystemAccessesFlag && mExperimentStarted) { + // If the Device Config flag is disabled, but the local device flag is enabled then + // update the local device flag to match. + mSecureSettings.putInt(Settings.Secure.LOCATION_INDICATOR_EXPERIMENT_STARTED, 0); + mExperimentStarted = false; + } + } + /** * Returns true if there currently exist active high power location requests. */ @@ -249,7 +287,6 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio } boolean hadActiveLocationRequests = mAreActiveLocationRequests; boolean shouldDisplay = false; - boolean showSystem = mShowSystemAccessesFlag || mShowSystemAccessesSetting; boolean systemAppOp = false; boolean nonSystemAppOp = false; boolean isSystemApp; @@ -267,7 +304,7 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio nonSystemAppOp = true; } - shouldDisplay = showSystem || shouldDisplay || !isSystemApp; + shouldDisplay = mShowSystemAccessesSetting || shouldDisplay || !isSystemApp; } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/LocationControllerImplTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/LocationControllerImplTest.java index 12cfb32ab57e6..14e087846417c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/LocationControllerImplTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/LocationControllerImplTest.java @@ -51,7 +51,7 @@ import com.android.systemui.settings.UserTracker; import com.android.systemui.statusbar.policy.LocationController.LocationChangeCallback; import com.android.systemui.util.DeviceConfigProxy; import com.android.systemui.util.DeviceConfigProxyFake; -import com.android.systemui.util.settings.SecureSettings; +import com.android.systemui.util.settings.FakeSettings; import com.google.common.collect.ImmutableList; @@ -70,11 +70,11 @@ public class LocationControllerImplTest extends SysuiTestCase { private TestableLooper mTestableLooper; private DeviceConfigProxy mDeviceConfigProxy; private UiEventLoggerFake mUiEventLogger; + private FakeSettings mSecureSettings; @Mock private PackageManager mPackageManager; @Mock private AppOpsController mAppOpsController; @Mock private UserTracker mUserTracker; - @Mock private SecureSettings mSecureSettings; @Before public void setup() { @@ -85,7 +85,7 @@ public class LocationControllerImplTest extends SysuiTestCase { .thenReturn(ImmutableList.of(new UserInfo(0, "name", 0))); mDeviceConfigProxy = new DeviceConfigProxyFake(); mUiEventLogger = new UiEventLoggerFake(); - + mSecureSettings = new FakeSettings(); mTestableLooper = TestableLooper.get(this); mLocationController = new LocationControllerImpl(mContext, @@ -248,8 +248,7 @@ public class LocationControllerImplTest extends SysuiTestCase { @Test public void testCallbackNotified_additionalOps_shouldShowSystem() { - when(mSecureSettings.getInt(Settings.Secure.LOCATION_SHOW_SYSTEM_OPS, 0)) - .thenReturn(1); + mSecureSettings.putInt(Settings.Secure.LOCATION_SHOW_SYSTEM_OPS, 1); LocationChangeCallback callback = mock(LocationChangeCallback.class); mLocationController.addCallback(callback); mDeviceConfigProxy.setProperty( @@ -282,8 +281,7 @@ public class LocationControllerImplTest extends SysuiTestCase { verify(callback, times(1)).onLocationActiveChanged(false); - when(mSecureSettings.getInt(Settings.Secure.LOCATION_SHOW_SYSTEM_OPS, 0)) - .thenReturn(0); + mSecureSettings.putInt(Settings.Secure.LOCATION_SHOW_SYSTEM_OPS, 0); mLocationController.onActiveStateChanged(AppOpsManager.OP_FINE_LOCATION, 0, "com.system.app", true); @@ -367,4 +365,46 @@ public class LocationControllerImplTest extends SysuiTestCase { // No new callbacks verify(callback).onLocationSettingsChanged(anyBoolean()); } + + @Test + public void testExperimentFlipsSystemFlag() throws Exception { + mSecureSettings.putInt(Settings.Secure.LOCATION_SHOW_SYSTEM_OPS, 0); + mDeviceConfigProxy.setProperty( + DeviceConfig.NAMESPACE_PRIVACY, + SystemUiDeviceConfigFlags.PROPERTY_LOCATION_INDICATORS_SMALL_ENABLED, + "true", + true); + // Show system experiment not running + mDeviceConfigProxy.setProperty( + DeviceConfig.NAMESPACE_PRIVACY, + SystemUiDeviceConfigFlags.PROPERTY_LOCATION_INDICATORS_SHOW_SYSTEM, + "false", + false); + mTestableLooper.processAllMessages(); + + // Flip experiment on + mDeviceConfigProxy.setProperty( + DeviceConfig.NAMESPACE_PRIVACY, + SystemUiDeviceConfigFlags.PROPERTY_LOCATION_INDICATORS_SHOW_SYSTEM, + "true", + true); + mTestableLooper.processAllMessages(); + + // Verify settings were flipped + assertThat(mSecureSettings.getInt(Settings.Secure.LOCATION_SHOW_SYSTEM_OPS)) + .isEqualTo(1); + assertThat(mSecureSettings.getInt(Settings.Secure.LOCATION_INDICATOR_EXPERIMENT_STARTED)) + .isEqualTo(1); + + // Flip experiment off + mDeviceConfigProxy.setProperty( + DeviceConfig.NAMESPACE_PRIVACY, + SystemUiDeviceConfigFlags.PROPERTY_LOCATION_INDICATORS_SHOW_SYSTEM, + "false", + false); + mTestableLooper.processAllMessages(); + + assertThat(mSecureSettings.getInt(Settings.Secure.LOCATION_INDICATOR_EXPERIMENT_STARTED)) + .isEqualTo(0); + } } \ No newline at end of file