Merge "[pm] refactor mAppIds to be more readable" into tm-dev

services/core/java/com/android/server/pm/AppIdSettingMap.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2022 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.server.pm;
+
+import android.os.Process;
+
+import com.android.server.utils.WatchedSparseArray;
+
+/**
+ * A wrapper over {@link WatchedSparseArray} that tracks the current maximum App ID.
+ */
+public class AppIdSettingMap extends WatchedSparseArray<SettingBase> {
+    private int mCurrentMaxAppId;
+
+    @Override
+    public void put(int key, SettingBase value) {
+        if (key > mCurrentMaxAppId) {
+            mCurrentMaxAppId = key;
+        }
+        super.put(key, value);
+    }
+
+    @Override
+    public AppIdSettingMap snapshot() {
+        AppIdSettingMap l = new AppIdSettingMap();
+        snapshot(l, this);
+        return l;
+    }
+
+    /**
+     * @return the maximum of all the App IDs that have been added to the map. 0 if map is empty.
+     */
+    public int getCurrentMaxAppId() {
+        return mCurrentMaxAppId;
+    }
+
+    /**
+     * @return the next available App ID that has not been added to the map
+     */
+    public int getNextAvailableAppId() {
+        if (mCurrentMaxAppId == 0) {
+            // No app id has been added yet
+            return Process.FIRST_APPLICATION_UID;
+        } else {
+            return mCurrentMaxAppId + 1;
+        }
+    }
+}

services/core/java/com/android/server/pm/Settings.java

@@ -40,7 +40,6 @@ import android.content.Intent;
 import android.content.IntentFilter;
 import android.content.pm.ActivityInfo;
 import android.content.pm.ApplicationInfo;
-import android.content.pm.ComponentInfo;
 import android.content.pm.IntentFilterVerificationInfo;
 import android.content.pm.PackageInstaller;
 import android.content.pm.PackageManager;
@@ -113,11 +112,9 @@ import com.android.server.pm.permission.LegacyPermissionState.PermissionState;
 import com.android.server.pm.pkg.PackageStateInternal;
 import com.android.server.pm.pkg.PackageUserState;
 import com.android.server.pm.pkg.PackageUserStateInternal;
-import com.android.server.pm.pkg.PackageUserStateUtils;
 import com.android.server.pm.pkg.SuspendParams;
 import com.android.server.pm.pkg.component.ParsedComponent;
 import com.android.server.pm.pkg.component.ParsedIntentInfo;
-import com.android.server.pm.pkg.component.ParsedMainComponent;
 import com.android.server.pm.pkg.component.ParsedPermission;
 import com.android.server.pm.pkg.component.ParsedProcess;
 import com.android.server.pm.pkg.parsing.PackageInfoWithoutStateUtils;
@@ -476,9 +473,9 @@ public final class Settings implements Watchable, Snappable {
     @Watched
     final WatchedArrayMap<String, SharedUserSetting> mSharedUsers = new WatchedArrayMap<>();
     @Watched
-    private final WatchedArrayList<SettingBase> mAppIds;
+    private final AppIdSettingMap mAppIds;
     @Watched
-    private final WatchedSparseArray<SettingBase> mOtherAppIds;
+    private final AppIdSettingMap mOtherAppIds;
 
     // For reading/writing settings file.
     @Watched
@@ -594,8 +591,8 @@ public final class Settings implements Watchable, Snappable {
 
         mLock = new PackageManagerTracedLock();
         mPackages.putAll(pkgSettings);
-        mAppIds = new WatchedArrayList<>();
+        mAppIds = new AppIdSettingMap();
-        mOtherAppIds = new WatchedSparseArray<>();
+        mOtherAppIds = new AppIdSettingMap();
         mSystemDir = null;
         mPermissions = null;
         mRuntimePermissionsPersistence = null;
@@ -631,8 +628,8 @@ public final class Settings implements Watchable, Snappable {
         mKeySetManagerService = new KeySetManagerService(mPackages);
 
         mLock = lock;
-        mAppIds = new WatchedArrayList<>();
+        mAppIds = new AppIdSettingMap();
-        mOtherAppIds = new WatchedSparseArray<>();
+        mOtherAppIds = new AppIdSettingMap();
         mPermissions = new LegacyPermissionSettings(lock);
         mRuntimePermissionsPersistence = new RuntimePermissionPersistence(
                 runtimePermissionsPersistence, new Consumer<Integer>() {
@@ -1278,7 +1275,8 @@ public final class Settings implements Watchable, Snappable {
     // Utility method that adds a PackageSetting to mPackages and
     // completes updating the shared user attributes and any restored
     // app link verification state
-    private void addPackageSettingLPw(PackageSetting p, SharedUserSetting sharedUser) {
+    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE)
+    void addPackageSettingLPw(PackageSetting p, SharedUserSetting sharedUser) {
         mPackages.put(p.getPackageName(), p);
         if (sharedUser != null) {
             SharedUserSetting existingSharedUserSetting = getSharedUserSettingLPr(p);
@@ -1301,7 +1299,7 @@ public final class Settings implements Watchable, Snappable {
             p.setAppId(sharedUser.mAppId);
         }
 
-        // If the we know about this user id, we have to update it as it
+        // If we know about this user id, we have to update it as it
         // has to point to the same PackageSetting instance as the package.
         Object userIdPs = getSettingLPr(p.getAppId());
         if (sharedUser == null) {
@@ -1366,20 +1364,13 @@ public final class Settings implements Watchable, Snappable {
         }
 
         if (appId >= Process.FIRST_APPLICATION_UID) {
-            int size = mAppIds.size();
+            if (mAppIds.get(appId) != null) {
-            final int index = appId - Process.FIRST_APPLICATION_UID;
-            // fill the array until our index becomes valid
-            while (index >= size) {
-                mAppIds.add(null);
-                size++;
-            }
-            if (mAppIds.get(index) != null) {
                 PackageManagerService.reportSettingsProblem(Log.WARN,
                         "Adding duplicate app id: " + appId
                         + " name=" + name);
                 return false;
             }
-            mAppIds.set(index, obj);
+            mAppIds.put(appId, obj);
         } else {
             if (mOtherAppIds.get(appId) != null) {
                 PackageManagerService.reportSettingsProblem(Log.WARN,
@@ -1395,9 +1386,7 @@ public final class Settings implements Watchable, Snappable {
     /** Gets the setting associated with the provided App ID */
     public SettingBase getSettingLPr(int appId) {
         if (appId >= Process.FIRST_APPLICATION_UID) {
-            final int size = mAppIds.size();
+            return mAppIds.get(appId);
-            final int index = appId - Process.FIRST_APPLICATION_UID;
-            return index < size ? mAppIds.get(index) : null;
         } else {
             return mOtherAppIds.get(appId);
         }
@@ -1406,9 +1395,7 @@ public final class Settings implements Watchable, Snappable {
     /** Unregisters the provided app ID. */
     void removeAppIdLPw(int appId) {
         if (appId >= Process.FIRST_APPLICATION_UID) {
-            final int size = mAppIds.size();
+            mAppIds.remove(appId);
-            final int index = appId - Process.FIRST_APPLICATION_UID;
-            if (index < size) mAppIds.set(index, null);
         } else {
             mOtherAppIds.remove(appId);
         }
@@ -1417,9 +1404,14 @@ public final class Settings implements Watchable, Snappable {
 
     private void replaceAppIdLPw(int appId, SettingBase obj) {
         if (appId >= Process.FIRST_APPLICATION_UID) {
-            final int size = mAppIds.size();
+            if (appId <= mAppIds.getCurrentMaxAppId()) {
-            final int index = appId - Process.FIRST_APPLICATION_UID;
+                mAppIds.put(appId, obj);
-            if (index < size) mAppIds.set(index, obj);
+            } else {
+                PackageManagerService.reportSettingsProblem(Log.WARN,
+                        "Error in package manager settings: calling replaceAppIdLpw to"
+                                + " replace SettingBase at appId=" + appId
+                                + " but nothing is replaced.");
+            }
         } else {
             mOtherAppIds.put(appId, obj);
         }
@@ -4304,22 +4296,21 @@ public final class Settings implements Watchable, Snappable {
 
     /** Returns a new AppID or -1 if we could not find an available AppID to assign */
     private int acquireAndRegisterNewAppIdLPw(SettingBase obj) {
-        // Let's be stupidly inefficient for now...
+        final int nextAvailableAppId = mAppIds.getNextAvailableAppId();
-        final int size = mAppIds.size();
+        for (int uid = mFirstAvailableUid; uid < nextAvailableAppId; uid++) {
-        for (int i = mFirstAvailableUid - Process.FIRST_APPLICATION_UID; i < size; i++) {
+            if (mAppIds.get(uid) == null) {
-            if (mAppIds.get(i) == null) {
+                mAppIds.put(uid, obj);
-                mAppIds.set(i, obj);
+                return uid;
-                return Process.FIRST_APPLICATION_UID + i;
             }
         }
 
         // None left?
-        if (size > (Process.LAST_APPLICATION_UID - Process.FIRST_APPLICATION_UID)) {
+        if (nextAvailableAppId > Process.LAST_APPLICATION_UID) {
             return -1;
         }
 
-        mAppIds.add(obj);
+        mAppIds.put(nextAvailableAppId, obj);
-        return Process.FIRST_APPLICATION_UID + size;
+        return nextAvailableAppId;
     }
 
     public VerifierDeviceIdentity getVerifierDeviceIdentityLPw(@NonNull Computer computer) {
@@ -4354,33 +4345,6 @@ public final class Settings implements Watchable, Snappable {
         return getDisabledSystemPkgLPr(enabledPackageSetting.getPackageName());
     }
 
-    boolean isEnabledAndMatchLPr(ComponentInfo componentInfo, long flags, int userId) {
-        final PackageSetting ps = mPackages.get(componentInfo.packageName);
-        if (ps == null) return false;
-
-        final PackageUserStateInternal userState = ps.readUserState(userId);
-        return PackageUserStateUtils.isMatch(userState, componentInfo, flags);
-    }
-
-    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
-    public boolean isEnabledAndMatchLPr(AndroidPackage pkg, ParsedMainComponent component,
-            long flags, int userId) {
-        final PackageSetting ps = mPackages.get(component.getPackageName());
-        if (ps == null) return false;
-
-        final PackageUserStateInternal userState = ps.readUserState(userId);
-        return PackageUserStateUtils.isMatch(userState, pkg.isSystem(), pkg.isEnabled(), component,
-                flags);
-    }
-
-    boolean isOrphaned(String packageName) {
-        final PackageSetting pkg = mPackages.get(packageName);
-        if (pkg == null) {
-            throw new IllegalArgumentException("Unknown package: " + packageName);
-        }
-        return pkg.getInstallSource().isOrphaned;
-    }
-
     int getApplicationEnabledSettingLPr(String packageName, int userId)
             throws PackageManager.NameNotFoundException {
         final PackageSetting pkg = mPackages.get(packageName);

services/tests/servicestests/src/com/android/server/pm/PackageManagerSettingsTests.java

@@ -31,11 +31,11 @@ import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import android.annotation.NonNull;
@@ -1103,6 +1103,59 @@ public class PackageManagerSettingsTests {
         assertThat(countDownLatch.getCount(), is(0L));
     }
 
+    @Test
+    public void testRegisterAndRemoveAppId() throws PackageManagerException {
+        // Test that the first new app UID should start from FIRST_APPLICATION_UID
+        final Settings settings = makeSettings();
+        final PackageSetting ps = createPackageSetting("com.foo");
+        assertTrue(settings.registerAppIdLPw(ps, false));
+        assertEquals(10000, ps.getAppId());
+        // Set up existing app IDs: 10000, 10001, 10003
+        final PackageSetting ps1 = createPackageSetting("com.foo1");
+        ps1.setAppId(10001);
+        final PackageSetting ps2 = createPackageSetting("com.foo2");
+        ps2.setAppId(10003);
+        final PackageSetting ps3 = createPackageSetting("com.foo3");
+        assertEquals(0, ps3.getAppId());
+        assertTrue(settings.registerAppIdLPw(ps1, false));
+        assertTrue(settings.registerAppIdLPw(ps2, false));
+        assertTrue(settings.registerAppIdLPw(ps3, false));
+        assertEquals(10001, ps1.getAppId());
+        assertEquals(10003, ps2.getAppId());
+        // Expecting the new one to start with the next available uid
+        assertEquals(10002, ps3.getAppId());
+        // Remove and insert a new one and the new one should not reuse the same uid
+        settings.removeAppIdLPw(10002);
+        final PackageSetting ps4 = createPackageSetting("com.foo4");
+        assertTrue(settings.registerAppIdLPw(ps4, false));
+        assertEquals(10004, ps4.getAppId());
+        // Keep adding more
+        final PackageSetting ps5 = createPackageSetting("com.foo5");
+        assertTrue(settings.registerAppIdLPw(ps5, false));
+        assertEquals(10005, ps5.getAppId());
+        // Remove the last one and the new one should use incremented uid
+        settings.removeAppIdLPw(10005);
+        final PackageSetting ps6 = createPackageSetting("com.foo6");
+        assertTrue(settings.registerAppIdLPw(ps6, false));
+        assertEquals(10006, ps6.getAppId());
+    }
+
+    /**
+     * Test replacing a PackageSetting with a SharedUserSetting in mAppIds
+     */
+    @Test
+    public void testAddPackageSetting() throws PackageManagerException {
+        final Settings settings = makeSettings();
+        final SharedUserSetting sus1 = new SharedUserSetting(
+                "TestUser", 0 /*pkgFlags*/, 0 /*pkgPrivateFlags*/);
+        sus1.mAppId = 10001;
+        final PackageSetting ps1 = createPackageSetting("com.foo");
+        ps1.setAppId(10001);
+        assertTrue(settings.registerAppIdLPw(ps1, false));
+        settings.addPackageSettingLPw(ps1, sus1);
+        assertSame(sus1, settings.getSharedUserSettingLPr(ps1));
+    }
+
     private void verifyUserState(PackageUserState userState,
             boolean notLaunched, boolean stopped, boolean installed) {
         assertThat(userState.getEnabledState(), is(0));