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 59969c0447b37..2a98694109a52 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/LocationControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/LocationControllerImpl.java @@ -43,6 +43,8 @@ import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; import com.android.internal.config.sysui.SystemUiDeviceConfigFlags; +import com.android.internal.logging.UiEvent; +import com.android.internal.logging.UiEventLogger; import com.android.systemui.BootCompleteCache; import com.android.systemui.appops.AppOpItem; import com.android.systemui.appops.AppOpsController; @@ -71,6 +73,7 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio private final DeviceConfigProxy mDeviceConfigProxy; private final BootCompleteCache mBootCompleteCache; private final UserTracker mUserTracker; + private final UiEventLogger mUiEventLogger; private final H mHandler; private final Handler mBackgroundHandler; private final PackageManager mPackageManager; @@ -84,13 +87,14 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio DeviceConfigProxy deviceConfigProxy, @Main Looper mainLooper, @Background Handler backgroundHandler, BroadcastDispatcher broadcastDispatcher, BootCompleteCache bootCompleteCache, - UserTracker userTracker, PackageManager packageManager) { + UserTracker userTracker, PackageManager packageManager, UiEventLogger uiEventLogger) { mContext = context; mAppOpsController = appOpsController; mDeviceConfigProxy = deviceConfigProxy; mBootCompleteCache = bootCompleteCache; mHandler = new H(mainLooper); mUserTracker = userTracker; + mUiEventLogger = uiEventLogger; mBackgroundHandler = backgroundHandler; mPackageManager = packageManager; mShouldDisplayAllAccesses = getAllAccessesSetting(); @@ -222,6 +226,9 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio } boolean hadActiveLocationRequests = mAreActiveLocationRequests; boolean shouldDisplay = false; + boolean systemAppOp = false; + boolean nonSystemAppOp = false; + boolean isSystemApp; List appOpsItems = mAppOpsController.getActiveAppOps(); final List profiles = mUserTracker.getUserProfiles(); @@ -229,18 +236,38 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio for (int i = 0; i < numItems; i++) { if (appOpsItems.get(i).getCode() == OP_FINE_LOCATION || appOpsItems.get(i).getCode() == OP_COARSE_LOCATION) { - if (mShowSystemAccesses) { - shouldDisplay = true; + isSystemApp = isSystemApp(profiles, appOpsItems.get(i)); + if (isSystemApp) { + systemAppOp = true; } else { - shouldDisplay |= !isSystemApp(profiles, appOpsItems.get(i)); + nonSystemAppOp = true; } + + shouldDisplay = mShowSystemAccesses || shouldDisplay || !isSystemApp; } } - mAreActiveLocationRequests = areActiveHighPowerLocationRequests() || shouldDisplay; + boolean highPowerOp = areActiveHighPowerLocationRequests(); + mAreActiveLocationRequests = highPowerOp || shouldDisplay; if (mAreActiveLocationRequests != hadActiveLocationRequests) { mHandler.sendEmptyMessage(H.MSG_LOCATION_ACTIVE_CHANGED); } + + // Log each of the types of location access that would cause the location indicator to be + // shown, regardless of device's setting state. This is used to understand how often a + // user would see the location indicator based on any settings state the device could be in. + if (!hadActiveLocationRequests && (highPowerOp || systemAppOp || nonSystemAppOp)) { + if (highPowerOp) { + mUiEventLogger.log( + LocationIndicatorEvent.LOCATION_INDICATOR_MONITOR_HIGH_POWER); + } + if (systemAppOp) { + mUiEventLogger.log(LocationIndicatorEvent.LOCATION_INDICATOR_SYSTEM_APP); + } + if (nonSystemAppOp) { + mUiEventLogger.log(LocationIndicatorEvent.LOCATION_INDICATOR_NON_SYSTEM_APP); + } + } } private boolean isSystemApp(List profiles, AppOpItem item) { @@ -283,6 +310,11 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio mAreActiveLocationRequests = areActiveHighPowerLocationRequests(); if (mAreActiveLocationRequests != hadActiveLocationRequests) { mHandler.sendEmptyMessage(H.MSG_LOCATION_ACTIVE_CHANGED); + if (mAreActiveLocationRequests) { + // Log that the indicator was shown for a high power op. + mUiEventLogger.log( + LocationIndicatorEvent.LOCATION_INDICATOR_MONITOR_HIGH_POWER); + } } } } @@ -341,4 +373,24 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio cb -> cb.onLocationSettingsChanged(isEnabled)); } } -} + + /** + * Enum for events which prompt the location indicator to appear. + */ + enum LocationIndicatorEvent implements UiEventLogger.UiEventEnum { + @UiEvent(doc = "Location indicator shown for high power access") + LOCATION_INDICATOR_MONITOR_HIGH_POWER(935), + @UiEvent(doc = "Location indicator shown for system app access") + LOCATION_INDICATOR_SYSTEM_APP(936), + @UiEvent(doc = "Location indicator shown for non system app access") + LOCATION_INDICATOR_NON_SYSTEM_APP(937); + + private final int mId; + LocationIndicatorEvent(int id) { + mId = id; + } + @Override public int getId() { + return mId; + } + } +} \ No newline at end of file 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 2126dda7b3109..a6e942673187a 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 @@ -14,6 +14,8 @@ package com.android.systemui.statusbar.policy; +import static com.google.common.truth.Truth.assertThat; + import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -34,6 +36,7 @@ import android.testing.TestableLooper.RunWithLooper; import androidx.test.filters.SmallTest; import com.android.internal.config.sysui.SystemUiDeviceConfigFlags; +import com.android.internal.logging.testing.UiEventLoggerFake; import com.android.systemui.BootCompleteCache; import com.android.systemui.SysuiTestCase; import com.android.systemui.appops.AppOpItem; @@ -60,6 +63,7 @@ public class LocationControllerImplTest extends SysuiTestCase { private LocationControllerImpl mLocationController; private TestableLooper mTestableLooper; private DeviceConfigProxy mDeviceConfigProxy; + private UiEventLoggerFake mUiEventLogger; @Mock private AppOpsController mAppOpsController; @Mock private UserTracker mUserTracker; @@ -72,6 +76,7 @@ public class LocationControllerImplTest extends SysuiTestCase { when(mUserTracker.getUserProfiles()) .thenReturn(ImmutableList.of(new UserInfo(0, "name", 0))); mDeviceConfigProxy = new DeviceConfigProxyFake(); + mUiEventLogger = new UiEventLoggerFake(); mTestableLooper = TestableLooper.get(this); mLocationController = new LocationControllerImpl(mContext, @@ -82,7 +87,8 @@ public class LocationControllerImplTest extends SysuiTestCase { mock(BroadcastDispatcher.class), mock(BootCompleteCache.class), mUserTracker, - mContext.getPackageManager()); + mContext.getPackageManager(), + mUiEventLogger); mTestableLooper.processAllMessages(); } @@ -160,6 +166,10 @@ public class LocationControllerImplTest extends SysuiTestCase { mTestableLooper.processAllMessages(); verify(callback, times(1)).onLocationActiveChanged(anyBoolean()); + assertThat(mUiEventLogger.numLogs()).isEqualTo(1); + assertThat(mUiEventLogger.eventId(0)).isEqualTo( + LocationControllerImpl.LocationIndicatorEvent.LOCATION_INDICATOR_MONITOR_HIGH_POWER + .getId()); } @Test @@ -253,6 +263,56 @@ public class LocationControllerImplTest extends SysuiTestCase { verify(callback, times(1)).onLocationActiveChanged(false); } + @Test + public void testCallbackNotified_verifyMetrics() { + LocationChangeCallback callback = mock(LocationChangeCallback.class); + mLocationController.addCallback(callback); + mDeviceConfigProxy.setProperty( + DeviceConfig.NAMESPACE_PRIVACY, + SystemUiDeviceConfigFlags.PROPERTY_LOCATION_INDICATORS_SMALL_ENABLED, + "true", + true); + mTestableLooper.processAllMessages(); + + when(mAppOpsController.getActiveAppOps()) + .thenReturn(ImmutableList.of( + new AppOpItem(AppOpsManager.OP_FINE_LOCATION, 0, "com.google.android.gms", + System.currentTimeMillis()), + new AppOpItem(AppOpsManager.OP_COARSE_LOCATION, 0, + "com.google.android.googlequicksearchbox", + System.currentTimeMillis()), + new AppOpItem(AppOpsManager.OP_MONITOR_HIGH_POWER_LOCATION, 0, + "com.google.android.apps.maps", + System.currentTimeMillis()))); + mLocationController.onActiveStateChanged(AppOpsManager.OP_FINE_LOCATION, 0, + "com.google.android.gms", true); + + mTestableLooper.processAllMessages(); + + verify(callback, times(1)).onLocationActiveChanged(true); + assertThat(mUiEventLogger.numLogs()).isEqualTo(3); + assertThat(mUiEventLogger.eventId(0)).isEqualTo( + LocationControllerImpl.LocationIndicatorEvent.LOCATION_INDICATOR_MONITOR_HIGH_POWER + .getId()); + // Even though the system access wasn't shown due to the device settings, ensure it was + // still logged. + assertThat(mUiEventLogger.eventId(1)).isEqualTo( + LocationControllerImpl.LocationIndicatorEvent.LOCATION_INDICATOR_SYSTEM_APP + .getId()); + assertThat(mUiEventLogger.eventId(2)).isEqualTo( + LocationControllerImpl.LocationIndicatorEvent.LOCATION_INDICATOR_NON_SYSTEM_APP + .getId()); + mUiEventLogger.getLogs().clear(); + + when(mAppOpsController.getActiveAppOps()).thenReturn(ImmutableList.of()); + mLocationController.onActiveStateChanged(AppOpsManager.OP_FINE_LOCATION, 0, + "com.google.android.gms", false); + mTestableLooper.processAllMessages(); + + verify(callback, times(1)).onLocationActiveChanged(false); + assertThat(mUiEventLogger.numLogs()).isEqualTo(0); + } + @Test public void testCallbackRemoved() { LocationChangeCallback callback = mock(LocationChangeCallback.class);