From fb3ceeb0b1512d52c55b475152e3513bfd6de63b Mon Sep 17 00:00:00 2001 From: Rhed Jao Date: Thu, 14 Oct 2021 11:37:46 +0000 Subject: [PATCH 1/4] Revert "Revert "Only update cache for new user on user create"" This reverts commit 6eecee9b7f4f5f757cde55cee28882f7c53c8905. Reason for revert: 187853334 Change-Id: If0e929e1e0f988409c640ee02a84d1c8750b142e --- .../com/android/server/pm/AppsFilter.java | 95 +++++++++++++------ .../server/pm/PackageManagerService.java | 2 +- .../com/android/server/pm/AppsFilterTest.java | 53 ++++++++++- 3 files changed, 119 insertions(+), 31 deletions(-) diff --git a/services/core/java/com/android/server/pm/AppsFilter.java b/services/core/java/com/android/server/pm/AppsFilter.java index 1e8e43d7a9a4a..9f443ddd58fbe 100644 --- a/services/core/java/com/android/server/pm/AppsFilter.java +++ b/services/core/java/com/android/server/pm/AppsFilter.java @@ -17,6 +17,7 @@ package com.android.server.pm; import static android.os.Trace.TRACE_TAG_PACKAGE_MANAGER; +import static android.os.UserHandle.USER_ALL; import static android.provider.DeviceConfig.NAMESPACE_PACKAGE_MANAGER_SERVICE; import static com.android.internal.annotations.VisibleForTesting.Visibility.PRIVATE; @@ -701,7 +702,7 @@ public class AppsFilter implements Watchable, Snappable { synchronized (mCacheLock) { if (mShouldFilterCache != null) { updateShouldFilterCacheForPackage(mShouldFilterCache, null, newPkgSetting, - settings, users, settings.size()); + settings, users, USER_ALL, settings.size()); if (additionalChangedPackages != null) { for (int index = 0; index < additionalChangedPackages.size(); index++) { String changedPackage = additionalChangedPackages.valueAt(index); @@ -714,7 +715,8 @@ public class AppsFilter implements Watchable, Snappable { } updateShouldFilterCacheForPackage(mShouldFilterCache, null, - changedPkgSetting, settings, users, settings.size()); + changedPkgSetting, settings, users, USER_ALL, + settings.size()); } } } // else, rebuild entire cache when system is ready @@ -851,22 +853,50 @@ public class AppsFilter implements Watchable, Snappable { } private void updateEntireShouldFilterCache() { + updateEntireShouldFilterCache(USER_ALL); + } + + private void updateEntireShouldFilterCache(int subjectUserId) { mStateProvider.runWithState((settings, users) -> { + int userId = subjectUserId; + if (!ArrayUtils.contains(users, subjectUserId)) { + Slog.e(TAG, "We encountered a new user that isn't a member of known users, " + + "updating the whole cache"); + userId = USER_ALL; + } WatchedSparseBooleanMatrix cache = - updateEntireShouldFilterCacheInner(settings, users); + updateEntireShouldFilterCacheInner(settings, users, userId); synchronized (mCacheLock) { + if (userId != USER_ALL) { + // if we're only updating a single user id, we need to copy over the prior + // cached values for the other users. + int[] uids = mShouldFilterCache.keys(); + for (int i = 0; i < uids.length; i++) { + int uid1 = uids[i]; + if (UserHandle.getUserId(uid1) == userId) { + continue; + } + for (int j = 0; j < uids.length; j++) { + int uid2 = uids[j]; + if (UserHandle.getUserId(uid2) == userId) { + continue; + } + cache.setValueAt(uid1, uid2, mShouldFilterCache.valueAt(uid1, uid2)); + } + } + } mShouldFilterCache = cache; } }); } private WatchedSparseBooleanMatrix updateEntireShouldFilterCacheInner( - ArrayMap settings, UserInfo[] users) { + ArrayMap settings, UserInfo[] users, int subjectUserId) { WatchedSparseBooleanMatrix cache = new WatchedSparseBooleanMatrix(users.length * settings.size()); for (int i = settings.size() - 1; i >= 0; i--) { updateShouldFilterCacheForPackage(cache, - null /*skipPackage*/, settings.valueAt(i), settings, users, i); + null /*skipPackage*/, settings.valueAt(i), settings, users, subjectUserId, i); } return cache; } @@ -887,8 +917,8 @@ public class AppsFilter implements Watchable, Snappable { packagesCache.put(settings.keyAt(i), pkg); } }); - WatchedSparseBooleanMatrix cache = - updateEntireShouldFilterCacheInner(settingsCopy, usersRef[0]); + WatchedSparseBooleanMatrix cache = updateEntireShouldFilterCacheInner( + settingsCopy, usersRef[0], USER_ALL); boolean[] changed = new boolean[1]; // We have a cache, let's make sure the world hasn't changed out from under us. mStateProvider.runWithState((settings, users) -> { @@ -918,10 +948,10 @@ public class AppsFilter implements Watchable, Snappable { }); } - public void onUsersChanged() { + public void onUserCreated(int newUserId) { synchronized (mCacheLock) { if (mShouldFilterCache != null) { - updateEntireShouldFilterCache(); + updateEntireShouldFilterCache(newUserId); onChanged(); } } @@ -934,7 +964,7 @@ public class AppsFilter implements Watchable, Snappable { return; } updateShouldFilterCacheForPackage(mShouldFilterCache, null /* skipPackage */, - settings.get(packageName), settings, users, + settings.get(packageName), settings, users, USER_ALL, settings.size() /*maxIndex*/); } }); @@ -942,7 +972,7 @@ public class AppsFilter implements Watchable, Snappable { private void updateShouldFilterCacheForPackage(WatchedSparseBooleanMatrix cache, @Nullable String skipPackageName, PackageSetting subjectSetting, ArrayMap allSettings, UserInfo[] allUsers, int maxIndex) { + PackageSetting> allSettings, UserInfo[] allUsers, int subjectUserId, int maxIndex) { for (int i = Math.min(maxIndex, allSettings.size() - 1); i >= 0; i--) { PackageSetting otherSetting = allSettings.valueAt(i); if (subjectSetting.getAppId() == otherSetting.getAppId()) { @@ -953,25 +983,34 @@ public class AppsFilter implements Watchable, Snappable { == skipPackageName) { continue; } - final int userCount = allUsers.length; - final int appxUidCount = userCount * allSettings.size(); - for (int su = 0; su < userCount; su++) { - int subjectUser = allUsers[su].id; - for (int ou = 0; ou < userCount; ou++) { - int otherUser = allUsers[ou].id; - int subjectUid = UserHandle.getUid(subjectUser, subjectSetting.getAppId()); - int otherUid = UserHandle.getUid(otherUser, otherSetting.getAppId()); - cache.put(subjectUid, otherUid, - shouldFilterApplicationInternal( - subjectUid, subjectSetting, otherSetting, otherUser)); - cache.put(otherUid, subjectUid, - shouldFilterApplicationInternal( - otherUid, otherSetting, subjectSetting, subjectUser)); + if (subjectUserId == USER_ALL) { + for (int su = 0; su < allUsers.length; su++) { + updateShouldFilterCacheForUser(cache, subjectSetting, allUsers, otherSetting, + allUsers[su].id); } + } else { + updateShouldFilterCacheForUser(cache, subjectSetting, allUsers, otherSetting, + subjectUserId); } } } + private void updateShouldFilterCacheForUser(WatchedSparseBooleanMatrix cache, + PackageSetting subjectSetting, UserInfo[] allUsers, PackageSetting otherSetting, + int subjectUserId) { + for (int ou = 0; ou < allUsers.length; ou++) { + int otherUser = allUsers[ou].id; + int subjectUid = UserHandle.getUid(subjectUserId, subjectSetting.getAppId()); + int otherUid = UserHandle.getUid(otherUser, otherSetting.getAppId()); + cache.put(subjectUid, otherUid, + shouldFilterApplicationInternal( + subjectUid, subjectSetting, otherSetting, otherUser)); + cache.put(otherUid, subjectUid, + shouldFilterApplicationInternal( + otherUid, otherSetting, subjectSetting, subjectUserId)); + } + } + private static boolean isSystemSigned(@NonNull SigningDetails sysSigningDetails, PackageSetting pkgSetting) { return pkgSetting.isSystem() @@ -1181,8 +1220,8 @@ public class AppsFilter implements Watchable, Snappable { continue; } updateShouldFilterCacheForPackage(mShouldFilterCache, - setting.getPackageName(), - siblingSetting, settings, users, settings.size()); + setting.getPackageName(), siblingSetting, settings, users, + USER_ALL, settings.size()); } } @@ -1199,7 +1238,7 @@ public class AppsFilter implements Watchable, Snappable { } updateShouldFilterCacheForPackage(mShouldFilterCache, null, - changedPkgSetting, settings, users, settings.size()); + changedPkgSetting, settings, users, USER_ALL, settings.size()); } } } diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 66320fe11e0b7..0b2fe3d4337d2 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -9300,7 +9300,7 @@ public class PackageManagerService extends IPackageManager.Stub synchronized (mLock) { scheduleWritePackageRestrictionsLocked(userId); scheduleWritePackageListLocked(userId); - mAppsFilter.onUsersChanged(); + mAppsFilter.onUserCreated(userId); } } diff --git a/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java b/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java index 76f233cd0fbbd..db1082faa92e4 100644 --- a/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java @@ -82,9 +82,17 @@ public class AppsFilterTest { private static final int DUMMY_OVERLAY_APPID = 10756; private static final int SYSTEM_USER = 0; private static final int SECONDARY_USER = 10; + private static final int ADDED_USER = 11; private static final int[] USER_ARRAY = {SYSTEM_USER, SECONDARY_USER}; - private static final UserInfo[] USER_INFO_LIST = Arrays.stream(USER_ARRAY).mapToObj( - id -> new UserInfo(id, Integer.toString(id), 0)).toArray(UserInfo[]::new); + private static final int[] USER_ARRAY_WITH_ADDED = {SYSTEM_USER, SECONDARY_USER, ADDED_USER}; + private static final UserInfo[] USER_INFO_LIST = toUserInfos(USER_ARRAY); + private static final UserInfo[] USER_INFO_LIST_WITH_ADDED = toUserInfos(USER_ARRAY_WITH_ADDED); + + private static UserInfo[] toUserInfos(int[] userIds) { + return Arrays.stream(userIds) + .mapToObj(id -> new UserInfo(id, Integer.toString(id), 0)) + .toArray(UserInfo[]::new); + } @Mock AppsFilter.FeatureConfig mFeatureConfigMock; @@ -317,6 +325,47 @@ public class AppsFilterTest { watcher.verifyNoChangeReported("shouldFilterApplication"); } + @Test + public void testOnUserCreated_FilterMatches() throws Exception { + final AppsFilter appsFilter = + new AppsFilter(mStateProvider, mFeatureConfigMock, new String[]{}, false, null, + mMockExecutor); + simulateAddBasicAndroid(appsFilter); + + appsFilter.onSystemReady(); + + PackageSetting target = simulateAddPackage(appsFilter, + pkgWithProvider("com.some.package", "com.some.authority"), DUMMY_TARGET_APPID); + PackageSetting calling = simulateAddPackage(appsFilter, + pkgQueriesProvider("com.some.other.package", "com.some.authority"), + DUMMY_CALLING_APPID); + + for (int subjectUserId : USER_ARRAY) { + for (int otherUserId : USER_ARRAY) { + assertFalse(appsFilter.shouldFilterApplication( + UserHandle.getUid(DUMMY_CALLING_APPID, subjectUserId), calling, target, + otherUserId)); + } + } + + // adds new user + doAnswer(invocation -> { + ((AppsFilter.StateProvider.CurrentStateCallback) invocation.getArgument(0)) + .currentState(mExisting, USER_INFO_LIST_WITH_ADDED); + return new Object(); + }).when(mStateProvider) + .runWithState(any(AppsFilter.StateProvider.CurrentStateCallback.class)); + appsFilter.onUserCreated(ADDED_USER); + + for (int subjectUserId : USER_ARRAY_WITH_ADDED) { + for (int otherUserId : USER_ARRAY_WITH_ADDED) { + assertFalse(appsFilter.shouldFilterApplication( + UserHandle.getUid(DUMMY_CALLING_APPID, subjectUserId), calling, target, + otherUserId)); + } + } + } + @Test public void testQueriesDifferentProvider_Filters() throws Exception { final AppsFilter appsFilter = From 72e972d5c2eb02d1d1ade4a03b6d6e23780836b5 Mon Sep 17 00:00:00 2001 From: Rhed Jao Date: Thu, 14 Oct 2021 12:02:52 +0000 Subject: [PATCH 2/4] Revert "Revert "Fixes incorrect array type comparison"" This reverts commit bf3e37f712e55c28bc610a12694e86aa45d5af77. Reason for revert: 187853334 Change-Id: I21071979d056486738c24dde0bccdf5971992f24 --- .../core/java/com/android/server/pm/AppsFilter.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/pm/AppsFilter.java b/services/core/java/com/android/server/pm/AppsFilter.java index 9f443ddd58fbe..afb91dad700c2 100644 --- a/services/core/java/com/android/server/pm/AppsFilter.java +++ b/services/core/java/com/android/server/pm/AppsFilter.java @@ -18,6 +18,7 @@ package com.android.server.pm; import static android.os.Trace.TRACE_TAG_PACKAGE_MANAGER; import static android.os.UserHandle.USER_ALL; +import static android.os.UserHandle.USER_NULL; import static android.provider.DeviceConfig.NAMESPACE_PACKAGE_MANAGER_SERVICE; import static com.android.internal.annotations.VisibleForTesting.Visibility.PRIVATE; @@ -858,8 +859,14 @@ public class AppsFilter implements Watchable, Snappable { private void updateEntireShouldFilterCache(int subjectUserId) { mStateProvider.runWithState((settings, users) -> { - int userId = subjectUserId; - if (!ArrayUtils.contains(users, subjectUserId)) { + int userId = USER_NULL; + for (int u = 0; u < users.length; u++) { + if (subjectUserId == users[u].id) { + userId = subjectUserId; + break; + } + } + if (userId == USER_NULL) { Slog.e(TAG, "We encountered a new user that isn't a member of known users, " + "updating the whole cache"); userId = USER_ALL; @@ -881,7 +888,7 @@ public class AppsFilter implements Watchable, Snappable { if (UserHandle.getUserId(uid2) == userId) { continue; } - cache.setValueAt(uid1, uid2, mShouldFilterCache.valueAt(uid1, uid2)); + cache.put(uid1, uid2, mShouldFilterCache.get(uid1, uid2)); } } } From dc4d343c3c15716605695e7cd7cf07491af70851 Mon Sep 17 00:00:00 2001 From: Rhed Jao Date: Wed, 13 Oct 2021 17:49:36 +0800 Subject: [PATCH 3/4] Add setCapacity and removeRange to WatchedSparseBooleanMatrix - This CL adds #setCapacity and #removeRange APIs to WatchedSparseBooleanMatrix for AppsFilter to update filter cache more efficiently when a user is created or deleted. - Fix a crash in the #pack function when the caller invokes #compat to shrink the matrix. Test: atest WatcherTest Bug: 187853334 Change-Id: I6a3c87a7492793767f888487381182955d564d62 --- .../utils/WatchedSparseBooleanMatrix.java | 52 +++++++++++-- .../com/android/server/utils/WatcherTest.java | 75 +++++++++++++++++++ 2 files changed, 122 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/utils/WatchedSparseBooleanMatrix.java b/services/core/java/com/android/server/utils/WatchedSparseBooleanMatrix.java index 9b0ef15c8bd69..25ae00004b3e1 100644 --- a/services/core/java/com/android/server/utils/WatchedSparseBooleanMatrix.java +++ b/services/core/java/com/android/server/utils/WatchedSparseBooleanMatrix.java @@ -262,6 +262,33 @@ public class WatchedSparseBooleanMatrix extends WatchableImpl implements Snappab onChanged(); } + /** + * Removes all of the mappings whose index is between {@code fromIndex}, inclusive, and + * {@code toIndex}, exclusive. The matrix does not shrink. + */ + public void removeRange(int fromIndex, int toIndex) { + if (toIndex < fromIndex) { + throw new ArrayIndexOutOfBoundsException("toIndex < fromIndex"); + } + final int num = toIndex - fromIndex; + if (num == 0) { + return; + } + validateIndex(fromIndex); + validateIndex(toIndex - 1); + for (int i = fromIndex; i < toIndex; i++) { + mInUse[mMap[i]] = false; + } + System.arraycopy(mKeys, toIndex, mKeys, fromIndex, mSize - toIndex); + System.arraycopy(mMap, toIndex, mMap, fromIndex, mSize - toIndex); + for (int i = mSize - num; i < mSize; i++) { + mKeys[i] = 0; + mMap[i] = 0; + } + mSize -= num; + onChanged(); + } + /** * Returns the number of key-value mappings that this WatchedSparseBooleanMatrix * currently stores. @@ -371,7 +398,7 @@ public class WatchedSparseBooleanMatrix extends WatchableImpl implements Snappab // Preemptively grow the matrix, which also grows the free list. growMatrix(); } - int newIndex = nextFree(); + int newIndex = nextFree(true /* acquire */); mKeys = GrowingArrayUtils.insert(mKeys, mSize, i, key); mMap = GrowingArrayUtils.insert(mMap, mSize, i, newIndex); mSize++; @@ -447,12 +474,12 @@ public class WatchedSparseBooleanMatrix extends WatchableImpl implements Snappab } /** - * Find an unused storage index, mark it in-use, and return it. + * Find an unused storage index, and return it. Mark it in-use if the {@code acquire} is true. */ - private int nextFree() { + private int nextFree(boolean acquire) { for (int i = 0; i < mInUse.length; i++) { if (!mInUse[i]) { - mInUse[i] = true; + mInUse[i] = acquire; return i; } } @@ -488,7 +515,8 @@ public class WatchedSparseBooleanMatrix extends WatchableImpl implements Snappab } // dst and src are identify raw (row, col) in mValues. srcIndex is the index (as // in the result of keyAt()) of the key being relocated. - for (int dst = nextFree(); dst < mSize; dst = nextFree()) { + for (int dst = nextFree(false); dst < mSize; dst = nextFree(false)) { + mInUse[dst] = true; int srcIndex = lastInuse(); int src = mMap[srcIndex]; mInUse[src] = false; @@ -538,6 +566,20 @@ public class WatchedSparseBooleanMatrix extends WatchableImpl implements Snappab return mOrder; } + /** + * Set capacity to enlarge the size of the 2D matrix. Capacity less than the {@link #capacity()} + * is not supported. + */ + public void setCapacity(int capacity) { + if (capacity <= mOrder) { + return; + } + if (capacity % STEP != 0) { + capacity = ((capacity / STEP) + 1) * STEP; + } + resizeMatrix(capacity); + } + /** * {@inheritDoc} */ diff --git a/services/tests/servicestests/src/com/android/server/utils/WatcherTest.java b/services/tests/servicestests/src/com/android/server/utils/WatcherTest.java index f361f4a8bb5c0..4ed4c236535fc 100644 --- a/services/tests/servicestests/src/com/android/server/utils/WatcherTest.java +++ b/services/tests/servicestests/src/com/android/server/utils/WatcherTest.java @@ -915,6 +915,23 @@ public class WatcherTest { } } + // Fill new cells in the matrix which has enlarged capacity. + private void fillNew(WatchedSparseBooleanMatrix matrix, int initialCapacity, + int newCapacity, int[] indexes) { + final int size = newCapacity; + for (int i = 0; i < size; i++) { + for (int j = 0; j < size; j++) { + if (i < initialCapacity && j < initialCapacity) { + // Do not touch old cells + continue; + } + final int row = indexes[i]; + final int col = indexes[j]; + matrix.put(row, col, cellValue(i, j)); + } + } + } + // Verify the content of a matrix. This asserts on mismatch. Selected indices may // have been deleted. private void verify(WatchedSparseBooleanMatrix matrix, int[] indexes, boolean[] absent) { @@ -989,6 +1006,24 @@ public class WatcherTest { assertTrue("Matrix shrink", finalCapacity - matrix.size() < matrix.STEP); } + private void matrixSetCapacity(WatchedSparseBooleanMatrix matrix, int newCapacity, + IndexGenerator indexer) { + final int initialCapacity = matrix.capacity(); + final int[] indexes = indexer.indexes(Math.max(initialCapacity, newCapacity)); + fill(matrix, initialCapacity, indexes); + + matrix.setCapacity(newCapacity); + fillNew(matrix, initialCapacity, newCapacity, indexes); + + assertEquals(matrix.size(), indexes.length); + verify(matrix, indexes, null); + // Test the keyAt/indexOfKey methods + for (int i = 0; i < matrix.size(); i++) { + int key = indexes[i]; + assertEquals(matrix.keyAt(matrix.indexOfKey(key)), key); + } + } + @Test public void testWatchedSparseBooleanMatrix() { final String name = "WatchedSparseBooleanMatrix"; @@ -1051,6 +1086,46 @@ public class WatcherTest { assertEquals(a.equals(s), false); } + @Test + public void testWatchedSparseBooleanMatrix_setCapacity() { + final IndexGenerator indexer = new IndexGenerator(3); + matrixSetCapacity(new WatchedSparseBooleanMatrix(500), 1000, indexer); + matrixSetCapacity(new WatchedSparseBooleanMatrix(1000), 500, indexer); + } + + @Test + public void testWatchedSparseBooleanMatrix_removeRangeAndShrink() { + final IndexGenerator indexer = new IndexGenerator(3); + final int initialCapacity = 500; + final int removeCounts = 33; + final WatchedSparseBooleanMatrix matrix = new WatchedSparseBooleanMatrix(initialCapacity); + final int[] indexes = indexer.indexes(initialCapacity); + final boolean[] absents = new boolean[initialCapacity]; + fill(matrix, initialCapacity, indexes); + assertEquals(matrix.size(), initialCapacity); + + for (int i = 0; i < initialCapacity / removeCounts; i++) { + final int size = matrix.size(); + final int fromIndex = (size / 2 < removeCounts ? 0 : size / 2 - removeCounts); + final int toIndex = (fromIndex + removeCounts > size ? size : fromIndex + removeCounts); + for (int index = fromIndex; index < toIndex; index++) { + final int key = matrix.keyAt(index); + for (int j = 0; j < indexes.length; j++) { + if (key == indexes[j]) { + absents[j] = true; + break; + } + } + } + matrix.removeRange(fromIndex, toIndex); + assertEquals(matrix.size(), size - (toIndex - fromIndex)); + verify(matrix, indexes, absents); + + matrix.compact(); + verify(matrix, indexes, absents); + } + } + @Test public void testNestedArrays() { final String name = "NestedArrays"; From d78fd7bf507d4d3606b71e52c1525a9ccd632631 Mon Sep 17 00:00:00 2001 From: Rhed Jao Date: Thu, 14 Oct 2021 19:00:57 +0800 Subject: [PATCH 4/4] Only update cache for new user on user create #2 - Snapshots whole cache and enlarge the capacity to accommodate the new user. Removes the logic of copying over the prior cached values for the other users to improve the user creation performance. This CL focus on the improvements of the creation of multiple users such as the fourth, fifth or sixth user. No obvious improvement for the first user creation. The execution time of AppsFilter#onUserCreated got 60% improved in the creation of sixth user compared with rebuilding whole cache. - Handle the user delete event to remove the cached values of the invalid user. Test: atest --user-type secondary_user AppsFilterTest Test: atest --user-type secondary_user AppEnumerationTests Test: [create 2nd user; run as primary] atest AppsFilterTest AppEnumerationTests Test: atest UserLifecycleTests#createUser Bug: 187853334 Change-Id: Iab93083509093ac3225adf0df466154f86e95bf2 --- .../com/android/server/pm/AppsFilter.java | 62 +++++++++++++------ .../server/pm/PackageManagerService.java | 1 + .../com/android/server/pm/AppsFilterTest.java | 19 +++++- 3 files changed, 61 insertions(+), 21 deletions(-) diff --git a/services/core/java/com/android/server/pm/AppsFilter.java b/services/core/java/com/android/server/pm/AppsFilter.java index afb91dad700c2..d3a5c6a2d1dfe 100644 --- a/services/core/java/com/android/server/pm/AppsFilter.java +++ b/services/core/java/com/android/server/pm/AppsFilter.java @@ -26,6 +26,7 @@ import static com.android.internal.annotations.VisibleForTesting.Visibility.PRIV import android.Manifest; import android.annotation.NonNull; import android.annotation.Nullable; +import android.annotation.UserIdInt; import android.content.Intent; import android.content.IntentFilter; import android.content.pm.PackageManager; @@ -874,24 +875,6 @@ public class AppsFilter implements Watchable, Snappable { WatchedSparseBooleanMatrix cache = updateEntireShouldFilterCacheInner(settings, users, userId); synchronized (mCacheLock) { - if (userId != USER_ALL) { - // if we're only updating a single user id, we need to copy over the prior - // cached values for the other users. - int[] uids = mShouldFilterCache.keys(); - for (int i = 0; i < uids.length; i++) { - int uid1 = uids[i]; - if (UserHandle.getUserId(uid1) == userId) { - continue; - } - for (int j = 0; j < uids.length; j++) { - int uid2 = uids[j]; - if (UserHandle.getUserId(uid2) == userId) { - continue; - } - cache.put(uid1, uid2, mShouldFilterCache.get(uid1, uid2)); - } - } - } mShouldFilterCache = cache; } }); @@ -899,8 +882,15 @@ public class AppsFilter implements Watchable, Snappable { private WatchedSparseBooleanMatrix updateEntireShouldFilterCacheInner( ArrayMap settings, UserInfo[] users, int subjectUserId) { - WatchedSparseBooleanMatrix cache = - new WatchedSparseBooleanMatrix(users.length * settings.size()); + final WatchedSparseBooleanMatrix cache; + if (subjectUserId == USER_ALL) { + cache = new WatchedSparseBooleanMatrix(users.length * settings.size()); + } else { + synchronized (mCacheLock) { + cache = mShouldFilterCache.snapshot(); + } + cache.setCapacity(users.length * settings.size()); + } for (int i = settings.size() - 1; i >= 0; i--) { updateShouldFilterCacheForPackage(cache, null /*skipPackage*/, settings.valueAt(i), settings, users, subjectUserId, i); @@ -964,6 +954,15 @@ public class AppsFilter implements Watchable, Snappable { } } + public void onUserDeleted(@UserIdInt int userId) { + synchronized (mCacheLock) { + if (mShouldFilterCache != null) { + removeShouldFilterCacheForUser(userId); + onChanged(); + } + } + } + private void updateShouldFilterCacheForPackage(String packageName) { mStateProvider.runWithState((settings, users) -> { synchronized (mCacheLock) { @@ -1018,6 +1017,29 @@ public class AppsFilter implements Watchable, Snappable { } } + @GuardedBy("mCacheLock") + private void removeShouldFilterCacheForUser(int userId) { + // Sorted uids with the ascending order + final int[] cacheUids = mShouldFilterCache.keys(); + final int size = cacheUids.length; + int pos = Arrays.binarySearch(cacheUids, UserHandle.getUid(userId, 0)); + final int fromIndex = (pos >= 0 ? pos : ~pos); + if (fromIndex >= size || UserHandle.getUserId(cacheUids[fromIndex]) != userId) { + Slog.w(TAG, "Failed to remove should filter cache for user " + userId + + ", fromIndex=" + fromIndex); + return; + } + pos = Arrays.binarySearch(cacheUids, UserHandle.getUid(userId + 1, 0) - 1); + final int toIndex = (pos >= 0 ? pos + 1 : ~pos); + if (fromIndex >= toIndex || UserHandle.getUserId(cacheUids[toIndex - 1]) != userId) { + Slog.w(TAG, "Failed to remove should filter cache for user " + userId + + ", fromIndex=" + fromIndex + ", toIndex=" + toIndex); + return; + } + mShouldFilterCache.removeRange(fromIndex, toIndex); + mShouldFilterCache.compact(); + } + private static boolean isSystemSigned(@NonNull SigningDetails sysSigningDetails, PackageSetting pkgSetting) { return pkgSetting.isSystem() diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 0b2fe3d4337d2..498485f5525c3 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -9279,6 +9279,7 @@ public class PackageManagerService extends IPackageManager.Stub mPendingBroadcasts.remove(userId); mInstantAppRegistry.onUserRemovedLPw(userId); mDeletePackageHelper.removeUnusedPackagesLPw(userManager, userId); + mAppsFilter.onUserDeleted(userId); } } diff --git a/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java b/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java index db1082faa92e4..b7161f8672d51 100644 --- a/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/AppsFilterTest.java @@ -326,7 +326,7 @@ public class AppsFilterTest { } @Test - public void testOnUserCreated_FilterMatches() throws Exception { + public void testOnUserUpdated_FilterMatches() throws Exception { final AppsFilter appsFilter = new AppsFilter(mStateProvider, mFeatureConfigMock, new String[]{}, false, null, mMockExecutor); @@ -364,6 +364,23 @@ public class AppsFilterTest { otherUserId)); } } + + // delete user + doAnswer(invocation -> { + ((AppsFilter.StateProvider.CurrentStateCallback) invocation.getArgument(0)) + .currentState(mExisting, USER_INFO_LIST); + return new Object(); + }).when(mStateProvider) + .runWithState(any(AppsFilter.StateProvider.CurrentStateCallback.class)); + appsFilter.onUserDeleted(ADDED_USER); + + for (int subjectUserId : USER_ARRAY) { + for (int otherUserId : USER_ARRAY) { + assertFalse(appsFilter.shouldFilterApplication( + UserHandle.getUid(DUMMY_CALLING_APPID, subjectUserId), calling, target, + otherUserId)); + } + } } @Test