Add cache to AppOpsController
Reduces the Binder calls from AppOpsControllerImpl to
PackageManager#getPermissionFlags by caching calls for 10s (moltmann@
recommended this expiration time).
Also, modify AppOpsControllerImpl#notifySuscribers to use the bg Thread.
Currently calls to getPermissionFlags only happen in two functions:
* AppOpsControllerImpl#notifySuscribers
* AppOpsControllerImpl#getActiveAppOpsForUser
First one in only called from bg thread and second one is called from a
bg thread from PrivacyItemController.
In a future CL, this calling will be enforced at the entry point
(getActiveAppOpsForUser)
Test: atest PermissionFlagsCacheTest AppOpsControllerTest
Bug: 134687592
Change-Id: I6fa9e2d865bc295f6325935e2df574b8b758214a
diff --git a/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java b/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java
index 49bd5bd..858ed6d 100644
--- a/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java
@@ -62,6 +62,8 @@
private H mBGHandler;
private final List<AppOpsController.Callback> mCallbacks = new ArrayList<>();
private final ArrayMap<Integer, Set<Callback>> mCallbacksByCode = new ArrayMap<>();
+ private final PermissionFlagsCache mFlagsCache;
+ private boolean mListening;
@GuardedBy("mActiveItems")
private final List<AppOpItem> mActiveItems = new ArrayList<>();
@@ -78,8 +80,14 @@
@Inject
public AppOpsControllerImpl(Context context, @Named(BG_LOOPER_NAME) Looper bgLooper) {
+ this(context, bgLooper, new PermissionFlagsCache(context));
+ }
+
+ @VisibleForTesting
+ protected AppOpsControllerImpl(Context context, Looper bgLooper, PermissionFlagsCache cache) {
mContext = context;
mAppOps = (AppOpsManager) context.getSystemService(Context.APP_OPS_SERVICE);
+ mFlagsCache = cache;
mBGHandler = new H(bgLooper);
final int numOps = OPS.length;
for (int i = 0; i < numOps; i++) {
@@ -94,6 +102,7 @@
@VisibleForTesting
protected void setListening(boolean listening) {
+ mListening = listening;
if (listening) {
mAppOps.startWatchingActive(OPS, this);
mAppOps.startWatchingNoted(OPS, this);
@@ -225,7 +234,7 @@
if (permission == null) {
return false;
}
- int permFlags = mContext.getPackageManager().getPermissionFlags(permission,
+ int permFlags = mFlagsCache.getPermissionFlags(permission,
packageName, UserHandle.getUserHandleForUid(uid));
return (permFlags & PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_GRANTED) != 0;
}
@@ -308,7 +317,7 @@
@Override
public void onOpActiveChanged(int code, int uid, String packageName, boolean active) {
if (updateActives(code, uid, packageName, active)) {
- notifySuscribers(code, uid, packageName, active);
+ mBGHandler.post(() -> notifySuscribers(code, uid, packageName, active));
}
}
@@ -319,7 +328,7 @@
}
if (result != AppOpsManager.MODE_ALLOWED) return;
addNoted(code, uid, packageName);
- notifySuscribers(code, uid, packageName, true);
+ mBGHandler.post(() -> notifySuscribers(code, uid, packageName, true));
}
private void notifySuscribers(int code, int uid, String packageName, boolean active) {
@@ -334,6 +343,7 @@
@Override
public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
pw.println("AppOpsController state:");
+ pw.println(" Listening: " + mListening);
pw.println(" Active Items:");
for (int i = 0; i < mActiveItems.size(); i++) {
final AppOpItem item = mActiveItems.get(i);
diff --git a/packages/SystemUI/src/com/android/systemui/appops/PermissionFlagsCache.kt b/packages/SystemUI/src/com/android/systemui/appops/PermissionFlagsCache.kt
new file mode 100644
index 0000000..f02c7af
--- /dev/null
+++ b/packages/SystemUI/src/com/android/systemui/appops/PermissionFlagsCache.kt
@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.systemui.appops
+
+import android.content.Context
+import android.content.pm.PackageManager
+import android.os.UserHandle
+import android.util.ArrayMap
+import com.android.internal.annotations.VisibleForTesting
+
+private data class PermissionFlag(val flag: Int, val timestamp: Long)
+
+private data class PermissionFlagKey(
+ val permission: String,
+ val packageName: String,
+ val user: UserHandle
+)
+
+internal const val CACHE_EXPIRATION = 10000L
+
+/**
+ * Cache for PackageManager's PermissionFlags.
+ *
+ * Flags older than [CACHE_EXPIRATION] will be retrieved again.
+ */
+internal open class PermissionFlagsCache(context: Context) {
+ private val packageManager = context.packageManager
+ private val permissionFlagsCache = ArrayMap<PermissionFlagKey, PermissionFlag>()
+
+ /**
+ * Retrieve permission flags from cache or PackageManager. There parameters will be passed
+ * directly to [PackageManager].
+ *
+ * Calls to this method should be done from a background thread.
+ */
+ fun getPermissionFlags(permission: String, packageName: String, user: UserHandle): Int {
+ val key = PermissionFlagKey(permission, packageName, user)
+ val now = getCurrentTime()
+ val value = permissionFlagsCache.getOrPut(key) {
+ PermissionFlag(getFlags(key), now)
+ }
+ if (now - value.timestamp > CACHE_EXPIRATION) {
+ val newValue = PermissionFlag(getFlags(key), now)
+ permissionFlagsCache.put(key, newValue)
+ return newValue.flag
+ } else {
+ return value.flag
+ }
+ }
+
+ private fun getFlags(key: PermissionFlagKey) =
+ packageManager.getPermissionFlags(key.permission, key.packageName, key.user)
+
+ @VisibleForTesting
+ protected open fun getCurrentTime() = System.currentTimeMillis()
+}
\ No newline at end of file
diff --git a/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java
index cc31531..540ac84 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java
@@ -39,7 +39,6 @@
import androidx.test.filters.SmallTest;
-import com.android.systemui.Dependency;
import com.android.systemui.SysuiTestCase;
import org.junit.Before;
@@ -65,28 +64,32 @@
private AppOpsController.Callback mCallback;
@Mock
private AppOpsControllerImpl.H mMockHandler;
+ @Mock
+ private PermissionFlagsCache mFlagsCache;
private AppOpsControllerImpl mController;
+ private TestableLooper mTestableLooper;
@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
+ mTestableLooper = TestableLooper.get(this);
getContext().addMockSystemService(AppOpsManager.class, mAppOpsManager);
// All permissions of TEST_UID and TEST_UID_OTHER are user sensitive. None of
// TEST_UID_NON_USER_SENSITIVE are user sensitive.
getContext().setMockPackageManager(mPackageManager);
- when(mPackageManager.getPermissionFlags(anyString(), anyString(),
+ when(mFlagsCache.getPermissionFlags(anyString(), anyString(),
eq(UserHandle.getUserHandleForUid(TEST_UID)))).thenReturn(
PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_GRANTED);
- when(mPackageManager.getPermissionFlags(anyString(), anyString(),
+ when(mFlagsCache.getPermissionFlags(anyString(), anyString(),
eq(UserHandle.getUserHandleForUid(TEST_UID_OTHER)))).thenReturn(
PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_GRANTED);
- when(mPackageManager.getPermissionFlags(anyString(), anyString(),
+ when(mFlagsCache.getPermissionFlags(anyString(), anyString(),
eq(UserHandle.getUserHandleForUid(TEST_UID_NON_USER_SENSITIVE)))).thenReturn(0);
- mController = new AppOpsControllerImpl(mContext, Dependency.get(Dependency.BG_LOOPER));
+ mController = new AppOpsControllerImpl(mContext, mTestableLooper.getLooper(), mFlagsCache);
}
@Test
@@ -110,6 +113,7 @@
AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true);
mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME,
AppOpsManager.MODE_ALLOWED);
+ mTestableLooper.processAllMessages();
verify(mCallback).onActiveStateChanged(AppOpsManager.OP_RECORD_AUDIO,
TEST_UID, TEST_PACKAGE_NAME, true);
}
@@ -119,6 +123,7 @@
mController.addCallback(new int[]{AppOpsManager.OP_FINE_LOCATION}, mCallback);
mController.onOpActiveChanged(
AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true);
+ mTestableLooper.processAllMessages();
verify(mCallback, never()).onActiveStateChanged(
anyInt(), anyInt(), anyString(), anyBoolean());
}
@@ -129,6 +134,7 @@
mController.removeCallback(new int[]{AppOpsManager.OP_RECORD_AUDIO}, mCallback);
mController.onOpActiveChanged(
AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true);
+ mTestableLooper.processAllMessages();
verify(mCallback, never()).onActiveStateChanged(
anyInt(), anyInt(), anyString(), anyBoolean());
}
@@ -139,6 +145,7 @@
mController.removeCallback(new int[]{AppOpsManager.OP_CAMERA}, mCallback);
mController.onOpActiveChanged(
AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true);
+ mTestableLooper.processAllMessages();
verify(mCallback).onActiveStateChanged(AppOpsManager.OP_RECORD_AUDIO,
TEST_UID, TEST_PACKAGE_NAME, true);
}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/appops/PermissionFlagsCacheTest.kt b/packages/SystemUI/tests/src/com/android/systemui/appops/PermissionFlagsCacheTest.kt
new file mode 100644
index 0000000..dc070de
--- /dev/null
+++ b/packages/SystemUI/tests/src/com/android/systemui/appops/PermissionFlagsCacheTest.kt
@@ -0,0 +1,88 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.systemui.appops
+
+import android.content.Context
+import android.content.pm.PackageManager
+import android.os.UserHandle
+import android.testing.AndroidTestingRunner
+import androidx.test.filters.SmallTest
+import com.android.systemui.SysuiTestCase
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.ArgumentMatchers.any
+import org.mockito.ArgumentMatchers.anyString
+import org.mockito.Mock
+import org.mockito.Mockito.times
+import org.mockito.Mockito.verify
+import org.mockito.MockitoAnnotations
+
+@SmallTest
+@RunWith(AndroidTestingRunner::class)
+class PermissionFlagsCacheTest : SysuiTestCase() {
+
+ companion object {
+ const val TEST_PERMISSION = "test_permission"
+ const val TEST_PACKAGE = "test_package"
+ }
+
+ @Mock
+ private lateinit var mPackageManager: PackageManager
+ @Mock
+ private lateinit var mUserHandle: UserHandle
+ private lateinit var flagsCache: TestPermissionFlagsCache
+
+ @Before
+ fun setUp() {
+ MockitoAnnotations.initMocks(this)
+ mContext.setMockPackageManager(mPackageManager)
+ flagsCache = TestPermissionFlagsCache(mContext)
+ }
+
+ @Test
+ fun testCallsPackageManager_exactlyOnce() {
+ flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, mUserHandle)
+ flagsCache.time = CACHE_EXPIRATION - 1
+ verify(mPackageManager).getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, mUserHandle)
+ }
+
+ @Test
+ fun testCallsPackageManager_cacheExpired() {
+ flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, mUserHandle)
+ flagsCache.time = CACHE_EXPIRATION + 1
+ flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, mUserHandle)
+ verify(mPackageManager, times(2))
+ .getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, mUserHandle)
+ }
+
+ @Test
+ fun testCallsPackageMaanger_multipleKeys() {
+ flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, mUserHandle)
+ flagsCache.getPermissionFlags(TEST_PERMISSION, "", mUserHandle)
+ verify(mPackageManager, times(2))
+ .getPermissionFlags(anyString(), anyString(), any())
+ }
+
+ private class TestPermissionFlagsCache(context: Context) : PermissionFlagsCache(context) {
+ var time = 0L
+
+ override fun getCurrentTime(): Long {
+ return time
+ }
+ }
+}
\ No newline at end of file
