Merge "Log when the location indicator is displayed in various situations and... (248f1a24) · Commits · e / os / android_frameworks_base

packages/SystemUI/src/com/android/systemui/privacy/AppOpsPrivacyItemMonitor.kt

+185 −26

Original line number	Diff line number	Diff line
		@@ -25,6 +25,8 @@ import android.content.pm.UserInfo
		import android.os.UserHandle
		import com.android.internal.annotations.GuardedBy
		import com.android.internal.annotations.VisibleForTesting
		import com.android.internal.logging.UiEvent
		import com.android.internal.logging.UiEventLogger
		import com.android.systemui.appops.AppOpItem
		import com.android.systemui.appops.AppOpsController
		import com.android.systemui.dagger.SysUISingleton
		@@ -58,6 +60,7 @@ constructor(
		private val packageManager: PackageManager,
		private val activityManager: ActivityManager,
		private val context: Context,
		private val uiEventLogger: UiEventLogger,
		) : PrivacyItemMonitor {

		@VisibleForTesting
		@@ -86,6 +89,34 @@ constructor(
		@GuardedBy("lock") private var locationAvailable = privacyConfig.locationAvailable
		@GuardedBy("lock") private var listening = false

		// Various state needed for logging

		// True if the location indicator was ON, when location delivered to foreground, non-system apps
		@GuardedBy("lock") private var lastLocationIndicator = false

		// True if the location indicator was ON, when location delivered to foreground, background,
		// system apps
		@GuardedBy("lock") private var lastLocationIndicatorWithSystem = false

		// True if the location indicator was ON, when location delivered to foreground, system,
		// non-system apps
		@GuardedBy("lock") private var lastLocationIndicatorWithBackround = false

		// True if the location indicator was ON, when location delivered to all apps
		@GuardedBy("lock") private var lastLocationIndicatorWithSystemAndBackround = false

		// Keep track of the last MONITOR_HIGH_POWER_LOCATION appOp, since this is not included in the
		// PrivacyItems but needs to be tracked for logging purposes.
		@GuardedBy("lock") private var lastHighPowerLocationOp = false

		// The following keep track of whether a type of location client exists in the current round
		// of active appOps

		@GuardedBy("lock") private var hasHighPowerLocationAccess = false
		@GuardedBy("lock") private var hasSystemLocationAccess = false
		@GuardedBy("lock") private var hasBackgroundLocationAccess = false
		@GuardedBy("lock") private var hasNonSystemForegroundLocationAccess = false

		private val appOpsCallback =
		object : AppOpsController.Callback {
		override fun onActiveStateChanged(
		@@ -197,7 +228,8 @@ constructor(
		// TODO(b/419834493): Consider refactoring this into a Flow that could be configured to run
		// on a bg context.
		Assert.isNotMainThread()
		return synchronized(lock) {
		val items =
		synchronized(lock) {
		activeAppOps
		.filter {
		currentUserProfiles.any { user ->
		@@ -208,6 +240,78 @@ constructor(
		.mapNotNull { toPrivacyItemLocked(it) }
		}
		.distinct()

		// Types of location accesses were stored when iterating through the app ops in
		// #shouldDisplayLocationOp and now they will be logged and the state will be cleared
		logLocationAccesses()

		// Keep track of the current privacy items in order to determine whether to log the next
		// round of privacy item changes.
		val locationItems =
		activeAppOps
		.filter { item ->
		(item.code == AppOpsManager.OP_FINE_LOCATION \|\|
		item.code == AppOpsManager.OP_COARSE_LOCATION)
		}
		.distinct()
		val locationOpBySystem = locationItems.any { item -> isSystemApp(item) }
		val locationOpByBackground = locationItems.any { item -> isBackgroundApp(item) }
		synchronized(lock) {
		lastLocationIndicator = items.any { it.privacyType == PrivacyType.TYPE_LOCATION }
		lastLocationIndicatorWithSystem = lastLocationIndicator \|\| locationOpBySystem
		lastLocationIndicatorWithBackround = lastLocationIndicator \|\| locationOpByBackground
		lastLocationIndicatorWithSystemAndBackround =
		lastLocationIndicator \|\| locationOpBySystem \|\| locationOpByBackground
		lastHighPowerLocationOp =
		activeAppOps.any { it.code == AppOpsManager.OP_MONITOR_HIGH_POWER_LOCATION }
		}

		return items
		}

		/**
		* Log which appOps would cause the location indicator to show in various situations. This
		* should only be logged if the location indicator was not already showing because the op would
		* not result in a change in the indicator display.
		*/
		private fun logLocationAccesses() {
		// TODO(b/419834493): Add logbuffer logging here for bugreport debugging.
		synchronized(lock) {
		if (!lastHighPowerLocationOp && hasHighPowerLocationAccess) {
		uiEventLogger.log {
		LocationIndicatorEvent.LOCATION_INDICATOR_MONITOR_HIGH_POWER.id
		}
		}
		if (!lastLocationIndicator && hasNonSystemForegroundLocationAccess) {
		uiEventLogger.log { LocationIndicatorEvent.LOCATION_INDICATOR_NON_SYSTEM_APP.id }
		}
		if (!lastLocationIndicatorWithSystem) {
		if (hasNonSystemForegroundLocationAccess \|\| hasSystemLocationAccess) {
		uiEventLogger.log { LocationIndicatorEvent.LOCATION_INDICATOR_SYSTEM_APP.id }
		}
		}
		if (!lastLocationIndicatorWithBackround) {
		if (hasNonSystemForegroundLocationAccess \|\| hasBackgroundLocationAccess) {
		uiEventLogger.log {
		LocationIndicatorEvent.LOCATION_INDICATOR_BACKGROUND_APP.id
		}
		}
		}
		if (!lastLocationIndicatorWithSystemAndBackround) {
		if (
		hasNonSystemForegroundLocationAccess \|\|
		hasSystemLocationAccess \|\|
		hasBackgroundLocationAccess
		) {
		uiEventLogger.log { LocationIndicatorEvent.LOCATION_INDICATOR_ALL_APP.id }
		}
		}

		hasHighPowerLocationAccess = false
		hasSystemLocationAccess = false
		hasBackgroundLocationAccess = false
		hasNonSystemForegroundLocationAccess = false
		}
		}

		@GuardedBy("lock")
		@@ -256,14 +360,37 @@ constructor(
		}
		}

		// Only display the location privacy item if a non-system, foreground client accessed location
		/**
		* Only display the location privacy item if a non-system, foreground client accessed location
		*
		* This method has the side effects of updating [hasSystemLocationAccess],
		* [hasBackgroundLocationAccess], [hasNonSystemForegroundLocationAccess], and
		* [hasHighPowerLocationAccess]
		*/
		private fun shouldDisplayLocationOp(item: AppOpItem): Boolean {
		if (
		item.code == AppOpsManager.OP_FINE_LOCATION \|\|
		item.code == AppOpsManager.OP_COARSE_LOCATION
		) {
		return !isSystemApp(item) && !isBackgroundApp(item)
		val isSystem = isSystemApp(item)
		val isBackground = isBackgroundApp(item)
		if (isSystem) {
		synchronized(lock) { hasSystemLocationAccess = true }
		}
		if (isBackground) {
		synchronized(lock) { hasBackgroundLocationAccess = true }
		}
		val result = !isSystem && !isBackground
		if (result) {
		synchronized(lock) { hasNonSystemForegroundLocationAccess = true }
		}
		return result
		}

		if (item.code == AppOpsManager.OP_MONITOR_HIGH_POWER_LOCATION) {
		synchronized(lock) { hasHighPowerLocationAccess = true }
		}

		return true
		}

		@@ -290,7 +417,8 @@ constructor(
		val permission = AppOpsManager.opToPermission(item.code)
		val permissionFlags: Int =
		packageManager.getPermissionFlags(permission, item.packageName, user)
		return if (
		val isSystem =
		if (
		PermissionChecker.checkPermissionForPreflight(
		context,
		permission,
		@@ -299,10 +427,12 @@ constructor(
		item.packageName,
		) == PermissionChecker.PERMISSION_GRANTED
		) {
		((permissionFlags and PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_GRANTED) == 0)
		((permissionFlags and PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_GRANTED) ==
		0)
		} else {
		(permissionFlags and PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_DENIED) == 0
		}
		return isSystem
		}

		/**
		@@ -314,8 +444,8 @@ constructor(
		private fun isBackgroundApp(item: AppOpItem): Boolean {
		for (processInfo in activityManager.runningAppProcesses) {
		if (processInfo.uid == item.uid) {
		return processInfo.importance >
		ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND_SERVICE
		return (processInfo.importance >
		ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND_SERVICE)
		}
		}
		return false
		@@ -335,4 +465,33 @@ constructor(
		}
		ipw.flush()
		}

		/** Enum for events which prompt the location indicator to appear. */
		enum class LocationIndicatorEvent(private val id: Int) : UiEventLogger.UiEventEnum {
		// Copied from LocationControllerImpl.java
		@UiEvent(doc = "Location indicator shown for high power access")
		LOCATION_INDICATOR_MONITOR_HIGH_POWER(935),
		// Copied from LocationControllerImpl.java
		@UiEvent(
		doc =
		"Location indicator shown for system, non-system, foreground app access (i.e., excluding background)"
		)
		LOCATION_INDICATOR_SYSTEM_APP(936),
		// Copied from LocationControllerImpl.java
		@UiEvent(
		doc =
		"Location indicator shown for non-system, foreground app access (i.e., excluding system and background)"
		)
		LOCATION_INDICATOR_NON_SYSTEM_APP(937),
		@UiEvent(
		doc =
		"Location indicator shown for non-system, foreground, background app access (i.e., excluding system)"
		)
		LOCATION_INDICATOR_BACKGROUND_APP(2325),
		@UiEvent(doc = "Location indicator shown for all access") LOCATION_INDICATOR_ALL_APP(2354);

		override fun getId(): Int {
		return id
		}
		}
		}

packages/SystemUI/src/com/android/systemui/statusbar/policy/LocationControllerImpl.java

+0 −42

Original line number	Diff line number	Diff line
		@@ -45,7 +45,6 @@ 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;
		@@ -277,22 +276,6 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio
		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) {
		@@ -335,11 +318,6 @@ 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);
		}
		}
		}
		}
		@@ -415,24 +393,4 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio
		}
		}
		}

		/**
		* 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

packages/SystemUI/tests/src/com/android/systemui/privacy/AppOpsPrivacyItemMonitorTest.kt

+124 −1

Original line number	Diff line number	Diff line
		@@ -25,6 +25,7 @@ import android.os.UserHandle
		import android.testing.TestableLooper.RunWithLooper
		import androidx.test.ext.junit.runners.AndroidJUnit4
		import androidx.test.filters.SmallTest
		import com.android.internal.logging.testing.UiEventLoggerFake
		import com.android.systemui.SysuiTestCase
		import com.android.systemui.appops.AppOpItem
		import com.android.systemui.appops.AppOpsController
		@@ -32,6 +33,7 @@ import com.android.systemui.privacy.logging.PrivacyLogger
		import com.android.systemui.settings.UserTracker
		import com.android.systemui.util.concurrency.FakeExecutor
		import com.android.systemui.util.time.FakeSystemClock
		import com.google.common.truth.Truth
		import org.hamcrest.Matchers.hasItem
		import org.hamcrest.Matchers.not
		import org.hamcrest.Matchers.nullValue
		@@ -91,6 +93,7 @@ class AppOpsPrivacyItemMonitorTest : SysuiTestCase() {

		private lateinit var appOpsPrivacyItemMonitor: AppOpsPrivacyItemMonitor
		private lateinit var executor: FakeExecutor
		private lateinit var uiEventLogger: UiEventLoggerFake

		fun createAppOpsPrivacyItemMonitor(): AppOpsPrivacyItemMonitor {
		return AppOpsPrivacyItemMonitor(
		@@ -102,6 +105,7 @@ class AppOpsPrivacyItemMonitorTest : SysuiTestCase() {
		packageManager,
		activityManager,
		mContext,
		uiEventLogger,
		)
		}

		@@ -109,6 +113,7 @@ class AppOpsPrivacyItemMonitorTest : SysuiTestCase() {
		fun setup() {
		MockitoAnnotations.initMocks(this)
		executor = FakeExecutor(FakeSystemClock())
		uiEventLogger = UiEventLoggerFake()

		// Listen to everything by default
		`when`(privacyConfig.micCameraAvailable).thenReturn(true)
		@@ -313,9 +318,91 @@ class AppOpsPrivacyItemMonitorTest : SysuiTestCase() {
		.`when`(appOpsController)
		.getActiveAppOps(anyBoolean())

		val result = appOpsPrivacyItemMonitor.getActivePrivacyItems()
		var result = appOpsPrivacyItemMonitor.getActivePrivacyItems()
		assertEquals(result.size, 1)
		assertEquals(result[0].application.packageName, "com.google.android.apps.maps")

		assertEquals(uiEventLogger.numLogs(), 4)
		Truth.assertThat(
		uiEventLogger.logs.any { log ->
		log.eventId ==
		AppOpsPrivacyItemMonitor.LocationIndicatorEvent
		.LOCATION_INDICATOR_NON_SYSTEM_APP
		.id
		}
		)
		.isTrue()
		Truth.assertThat(
		uiEventLogger.logs.any { log ->
		log.eventId ==
		AppOpsPrivacyItemMonitor.LocationIndicatorEvent
		.LOCATION_INDICATOR_SYSTEM_APP
		.id
		}
		)
		.isTrue()
		Truth.assertThat(
		uiEventLogger.logs.any { log ->
		log.eventId ==
		AppOpsPrivacyItemMonitor.LocationIndicatorEvent
		.LOCATION_INDICATOR_BACKGROUND_APP
		.id
		}
		)
		.isTrue()
		Truth.assertThat(
		uiEventLogger.logs.any { log ->
		log.eventId ==
		AppOpsPrivacyItemMonitor.LocationIndicatorEvent.LOCATION_INDICATOR_ALL_APP
		.id
		}
		)
		.isTrue()

		// Simulate a new round of appOps and confirm that there are no additional logs because the
		// indicator is already showing.
		result = appOpsPrivacyItemMonitor.getActivePrivacyItems()
		assertEquals(result.size, 1)
		assertEquals(result[0].application.packageName, "com.google.android.apps.maps")
		// Assert no additional logging events
		assertEquals(uiEventLogger.numLogs(), 4)

		// Simulate a round of appOps where location is disabled so that the indicator disappears.
		doReturn(listOf<AppOpItem>()).`when`(appOpsController).getActiveAppOps(anyBoolean())
		result = appOpsPrivacyItemMonitor.getActivePrivacyItems()
		assertEquals(result.size, 0)
		// Assert no additional logging events
		assertEquals(uiEventLogger.numLogs(), 4)

		// Simulate a round of appOps where the location indicator appears again and the logging
		// count increases.
		doReturn(
		listOf(
		// Regular item which should not be filtered
		AppOpItem(
		AppOpsManager.OP_COARSE_LOCATION,
		TEST_UID,
		"com.google.android.apps.maps",
		0,
		)
		)
		)
		.`when`(appOpsController)
		.getActiveAppOps(anyBoolean())
		result = appOpsPrivacyItemMonitor.getActivePrivacyItems()
		assertEquals(result.size, 1)
		assertEquals(result[0].application.packageName, "com.google.android.apps.maps")
		// Assert there are additional logging events
		assertEquals(uiEventLogger.numLogs(), 8)
		Truth.assertThat(
		uiEventLogger.logs.count { log ->
		log.eventId ==
		AppOpsPrivacyItemMonitor.LocationIndicatorEvent
		.LOCATION_INDICATOR_NON_SYSTEM_APP
		.id
		}
		)
		.isEqualTo(2)
		}

		@Test
		@@ -335,6 +422,24 @@ class AppOpsPrivacyItemMonitorTest : SysuiTestCase() {
		.getActiveAppOps(anyBoolean())

		assertEquals(appOpsPrivacyItemMonitor.getActivePrivacyItems().size, 0)
		assertEquals(uiEventLogger.numLogs(), 2)
		Truth.assertThat(
		uiEventLogger.logs.any { log ->
		log.eventId ==
		AppOpsPrivacyItemMonitor.LocationIndicatorEvent
		.LOCATION_INDICATOR_SYSTEM_APP
		.id
		}
		)
		.isTrue()
		Truth.assertThat(
		uiEventLogger.logs.any { log ->
		log.eventId ==
		AppOpsPrivacyItemMonitor.LocationIndicatorEvent.LOCATION_INDICATOR_ALL_APP
		.id
		}
		)
		.isTrue()
		}

		@Test
		@@ -369,6 +474,24 @@ class AppOpsPrivacyItemMonitorTest : SysuiTestCase() {
		.getActiveAppOps(anyBoolean())

		assertEquals(appOpsPrivacyItemMonitor.getActivePrivacyItems().size, 0)
		assertEquals(uiEventLogger.numLogs(), 2)
		Truth.assertThat(
		uiEventLogger.logs.any { log ->
		log.eventId ==
		AppOpsPrivacyItemMonitor.LocationIndicatorEvent
		.LOCATION_INDICATOR_BACKGROUND_APP
		.id
		}
		)
		.isTrue()
		Truth.assertThat(
		uiEventLogger.logs.any { log ->
		log.eventId ==
		AppOpsPrivacyItemMonitor.LocationIndicatorEvent.LOCATION_INDICATOR_ALL_APP
		.id
		}
		)
		.isTrue()
		}

		@Test

packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/LocationControllerImplTest.java

+0 −20

Original line number	Diff line number	Diff line
		@@ -15,8 +15,6 @@
		package com.android.systemui.statusbar.policy;


		import static com.google.common.truth.Truth.assertThat;

		import static org.mockito.ArgumentMatchers.any;
		import static org.mockito.ArgumentMatchers.anyBoolean;
		import static org.mockito.ArgumentMatchers.eq;
		@@ -176,10 +174,6 @@ 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
		@@ -323,19 +317,6 @@ public class LocationControllerImplTest extends SysuiTestCase {
		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,
		@@ -343,7 +324,6 @@ public class LocationControllerImplTest extends SysuiTestCase {
		mTestableLooper.processAllMessages();

		verify(callback, times(1)).onLocationActiveChanged(false);
		assertThat(mUiEventLogger.numLogs()).isEqualTo(0);
		}

		@Test