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
This commit is contained in:
Fabian Kozynski
2019-06-07 13:07:27 -04:00
parent 8b19acac90
commit 7c86b633e2
4 changed files with 183 additions and 8 deletions

View File

@@ -62,6 +62,8 @@ public class AppOpsControllerImpl implements AppOpsController,
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 @@ public class AppOpsControllerImpl implements AppOpsController,
@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 @@ public class AppOpsControllerImpl implements AppOpsController,
@VisibleForTesting
protected void setListening(boolean listening) {
mListening = listening;
if (listening) {
mAppOps.startWatchingActive(OPS, this);
mAppOps.startWatchingNoted(OPS, this);
@@ -225,7 +234,7 @@ public class AppOpsControllerImpl implements AppOpsController,
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 @@ public class AppOpsControllerImpl implements AppOpsController,
@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 @@ public class AppOpsControllerImpl implements AppOpsController,
}
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 @@ public class AppOpsControllerImpl implements AppOpsController,
@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);

View File

@@ -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()
}

View File

@@ -39,7 +39,6 @@ import android.testing.TestableLooper;
import androidx.test.filters.SmallTest;
import com.android.systemui.Dependency;
import com.android.systemui.SysuiTestCase;
import org.junit.Before;
@@ -65,28 +64,32 @@ public class AppOpsControllerTest extends SysuiTestCase {
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 @@ public class AppOpsControllerTest extends SysuiTestCase {
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 @@ public class AppOpsControllerTest extends SysuiTestCase {
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 @@ public class AppOpsControllerTest extends SysuiTestCase {
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 @@ public class AppOpsControllerTest extends SysuiTestCase {
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);
}

View File

@@ -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
}
}
}