Persist permission state for updated apps synchronously.

Because if we lose the changes, an updated app won't be reconsidered for
new install permissions.

Fixes: 221899913
Test: presubmit
Change-Id: Ib82189b07cf4dfd132be37fb19843d770e871f8c
Merged-In: Ib82189b07cf4dfd132be37fb19843d770e871f8c

1 files changed, 16 insertions, 5 deletions

diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
index 290f4aea498e..dbd8f35899dd 100644
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
@@ -2626,6 +2626,7 @@ public class PermissionManagerService extends IPermissionManager.Stub {
         final int[] userIds = filterUserId == UserHandle.USER_ALL ? getAllUserIds()
                 : new int[] { filterUserId };
 
+        boolean installPermissionsChanged = false;
         boolean runtimePermissionsRevoked = false;
         int[] updatedUserIds = EMPTY_INT_ARRAY;
 
@@ -2739,7 +2740,7 @@ public class PermissionManagerService extends IPermissionManager.Stub {
 
                 UidPermissionState origState = uidState;
 
-                boolean changedInstallPermission = false;
+                boolean installPermissionsChangedForUser = false;
 
                 if (replace) {
                     userState.setInstallPermissionsFixed(ps.name, false);
@@ -2904,7 +2905,7 @@ public class PermissionManagerService extends IPermissionManager.Stub {
                                                     && origState.isPermissionGranted(permName))))) {
                         // Grant an install permission.
                         if (uidState.grantPermission(bp)) {
-                            changedInstallPermission = true;
+                            installPermissionsChangedForUser = true;
                         }
                     } else if (bp.isRuntime()) {
                         boolean hardRestricted = bp.isHardRestricted();
@@ -3034,12 +3035,12 @@ public class PermissionManagerService extends IPermissionManager.Stub {
                             }
                         }
                         if (uidState.removePermissionState(bp.getName())) {
-                            changedInstallPermission = true;
+                            installPermissionsChangedForUser = true;
                         }
                     }
                 }
 
-                if ((changedInstallPermission || replace)
+                if ((installPermissionsChangedForUser || replace)
                         && !userState.areInstallPermissionsFixed(ps.name)
                         && !ps.isSystem() || ps.getPkgState().isUpdatedSystemApp()) {
                     // This is the first that we have heard about this package, so the
@@ -3048,6 +3049,12 @@ public class PermissionManagerService extends IPermissionManager.Stub {
                     userState.setInstallPermissionsFixed(ps.name, true);
                 }
 
+                if (installPermissionsChangedForUser) {
+                    installPermissionsChanged = true;
+                    if (replace) {
+                        updatedUserIds = ArrayUtils.appendInt(updatedUserIds, userId);
+                    }
+                }
                 updatedUserIds = revokePermissionsNoLongerImplicitLocked(uidState,
                         pkg.getPackageName(), uidImplicitPermissions, uidTargetSdkVersion, userId,
                         updatedUserIds);
@@ -3064,8 +3071,12 @@ public class PermissionManagerService extends IPermissionManager.Stub {
         // Persist the runtime permissions state for users with changes. If permissions
         // were revoked because no app in the shared user declares them we have to
         // write synchronously to avoid losing runtime permissions state.
+        // Also write synchronously if we changed any install permission for an updated app, because
+        // the install permission state is likely already fixed before update, and if we lose the
+        // changes here the app won't be reconsidered for newly-added install permissions.
         if (callback != null) {
-            callback.onPermissionUpdated(updatedUserIds, runtimePermissionsRevoked);
+            callback.onPermissionUpdated(updatedUserIds,
+                    (replace && installPermissionsChanged) || runtimePermissionsRevoked);
         }
 
         for (int userId : updatedUserIds) {