Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1cd2eb67 authored by Hai Zhang's avatar Hai Zhang
Browse files

Refactor updatePermissions() into onPackageInstalled().

1. For the case where a dangling system PackageSetting is left without
a (disabled) factory package, the previous logic was to remove its data
directory before ag/920260, which deferred the removal into
reconcileAppsDataLI(). However, there are more package related states
that should be cleaned up apart from the data directory, hence
ag/7278993 added a call to removePackageDataLIF() for the similar case
when handling dangling updated system apps. Meanwhile, ag/11872632 was
later submitted to remove the permission state for this case of
factory packages, but still, it seems more consistent to also call
removePackageDataLIF() for consistency and better cleanup.

2. For enableCompressedPackage() and installPackageFromSystemLIF(),
they are simply changed from calling updatePermissions() to call
onPackageInstalled() with no additional permission granting.

3. onPackageInstalled() is changed to call updatePermissions() as the
first thing, and updatePermissions() is removed from the internal API.

4. The call to updatePermissions() is removed from
updateSettingsInternalLI(), because the only call path into it goes
back to processInstallRequestsAsync(), which will always call
restoreAndPostInstall() which goes into handlePackagePostInstall() and
call onPackageInstalled() there. Packages needs to go through
handlePackagePostInstall() to get their permission allowlisted/granted
anyway so not calling updatePermissions() again earlier shouldn't be a
problem.

Bug: 158736025
Test: presubmit
Test: manual
Test: adb remount and push an APK to /system/priv-app
Test: adb reboot and verify package is system package
Test: flash a build with this change so that system partition is reset
      and app is gone
Test: onPackageInstalled() logged stacktrace for being called and
      package setting is removed.
Change-Id: Id68428b3aca18c0badee58d1a57663510db438e9
parent ea789fd6
Loading
Loading
Loading
Loading
+13 −17
Original line number Diff line number Diff line
@@ -3305,6 +3305,7 @@ public class PackageManagerService extends IPackageManager.Stub
            // Stub packages must either be replaced with full versions in the /data
            // partition or be disabled.
            final List<String> stubSystemApps = new ArrayList<>();
            final int[] userIds = mUserManager.getUserIds();
            if (!mOnlyCore) {
                // do this first before mucking with mPackages for the "expecting better" case
                final int numPackages = mPackages.size();
@@ -3355,15 +3356,9 @@ public class PackageManagerService extends IPackageManager.Stub
                    }
                    if (!mSettings.isDisabledSystemPackageLPr(ps.name)) {
                        mSettings.mPackages.removeAt(index);
                        logCriticalInfo(Log.WARN, "System package " + ps.name
                                + " no longer exists; it's data will be wiped");
                        // Assume package is truly gone and wipe residual permissions.
                        mPermissionManager.updatePermissions(ps.name, null);
                        // Actual deletion of code and data will be handled by later
                        // reconciliation step
                        removePackageDataLIF(ps, userIds, null, 0, false);
                    } else {
                        // we still have a disabled system package, but, it still might have
                        // been removed. check the code path still exists and check there's
@@ -3422,7 +3417,6 @@ public class PackageManagerService extends IPackageManager.Stub
                // Remove disable package settings for updated system apps that were
                // removed via an OTA. If the update is no longer present, remove the
                // app completely. Otherwise, revoke their system privileges.
                final int[] userIds = mUserManager.getUserIds();
                for (int i = possiblyDeletedUpdatedSystemApps.size() - 1; i >= 0; --i) {
                    final String packageName = possiblyDeletedUpdatedSystemApps.get(i);
                    final AndroidPackage pkg = mPackages.get(packageName);
@@ -3757,7 +3751,6 @@ public class PackageManagerService extends IPackageManager.Stub
                            UserHandle.USER_SYSTEM).getLongVersionCode());
            // Initialize InstantAppRegistry's Instant App list for all users.
            final int[] userIds = UserManagerService.getInstance().getUserIds();
            for (AndroidPackage pkg : mPackages.values()) {
                if (pkg.isSystem()) {
                    continue;
@@ -3923,7 +3916,11 @@ public class PackageManagerService extends IPackageManager.Stub
                    } catch (PackageManagerException e) {
                        Slog.e(TAG, "updateAllSharedLibrariesLPw failed: ", e);
                    }
                    mPermissionManager.updatePermissions(pkg.getPackageName(), pkg);
                    final int[] userIds = mUserManager.getUserIds();
                    for (final int userId : userIds) {
                        mPermissionManager.onPackageInstalled(pkg, Collections.emptyList(),
                                Collections.emptyList(), MODE_DEFAULT, userId);
                    }
                    writeSettingsLPrTEMP();
                }
            } catch (PackageManagerException e) {
@@ -16476,8 +16473,6 @@ public class PackageManagerService extends IPackageManager.Stub
        if (DEBUG_INSTALL) Slog.d(TAG, "New package installed in " + pkg.getPath());
        synchronized (mLock) {
// NOTE: This changes slightly to include UPDATE_PERMISSIONS_ALL regardless of the size of pkg.permissions
            mPermissionManager.updatePermissions(pkgName, pkg);
            // For system-bundled packages, we assume that installing an upgraded version
            // of the package implies that the user actually wants to run that new code,
            // so we enable the package.
@@ -19726,11 +19721,12 @@ public class PackageManagerService extends IPackageManager.Stub
                }
            }
            // The update permissions method below will take care of removing obsolete permissions
            // and granting install permissions.
            mPermissionManager.updatePermissions(pkg.getPackageName(), pkg);
            for (final int userId : allUserHandles) {
                // The method below will take care of removing obsolete permissions and granting
                // install permissions.
                mPermissionManager.onPackageInstalled(pkg, Collections.emptyList(),
                        Collections.emptyList(), MODE_DEFAULT, userId);
                if (applyUserRestrictions) {
                for (int userId : allUserHandles) {
                    mSettings.writeRuntimePermissionsForUserLPr(userId, false);
                }
            }
+4 −11
Original line number Diff line number Diff line
@@ -4091,17 +4091,14 @@ public class PermissionManagerService extends IPermissionManager.Stub {
     *
     * @param packageName The package that is updated
     * @param pkg The package that is updated, or {@code null} if package is deleted
     * @param allPackages All currently known packages
     * @param callback Callback to call after permission changes
     */
    private void updatePermissions(@NonNull String packageName, @Nullable AndroidPackage pkg,
            @NonNull PermissionCallback callback) {
    private void updatePermissions(@NonNull String packageName, @Nullable AndroidPackage pkg) {
        // If the package is being deleted, update the permissions of all the apps
        final int flags =
                (pkg == null ? UPDATE_PERMISSIONS_ALL | UPDATE_PERMISSIONS_REPLACE_PKG
                        : UPDATE_PERMISSIONS_REPLACE_PKG);
        updatePermissions(
                packageName, pkg, getVolumeUuidForPackage(pkg), flags, callback);
                packageName, pkg, getVolumeUuidForPackage(pkg), flags, mDefaultPermissionCallback);
    }

    /**
@@ -4901,6 +4898,7 @@ public class PermissionManagerService extends IPermissionManager.Stub {
            @NonNull List<String> grantedPermissions,
            @NonNull List<String> allowlistedRestrictedPermissions, int autoRevokePermissionsMode,
            @UserIdInt int userId) {
        updatePermissions(pkg.getPackageName(), pkg);
        addAllowlistedRestrictedPermissionsInternal(pkg, allowlistedRestrictedPermissions,
                FLAG_PERMISSION_WHITELIST_INSTALLER, userId);
        if (autoRevokePermissionsMode == AppOpsManager.MODE_ALLOWED
@@ -4945,7 +4943,7 @@ public class PermissionManagerService extends IPermissionManager.Stub {
            resetRuntimePermissionsInternal(pkg, userId);
            return;
        }
        updatePermissions(packageName, null, mDefaultPermissionCallback);
        updatePermissions(packageName, null);
        if (sharedUserPkgs.isEmpty()) {
            removeUidState(appId, userId);
        } else {
@@ -5100,11 +5098,6 @@ public class PermissionManagerService extends IPermissionManager.Stub {
            return PermissionManagerService.this.getAppOpPermissionPackagesInternal(permissionName);
        }
        @Override
        public void updatePermissions(@NonNull String packageName, @Nullable AndroidPackage pkg) {
            PermissionManagerService.this
                    .updatePermissions(packageName, pkg, mDefaultPermissionCallback);
        }
        @Override
        public void updateAllPermissions(@Nullable String volumeUuid, boolean sdkUpdated) {
            PermissionManagerService.this
                    .updateAllPermissions(volumeUuid, sdkUpdated, mDefaultPermissionCallback);
+1 −17
Original line number Diff line number Diff line
@@ -188,22 +188,6 @@ public abstract class PermissionManagerServiceInternal extends PermissionManager
    public abstract boolean isPermissionsReviewRequired(@NonNull String packageName,
            @UserIdInt int userId);

    /**
     * Update permissions when a package changed.
     *
     * <p><ol>
     *     <li>Reconsider the ownership of permission</li>
     *     <li>Update the state (grant, flags) of the permissions</li>
     * </ol>
     *
     * @param packageName The package that is updated
     * @param pkg The package that is updated, or {@code null} if package is deleted
     * @param allPackages All currently known packages
     * @param callback Callback to call after permission changes
     */
    public abstract void updatePermissions(@NonNull String packageName,
            @Nullable AndroidPackage pkg);

    /**
     * Update all permissions for all apps.
     *
@@ -489,7 +473,7 @@ public abstract class PermissionManagerServiceInternal extends PermissionManager
            @Nullable AndroidPackage oldPkg);

    /**
     * Callback when a package has been installed for certain users.
     * Callback when a package has been installed for a user.
     *
     * @param pkg the installed package
     * @param grantedPermissions the permissions to be granted