Merge changes from topic "pm_appsfilter_create_user_improve"

* changes:
  Only update cache for new user on user create #2
  Add setCapacity and removeRange to WatchedSparseBooleanMatrix
  Revert "Revert "Fixes incorrect array type comparison""
  Revert "Revert "Only update cache for new user on user create""
This commit is contained in:
Rhed Jao
2021-10-20 04:49:54 +00:00
committed by Android (Google) Code Review
5 changed files with 290 additions and 38 deletions

View File

@@ -17,6 +17,8 @@
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;
@@ -24,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;
@@ -701,7 +704,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 +717,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,9 +855,25 @@ public class AppsFilter implements Watchable, Snappable {
}
private void updateEntireShouldFilterCache() {
updateEntireShouldFilterCache(USER_ALL);
}
private void updateEntireShouldFilterCache(int subjectUserId) {
mStateProvider.runWithState((settings, users) -> {
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;
}
WatchedSparseBooleanMatrix cache =
updateEntireShouldFilterCacheInner(settings, users);
updateEntireShouldFilterCacheInner(settings, users, userId);
synchronized (mCacheLock) {
mShouldFilterCache = cache;
}
@@ -861,12 +881,19 @@ public class AppsFilter implements Watchable, Snappable {
}
private WatchedSparseBooleanMatrix updateEntireShouldFilterCacheInner(
ArrayMap<String, PackageSetting> settings, UserInfo[] users) {
WatchedSparseBooleanMatrix cache =
new WatchedSparseBooleanMatrix(users.length * settings.size());
ArrayMap<String, PackageSetting> settings, UserInfo[] users, int subjectUserId) {
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, i);
null /*skipPackage*/, settings.valueAt(i), settings, users, subjectUserId, i);
}
return cache;
}
@@ -887,8 +914,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 +945,19 @@ public class AppsFilter implements Watchable, Snappable {
});
}
public void onUsersChanged() {
public void onUserCreated(int newUserId) {
synchronized (mCacheLock) {
if (mShouldFilterCache != null) {
updateEntireShouldFilterCache();
updateEntireShouldFilterCache(newUserId);
onChanged();
}
}
}
public void onUserDeleted(@UserIdInt int userId) {
synchronized (mCacheLock) {
if (mShouldFilterCache != null) {
removeShouldFilterCacheForUser(userId);
onChanged();
}
}
@@ -934,7 +970,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 +978,7 @@ public class AppsFilter implements Watchable, Snappable {
private void updateShouldFilterCacheForPackage(WatchedSparseBooleanMatrix cache,
@Nullable String skipPackageName, PackageSetting subjectSetting, ArrayMap<String,
PackageSetting> 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 +989,57 @@ 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));
}
}
@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()
@@ -1181,8 +1249,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 +1267,7 @@ public class AppsFilter implements Watchable, Snappable {
}
updateShouldFilterCacheForPackage(mShouldFilterCache, null,
changedPkgSetting, settings, users, settings.size());
changedPkgSetting, settings, users, USER_ALL, settings.size());
}
}
}

View File

@@ -9279,6 +9279,7 @@ public class PackageManagerService extends IPackageManager.Stub
mPendingBroadcasts.remove(userId);
mInstantAppRegistry.onUserRemovedLPw(userId);
mDeletePackageHelper.removeUnusedPackagesLPw(userManager, userId);
mAppsFilter.onUserDeleted(userId);
}
}
@@ -9300,7 +9301,7 @@ public class PackageManagerService extends IPackageManager.Stub
synchronized (mLock) {
scheduleWritePackageRestrictionsLocked(userId);
scheduleWritePackageListLocked(userId);
mAppsFilter.onUsersChanged();
mAppsFilter.onUserCreated(userId);
}
}

View File

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

View File

@@ -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,64 @@ public class AppsFilterTest {
watcher.verifyNoChangeReported("shouldFilterApplication");
}
@Test
public void testOnUserUpdated_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));
}
}
// 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
public void testQueriesDifferentProvider_Filters() throws Exception {
final AppsFilter appsFilter =

View File

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