From 959c54f49112322b9caaac962a6a3bd2fabfd782 Mon Sep 17 00:00:00 2001 From: Winson Date: Thu, 14 May 2020 17:02:42 -0700 Subject: [PATCH 1/2] Reset default browser on installExistingForUser Previously the preferred browser was only reset on the initial install of a package. So if a different user already had the browser installed, adding it to another user would not reset their default browser. In general the post install steps should be split up into postFirstInstall and postInstallForUser, where the former calls the latter, but that's a much larger change. This doesn't fix an unfortunate bug where uninstalling a browser and re-installing it without re-querying for the preferred browser will clear the browser role, but not wipe the preferred Activity from settings. Fixing this requires a much larger refactor, and is probably not worth it given the assumption that users don't re-install apps expecting it to reset the browser. Bug: 152898545 Test: manual install browser to personal profile and then to work profile, verify disambiguation Activity pops up Change-Id: Ie02b45c28c705bdcfb5bf89b9e7a33b4850cf0f7 --- .../server/pm/PackageManagerService.java | 71 +++++++++++-------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 7959461c58248..7093e9aeb0d42 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -1566,13 +1566,17 @@ public class PackageManagerService extends IPackageManager.Stub // Recordkeeping of restore-after-install operations that are currently in flight // between the Package Manager and the Backup Manager static class PostInstallData { + @Nullable public final InstallArgs args; + @NonNull public final PackageInstalledInfo res; + @Nullable public final Runnable mPostInstallRunnable; - PostInstallData(InstallArgs _a, PackageInstalledInfo _r, Runnable postInstallRunnable) { - args = _a; - res = _r; + PostInstallData(@Nullable InstallArgs args, @NonNull PackageInstalledInfo res, + @Nullable Runnable postInstallRunnable) { + this.args = args; + this.res = res; mPostInstallRunnable = postInstallRunnable; } } @@ -1714,7 +1718,7 @@ public class PackageManagerService extends IPackageManager.Stub if (data != null && data.mPostInstallRunnable != null) { data.mPostInstallRunnable.run(); - } else if (data != null) { + } else if (data != null && data.args != null) { InstallArgs args = data.args; PackageInstalledInfo parentRes = data.res; @@ -2306,27 +2310,8 @@ public class PackageManagerService extends IPackageManager.Stub // Work that needs to happen on first install within each user if (firstUserIds != null && firstUserIds.length > 0) { for (int userId : firstUserIds) { - // If this app is a browser and it's newly-installed for some - // users, clear any default-browser state in those users. The - // app's nature doesn't depend on the user, so we can just check - // its browser nature in any user and generalize. - if (packageIsBrowser(packageName, userId)) { - // If this browser is restored from user's backup, do not clear - // default-browser state for this user - if (pkgSetting.getInstallReason(userId) - != PackageManager.INSTALL_REASON_DEVICE_RESTORE) { - mPermissionManager.setDefaultBrowser(null, true, true, userId); - } - } - - // We may also need to apply pending (restored) runtime permission grants - // within these users. - mPermissionManager.restoreDelayedRuntimePermissions(packageName, - UserHandle.of(userId)); - - // Persistent preferred activity might have came into effect due to this - // install. - updateDefaultHomeNotLocked(userId); + clearRolesAndRestorePermissionsForNewUserInstall(packageName, + pkgSetting.getInstallReason(userId), userId); } } @@ -13122,9 +13107,15 @@ public class PackageManagerService extends IPackageManager.Stub createPackageInstalledInfo(PackageManager.INSTALL_SUCCEEDED); res.pkg = pkgSetting.pkg; res.newUsers = new int[]{ userId }; - PostInstallData postInstallData = intentSender == null ? null : - new PostInstallData(null, res, () -> onRestoreComplete(res.returnCode, - mContext, intentSender)); + + PostInstallData postInstallData = + new PostInstallData(null, res, () -> { + clearRolesAndRestorePermissionsForNewUserInstall(packageName, + pkgSetting.getInstallReason(userId), userId); + if (intentSender != null) { + onRestoreComplete(res.returnCode, mContext, intentSender); + } + }); restoreAndPostInstall(userId, res, postInstallData); } } finally { @@ -19749,6 +19740,30 @@ public class PackageManagerService extends IPackageManager.Stub } } + private void clearRolesAndRestorePermissionsForNewUserInstall(String packageName, + int installReason, @UserIdInt int userId) { + // If this app is a browser and it's newly-installed for some + // users, clear any default-browser state in those users. The + // app's nature doesn't depend on the user, so we can just check + // its browser nature in any user and generalize. + if (packageIsBrowser(packageName, userId)) { + // If this browser is restored from user's backup, do not clear + // default-browser state for this user + if (installReason != PackageManager.INSTALL_REASON_DEVICE_RESTORE) { + mPermissionManager.setDefaultBrowser(null, true, true, userId); + } + } + + // We may also need to apply pending (restored) runtime permission grants + // within these users. + mPermissionManager.restoreDelayedRuntimePermissions(packageName, + UserHandle.of(userId)); + + // Persistent preferred activity might have came into effect due to this + // install. + updateDefaultHomeNotLocked(userId); + } + @Override public void resetApplicationPreferences(int userId) { mContext.enforceCallingOrSelfPermission( From cd58bf45ecc71a67495fbf509e985fb7c4b40d8b Mon Sep 17 00:00:00 2001 From: Winson Date: Thu, 14 May 2020 17:08:21 -0700 Subject: [PATCH 2/2] Remove residual childPackages code This feature was removed in R, so this code was never called. Cleaned up so that handlePackagePostInstall can assume that it's only called once per user per install session. Bug: 152898545 Test: manual device boots Change-Id: I4431383ad77effed3aca56e20d80242a0557f34a --- .../server/pm/PackageManagerService.java | 46 ------------------- 1 file changed, 46 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 7093e9aeb0d42..42ead8e5d116a 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -1736,26 +1736,12 @@ public class PackageManagerService extends IPackageManager.Stub : args.whitelistedRestrictedPermissions; int autoRevokePermissionsMode = args.autoRevokePermissionsMode; - // Handle the parent package handlePackagePostInstall(parentRes, grantPermissions, killApp, virtualPreload, grantedPermissions, whitelistedRestrictedPermissions, autoRevokePermissionsMode, didRestore, args.installSource.installerPackageName, args.observer, args.mDataLoaderType); - // Handle the child packages - final int childCount = (parentRes.addedChildPackages != null) - ? parentRes.addedChildPackages.size() : 0; - for (int i = 0; i < childCount; i++) { - PackageInstalledInfo childRes = parentRes.addedChildPackages.valueAt(i); - handlePackagePostInstall(childRes, grantPermissions, - killApp, virtualPreload, grantedPermissions, - whitelistedRestrictedPermissions, autoRevokePermissionsMode, - false /*didRestore*/, - args.installSource.installerPackageName, args.observer, - args.mDataLoaderType); - } - // Log tracing if needed if (args.traceMethod != null) { Trace.asyncTraceEnd(TRACE_TAG_PACKAGE_MANAGER, args.traceMethod, @@ -15792,7 +15778,6 @@ public class PackageManagerService extends IPackageManager.Stub String returnMsg; String installerPackageName; PackageRemovedInfo removedInfo; - ArrayMap addedChildPackages; // The set of packages consuming this shared library or null if no consumers exist. ArrayList libraryConsumers; PackageFreezer freezer; @@ -15806,37 +15791,21 @@ public class PackageManagerService extends IPackageManager.Stub public void setError(String msg, PackageParserException e) { setReturnCode(e.error); setReturnMessage(ExceptionUtils.getCompleteMessage(msg, e)); - final int childCount = (addedChildPackages != null) ? addedChildPackages.size() : 0; - for (int i = 0; i < childCount; i++) { - addedChildPackages.valueAt(i).setError(msg, e); - } Slog.w(TAG, msg, e); } public void setError(String msg, PackageManagerException e) { returnCode = e.error; setReturnMessage(ExceptionUtils.getCompleteMessage(msg, e)); - final int childCount = (addedChildPackages != null) ? addedChildPackages.size() : 0; - for (int i = 0; i < childCount; i++) { - addedChildPackages.valueAt(i).setError(msg, e); - } Slog.w(TAG, msg, e); } public void setReturnCode(int returnCode) { this.returnCode = returnCode; - final int childCount = (addedChildPackages != null) ? addedChildPackages.size() : 0; - for (int i = 0; i < childCount; i++) { - addedChildPackages.valueAt(i).returnCode = returnCode; - } } private void setReturnMessage(String returnMsg) { this.returnMsg = returnMsg; - final int childCount = (addedChildPackages != null) ? addedChildPackages.size() : 0; - for (int i = 0; i < childCount; i++) { - addedChildPackages.valueAt(i).returnMsg = returnMsg; - } } // In some error cases we want to convey more info back to the observer @@ -17386,7 +17355,6 @@ public class PackageManagerService extends IPackageManager.Stub int targetParseFlags = parseFlags; final PackageSetting ps; final PackageSetting disabledPs; - final PackageSetting[] childPackages; if (replace) { if (parsedPackage.isStaticSharedLibrary()) { // Static libs have a synthetic package name containing the version @@ -18388,7 +18356,6 @@ public class PackageManagerService extends IPackageManager.Stub final boolean killApp = (deleteFlags & PackageManager.DELETE_DONT_KILL_APP) == 0; info.sendPackageRemovedBroadcasts(killApp); info.sendSystemPackageUpdatedBroadcasts(); - info.sendSystemPackageAppearedBroadcasts(); } // Force a gc here. Runtime.getRuntime().gc(); @@ -18446,7 +18413,6 @@ public class PackageManagerService extends IPackageManager.Stub SparseArray broadcastWhitelist; // Clean up resources deleted packages. InstallArgs args = null; - ArrayMap appearedChildPackages; PackageRemovedInfo(PackageSender packageSender) { this.packageSender = packageSender; @@ -18462,18 +18428,6 @@ public class PackageManagerService extends IPackageManager.Stub } } - void sendSystemPackageAppearedBroadcasts() { - final int packageCount = (appearedChildPackages != null) - ? appearedChildPackages.size() : 0; - for (int i = 0; i < packageCount; i++) { - PackageInstalledInfo installedInfo = appearedChildPackages.valueAt(i); - packageSender.sendPackageAddedForNewUsers(installedInfo.name, - true /*sendBootCompleted*/, false /*startReceiver*/, - UserHandle.getAppId(installedInfo.uid), installedInfo.newUsers, null, - DataLoaderType.NONE); - } - } - private void sendSystemPackageUpdatedBroadcastsInternal() { Bundle extras = new Bundle(2); extras.putInt(Intent.EXTRA_UID, removedAppId >= 0 ? removedAppId : uid);