Remove INSTALLED_USERS extra from ACTION_PACKAGE_ENABLE_ROLLBACK.

This changes RollbackManagerServiceImpl to use the list of installed users
that is passed in to snapshotAndRestoreUserData for the purpose of snapshotting.

With this change, it is no longer necessary for the list of installed users to be
added as an extra to the ENABLE_ROLLBACK intent, so that extra
(EXTRA_ENABLE_ROLLBACK_INSTALLED_USERS) is removed.

Bug: 139181444
Test: atest RollbackTest
Test: atest AppDataRollbackHelperTest
Change-Id: Iffd63977a7498e3cba20d2039af623c921d38f57

core/java/android/content/pm/PackageManagerInternal.java

@@ -674,12 +674,6 @@ public abstract class PackageManagerInternal {
     public static final String EXTRA_ENABLE_ROLLBACK_INSTALL_FLAGS =
             "android.content.pm.extra.ENABLE_ROLLBACK_INSTALL_FLAGS";
 
-    /**
-     * Extra field name for the set of installed users for a given rollback package.
-     */
-    public static final String EXTRA_ENABLE_ROLLBACK_INSTALLED_USERS =
-            "android.content.pm.extra.ENABLE_ROLLBACK_INSTALLED_USERS";
-
     /**
      * Extra field name for the user id an install is associated with when
      * enabling rollback.

core/java/android/content/rollback/PackageRollbackInfo.java

@@ -76,10 +76,10 @@ public final class PackageRollbackInfo implements Parcelable {
     private final boolean mIsApex;
 
     /*
-     * The list of users the package is installed for.
+     * The list of users for which snapshots have been saved.
      */
     // NOTE: Not a part of the Parcelable representation of this object.
-    private final IntArray mInstalledUsers;
+    private final IntArray mSnapshottedUsers;
 
     /**
      * A mapping between user and an inode of theirs CE data snapshot.
@@ -148,8 +148,8 @@ public final class PackageRollbackInfo implements Parcelable {
     }
 
     /** @hide */
-    public IntArray getInstalledUsers() {
+    public IntArray getSnapshottedUsers() {
-        return mInstalledUsers;
+        return mSnapshottedUsers;
     }
 
     /** @hide */
@@ -179,14 +179,14 @@ public final class PackageRollbackInfo implements Parcelable {
     public PackageRollbackInfo(VersionedPackage packageRolledBackFrom,
             VersionedPackage packageRolledBackTo,
             @NonNull IntArray pendingBackups, @NonNull ArrayList<RestoreInfo> pendingRestores,
-            boolean isApex, @NonNull IntArray installedUsers,
+            boolean isApex, @NonNull IntArray snapshottedUsers,
             @NonNull SparseLongArray ceSnapshotInodes) {
         this.mVersionRolledBackFrom = packageRolledBackFrom;
         this.mVersionRolledBackTo = packageRolledBackTo;
         this.mPendingBackups = pendingBackups;
         this.mPendingRestores = pendingRestores;
         this.mIsApex = isApex;
-        this.mInstalledUsers = installedUsers;
+        this.mSnapshottedUsers = snapshottedUsers;
         this.mCeSnapshotInodes = ceSnapshotInodes;
     }
 
@@ -196,7 +196,7 @@ public final class PackageRollbackInfo implements Parcelable {
         this.mIsApex = in.readBoolean();
         this.mPendingRestores = null;
         this.mPendingBackups = null;
-        this.mInstalledUsers = null;
+        this.mSnapshottedUsers = null;
         this.mCeSnapshotInodes = null;
     }
 

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

@@ -14580,17 +14580,6 @@ public class PackageManagerService extends IPackageManager.Stub
                             TRACE_TAG_PACKAGE_MANAGER, "enable_rollback", enableRollbackToken);
                     mPendingEnableRollback.append(enableRollbackToken, this);
 
-                    final int[] installedUsers;
-                    synchronized (mLock) {
-                        PackageSetting ps = mSettings.getPackageLPr(pkgLite.packageName);
-                        if (ps != null) {
-                            installedUsers = ps.queryInstalledUsers(mUserManager.getUserIds(),
-                                    true);
-                        } else {
-                            installedUsers = new int[0];
-                        }
-                    }
-
                     Intent enableRollbackIntent = new Intent(Intent.ACTION_PACKAGE_ENABLE_ROLLBACK);
                     enableRollbackIntent.putExtra(
                             PackageManagerInternal.EXTRA_ENABLE_ROLLBACK_TOKEN,
@@ -14598,9 +14587,6 @@ public class PackageManagerService extends IPackageManager.Stub
                     enableRollbackIntent.putExtra(
                             PackageManagerInternal.EXTRA_ENABLE_ROLLBACK_INSTALL_FLAGS,
                             installFlags);
-                    enableRollbackIntent.putExtra(
-                            PackageManagerInternal.EXTRA_ENABLE_ROLLBACK_INSTALLED_USERS,
-                            installedUsers);
                     enableRollbackIntent.putExtra(
                             PackageManagerInternal.EXTRA_ENABLE_ROLLBACK_USER,
                             getRollbackUser().getIdentifier());

services/core/java/com/android/server/rollback/AppDataRollbackHelper.java

@@ -52,12 +52,13 @@ public class AppDataRollbackHelper {
     }
 
     /**
-     * Creates an app data snapshot for a specified {@code packageRollbackInfo}. Updates said {@code
+     * Creates an app data snapshot for a specified {@code packageRollbackInfo} and the specified
-     * packageRollbackInfo} with the inodes of the CE user data snapshot folders.
+     * {@code userIds}. Updates said {@code packageRollbackInfo} with the inodes of the CE user data
+     * snapshot folders.
      */
-    public void snapshotAppData(int snapshotId, PackageRollbackInfo packageRollbackInfo) {
+    public void snapshotAppData(
-        final int[] installedUsers = packageRollbackInfo.getInstalledUsers().toArray();
+            int snapshotId, PackageRollbackInfo packageRollbackInfo, int[] userIds) {
-        for (int user : installedUsers) {
+        for (int user : userIds) {
             final int storageFlags;
             if (isUserCredentialLocked(user)) {
                 // We've encountered a user that hasn't unlocked on a FBE device, so we can't copy
@@ -80,6 +81,7 @@ public class AppDataRollbackHelper {
                         + packageRollbackInfo.getPackageName() + ", userId: " + user, ie);
             }
         }
+        packageRollbackInfo.getSnapshottedUsers().addAll(IntArray.wrap(userIds));
     }
 
     /**

services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java

@@ -177,16 +177,14 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
                             PackageManagerInternal.EXTRA_ENABLE_ROLLBACK_TOKEN, -1);
                     int installFlags = intent.getIntExtra(
                             PackageManagerInternal.EXTRA_ENABLE_ROLLBACK_INSTALL_FLAGS, 0);
-                    int[] installedUsers = intent.getIntArrayExtra(
-                            PackageManagerInternal.EXTRA_ENABLE_ROLLBACK_INSTALLED_USERS);
                     int user = intent.getIntExtra(
                             PackageManagerInternal.EXTRA_ENABLE_ROLLBACK_USER, 0);
 
                     File newPackageCodePath = new File(intent.getData().getPath());
 
                     getHandler().post(() -> {
-                        boolean success = enableRollback(installFlags, newPackageCodePath,
+                        boolean success =
-                                installedUsers, user, token);
+                                enableRollback(installFlags, newPackageCodePath, user, token);
                         int ret = PackageManagerInternal.ENABLE_ROLLBACK_SUCCEEDED;
                         if (!success) {
                             ret = PackageManagerInternal.ENABLE_ROLLBACK_FAILED;
@@ -821,13 +819,12 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
      *
      * @param installFlags information about what is being installed.
      * @param newPackageCodePath path to the package about to be installed.
-     * @param installedUsers the set of users for which a given package is installed.
      * @param user the user that owns the install session to enable rollback on.
      * @param token the distinct rollback token sent by package manager.
      * @return true if enabling the rollback succeeds, false otherwise.
      */
-    private boolean enableRollback(int installFlags, File newPackageCodePath,
+    private boolean enableRollback(
-            int[] installedUsers, @UserIdInt int user, int token) {
+            int installFlags, File newPackageCodePath, @UserIdInt int user, int token) {
 
         // Find the session id associated with this install.
         // TODO: It would be nice if package manager or package installer told
@@ -872,38 +869,15 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
 
         // Check to see if this is the apk session for a staged session with
         // rollback enabled.
-        // TODO: This check could be made more efficient.
-        RollbackData rd = null;
         synchronized (mLock) {
             for (int i = 0; i < mRollbacks.size(); ++i) {
                 RollbackData data = mRollbacks.get(i);
                 if (data.apkSessionId == parentSession.getSessionId()) {
-                    rd = data;
+                    // This is the apk session for a staged session with rollback enabled. We do not
-                    break;
+                    // need to create a new rollback for this session.
-                }
-            }
-        }
-
-        if (rd != null) {
-            // This is the apk session for a staged session. We do not need to create a new rollback
-            // for this session.
-            PackageParser.PackageLite newPackage = null;
-            try {
-                newPackage = PackageParser.parsePackageLite(
-                        new File(packageSession.resolvedBaseCodePath), 0);
-            } catch (PackageParser.PackageParserException e) {
-                Slog.e(TAG, "Unable to parse new package", e);
-                return false;
-            }
-            String packageName = newPackage.packageName;
-            for (PackageRollbackInfo info : rd.info.getPackages()) {
-                if (info.getPackageName().equals(packageName)) {
-                    info.getInstalledUsers().addAll(IntArray.wrap(installedUsers));
                     return true;
                 }
             }
-            Slog.e(TAG, "Unable to find package in apk session");
-            return false;
         }
 
         NewRollback newRollback;
@@ -919,7 +893,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
         }
         newRollback.addToken(token);
 
-        return enableRollbackForPackageSession(newRollback.data, packageSession, installedUsers);
+        return enableRollbackForPackageSession(newRollback.data, packageSession);
     }
 
     /**
@@ -930,7 +904,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
      * @return true on success, false on failure.
      */
     private boolean enableRollbackForPackageSession(RollbackData data,
-            PackageInstaller.SessionInfo session, @NonNull int[] installedUsers) {
+            PackageInstaller.SessionInfo session) {
         // TODO: Don't attempt to enable rollback for split installs.
         final int installFlags = session.installFlags;
         if ((installFlags & PackageManager.INSTALL_ENABLE_ROLLBACK) == 0) {
@@ -988,8 +962,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
         PackageRollbackInfo packageRollbackInfo = new PackageRollbackInfo(
                 newVersion, installedVersion,
                 new IntArray() /* pendingBackups */, new ArrayList<>() /* pendingRestores */,
-                isApex, IntArray.wrap(installedUsers),
+                isApex, new IntArray(), new SparseLongArray() /* ceSnapshotInodes */);
-                new SparseLongArray() /* ceSnapshotInodes */);
 
         try {
             ApplicationInfo appInfo = pkgInfo.applicationInfo;
@@ -1019,7 +992,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
         }
 
         getHandler().post(() -> {
-            snapshotUserDataInternal(packageName);
+            snapshotUserDataInternal(packageName, userIds);
             restoreUserDataInternal(packageName, userIds, appId, ceDataInode, seInfo, token);
             final PackageManagerInternal pmi = LocalServices.getService(
                     PackageManagerInternal.class);
@@ -1027,7 +1000,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
         });
     }
 
-    private void snapshotUserDataInternal(String packageName) {
+    private void snapshotUserDataInternal(String packageName, int[] userIds) {
         synchronized (mLock) {
             // staged installs
             for (int i = 0; i < mRollbacks.size(); i++) {
@@ -1038,7 +1011,8 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
 
                 for (PackageRollbackInfo info : data.info.getPackages()) {
                     if (info.getPackageName().equals(packageName)) {
-                        mAppDataRollbackHelper.snapshotAppData(data.info.getRollbackId(), info);
+                        mAppDataRollbackHelper.snapshotAppData(
+                                data.info.getRollbackId(), info, userIds);
                         saveRollbackData(data);
                         break;
                     }
@@ -1049,8 +1023,8 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
             for (NewRollback rollback : mNewRollbacks) {
                 info = getPackageRollbackInfo(rollback.data, packageName);
                 if (info != null) {
-                    mAppDataRollbackHelper.snapshotAppData(rollback.data.info.getRollbackId(),
+                    mAppDataRollbackHelper.snapshotAppData(
-                            info);
+                            rollback.data.info.getRollbackId(), info, userIds);
                     saveRollbackData(rollback.data);
                 }
             }
@@ -1114,8 +1088,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
             }
 
             if (!session.isMultiPackage()) {
-                if (!enableRollbackForPackageSession(newRollback.data, session,
+                if (!enableRollbackForPackageSession(newRollback.data, session)) {
-                            new int[0])) {
                     Slog.e(TAG, "Unable to enable rollback for session: " + sessionId);
                     result.offer(false);
                     return;
@@ -1129,8 +1102,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
                         result.offer(false);
                         return;
                     }
-                    if (!enableRollbackForPackageSession(newRollback.data, childSession,
+                    if (!enableRollbackForPackageSession(newRollback.data, childSession)) {
-                                new int[0])) {
                         Slog.e(TAG, "Unable to enable rollback for session: " + sessionId);
                         result.offer(false);
                         return;
@@ -1407,9 +1379,9 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
 
     private void deleteRollback(RollbackData rollbackData) {
         for (PackageRollbackInfo info : rollbackData.info.getPackages()) {
-            IntArray installedUsers = info.getInstalledUsers();
+            IntArray snapshottedUsers = info.getSnapshottedUsers();
-            for (int i = 0; i < installedUsers.size(); i++) {
+            for (int i = 0; i < snapshottedUsers.size(); i++) {
-                int userId = installedUsers.get(i);
+                int userId = snapshottedUsers.get(i);
                 mAppDataRollbackHelper.destroyAppDataSnapshot(rollbackData.info.getRollbackId(),
                         info, userId);
             }

services/core/java/com/android/server/rollback/RollbackStore.java

@@ -319,13 +319,14 @@ class RollbackStore {
 
         IntArray pendingBackups = info.getPendingBackups();
         List<RestoreInfo> pendingRestores = info.getPendingRestores();
-        IntArray installedUsers = info.getInstalledUsers();
+        IntArray snapshottedUsers = info.getSnapshottedUsers();
         json.put("pendingBackups", convertToJsonArray(pendingBackups));
         json.put("pendingRestores", convertToJsonArray(pendingRestores));
 
         json.put("isApex", info.isApex());
 
-        json.put("installedUsers", convertToJsonArray(installedUsers));
+        // Field is named 'installedUsers' for legacy reasons.
+        json.put("installedUsers", convertToJsonArray(snapshottedUsers));
         json.put("ceSnapshotInodes", ceSnapshotInodesToJson(info.getCeSnapshotInodes()));
 
         return json;
@@ -345,12 +346,13 @@ class RollbackStore {
 
         final boolean isApex = json.getBoolean("isApex");
 
-        final IntArray installedUsers = convertToIntArray(json.getJSONArray("installedUsers"));
+        // Field is named 'installedUsers' for legacy reasons.
+        final IntArray snapshottedUsers = convertToIntArray(json.getJSONArray("installedUsers"));
         final SparseLongArray ceSnapshotInodes = ceSnapshotInodesFromJson(
                 json.getJSONArray("ceSnapshotInodes"));
 
         return new PackageRollbackInfo(versionRolledBackFrom, versionRolledBackTo,
-                pendingBackups, pendingRestores, isApex, installedUsers, ceSnapshotInodes);
+                pendingBackups, pendingRestores, isApex, snapshottedUsers, ceSnapshotInodes);
     }
 
     private static JSONArray versionedPackagesToJson(List<VersionedPackage> packages)

services/tests/servicestests/src/com/android/server/rollback/AppDataRollbackHelperTest.java

@@ -16,6 +16,7 @@
 
 package com.android.server.rollback;
 
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
@@ -58,8 +59,8 @@ public class AppDataRollbackHelperTest {
         // All users are unlocked so we should snapshot data for them.
         doReturn(true).when(helper).isUserCredentialLocked(eq(10));
         doReturn(true).when(helper).isUserCredentialLocked(eq(11));
-        PackageRollbackInfo info = createPackageRollbackInfo("com.foo.bar", new int[]{10, 11});
+        PackageRollbackInfo info = createPackageRollbackInfo("com.foo.bar");
-        helper.snapshotAppData(5, info);
+        helper.snapshotAppData(5, info, new int[]{10, 11});
 
         assertEquals(2, info.getPendingBackups().size());
         assertEquals(10, info.getPendingBackups().get(0));
@@ -79,8 +80,8 @@ public class AppDataRollbackHelperTest {
         doReturn(true).when(helper).isUserCredentialLocked(eq(11));
         when(installer.snapshotAppData(anyString(), anyInt(), anyInt(), anyInt())).thenReturn(239L);
 
-        PackageRollbackInfo info2 = createPackageRollbackInfo("com.foo.bar", new int[]{10, 11});
+        PackageRollbackInfo info2 = createPackageRollbackInfo("com.foo.bar");
-        helper.snapshotAppData(7, info2);
+        helper.snapshotAppData(7, info2, new int[]{10, 11});
         assertEquals(1, info2.getPendingBackups().size());
         assertEquals(11, info2.getPendingBackups().get(0));
 
@@ -278,4 +279,15 @@ public class AppDataRollbackHelperTest {
 
         inOrder.verifyNoMoreInteractions();
     }
+
+    @Test
+    public void snapshotAddDataSavesSnapshottedUsersToInfo() {
+        Installer installer = mock(Installer.class);
+        AppDataRollbackHelper helper = new AppDataRollbackHelper(installer);
+
+        PackageRollbackInfo info = createPackageRollbackInfo("com.foo.bar");
+        helper.snapshotAppData(5, info, new int[]{10, 11});
+
+        assertArrayEquals(info.getSnapshottedUsers().toArray(), new int[]{10, 11});
+    }
 }