Merge "Log when the location indicator is displayed as well as the type of location access which prompted the display."

This commit is contained in:
Kate Montgomery
2022-01-25 23:52:31 +00:00
committed by Android (Google) Code Review
2 changed files with 119 additions and 7 deletions

View File

@@ -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<AppOpItem> appOpsItems = mAppOpsController.getActiveAppOps();
final List<UserInfo> 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<UserInfo> 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;
}
}
}

View File

@@ -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);