From ef4b48de59cfd432b17bf212eca8db9bbec5054e Mon Sep 17 00:00:00 2001 From: Winson Chiu Date: Mon, 19 Sep 2022 23:03:01 +0000 Subject: Only send SUSPENSION_CHANGED for changed packages Additional fix follow up to Ie06df12ff3da6cf209fb3149e6aa80b0e63ce56e. With the write commit refactor, this was sending unnecessary changed broadcasts. This fixes it using a real old vs. new equality check, taking into account both presence and deep object equality. This restricts the broadcasts to only being sent if the package SuspendParams metadata has actually changed. Bug: 245426112 Test: atest SuspendPackagesTest Test: atest android.suspendapps.cts Merged-In: Ic4287d2a7b1b8ae0779b234e5ccd8a7a0787f265 Change-Id: Ic4287d2a7b1b8ae0779b234e5ccd8a7a0787f265 --- .../android/server/pm/SuspendPackageHelper.java | 58 +++++++++++----------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/services/core/java/com/android/server/pm/SuspendPackageHelper.java b/services/core/java/com/android/server/pm/SuspendPackageHelper.java index 7ad4d226d801..eebde3613fac 100644 --- a/services/core/java/com/android/server/pm/SuspendPackageHelper.java +++ b/services/core/java/com/android/server/pm/SuspendPackageHelper.java @@ -110,12 +110,12 @@ public final class SuspendPackageHelper { final SuspendParams newSuspendParams = new SuspendParams(dialogInfo, appExtras, launcherExtras); - final List changedPackagesList = new ArrayList<>(packageNames.length); - final IntArray changedUids = new IntArray(packageNames.length); - final IntArray modifiedUids = new IntArray(packageNames.length); final List unmodifiablePackages = new ArrayList<>(packageNames.length); - ArraySet modifiedPackages = new ArraySet<>(); + final List notifyPackagesList = new ArrayList<>(packageNames.length); + final IntArray notifyUids = new IntArray(packageNames.length); + final ArraySet changedPackagesList = new ArraySet<>(packageNames.length); + final IntArray changedUids = new IntArray(packageNames.length); final boolean[] canSuspend = suspended ? canSuspendPackageForUser(snapshot, packageNames, userId, callingUid) : null; @@ -143,21 +143,16 @@ public final class SuspendPackageHelper { final WatchedArrayMap suspendParamsMap = packageState.getUserStateOrDefault(userId).getSuspendParams(); - if (suspended) { - if (suspendParamsMap != null && suspendParamsMap.containsKey(packageName)) { - final SuspendParams suspendParams = suspendParamsMap.get(packageName); - // Skip if there's no changes - if (suspendParams != null - && Objects.equals(suspendParams.getDialogInfo(), dialogInfo) - && Objects.equals(suspendParams.getAppExtras(), appExtras) - && Objects.equals(suspendParams.getLauncherExtras(), - launcherExtras)) { - // Carried over API behavior, must notify change even if no change - changedPackagesList.add(packageName); - changedUids.add(UserHandle.getUid(userId, packageState.getAppId())); - continue; - } - } + + SuspendParams oldSuspendParams = suspendParamsMap == null + ? null : suspendParamsMap.get(packageName); + boolean changed = !Objects.equals(oldSuspendParams, newSuspendParams); + + if (suspended && !changed) { + // Carried over API behavior, must notify change even if no change + notifyPackagesList.add(packageName); + notifyUids.add(UserHandle.getUid(userId, packageState.getAppId())); + continue; } // If only the callingPackage is suspending this package, @@ -166,18 +161,21 @@ public final class SuspendPackageHelper { && CollectionUtils.size(suspendParamsMap) == 1 && suspendParamsMap.containsKey(callingPackage); if (suspended || packageUnsuspended) { + // Always notify of a suspend call + notify when fully unsuspended + notifyPackagesList.add(packageName); + notifyUids.add(UserHandle.getUid(userId, packageState.getAppId())); + } + + if (changed) { changedPackagesList.add(packageName); changedUids.add(UserHandle.getUid(userId, packageState.getAppId())); } - - modifiedPackages.add(packageName); - modifiedUids.add(UserHandle.getUid(userId, packageState.getAppId())); } mPm.commitPackageStateMutation(null, mutator -> { - final int size = modifiedPackages.size(); + final int size = changedPackagesList.size(); for (int index = 0; index < size; index++) { - final String packageName = modifiedPackages.valueAt(index); + final String packageName = changedPackagesList.valueAt(index); final PackageUserStateWrite userState = mutator.forPackage(packageName) .userState(userId); if (suspended) { @@ -190,19 +188,19 @@ public final class SuspendPackageHelper { final Computer newSnapshot = mPm.snapshotComputer(); - if (!changedPackagesList.isEmpty()) { - final String[] changedPackages = changedPackagesList.toArray(new String[0]); + if (!notifyPackagesList.isEmpty()) { + final String[] notifyPackages = notifyPackagesList.toArray(new String[0]); sendPackagesSuspendedForUser(newSnapshot, suspended ? Intent.ACTION_PACKAGES_SUSPENDED : Intent.ACTION_PACKAGES_UNSUSPENDED, - changedPackages, changedUids.toArray(), userId); - sendMyPackageSuspendedOrUnsuspended(changedPackages, suspended, userId); + notifyPackages, notifyUids.toArray(), userId); + sendMyPackageSuspendedOrUnsuspended(notifyPackages, suspended, userId); mPm.scheduleWritePackageRestrictions(userId); } // Send the suspension changed broadcast to ensure suspension state is not stale. - if (!modifiedPackages.isEmpty()) { + if (!changedPackagesList.isEmpty()) { sendPackagesSuspendedForUser(newSnapshot, Intent.ACTION_PACKAGES_SUSPENSION_CHANGED, - modifiedPackages.toArray(new String[0]), modifiedUids.toArray(), userId); + changedPackagesList.toArray(new String[0]), changedUids.toArray(), userId); } return unmodifiablePackages.toArray(new String[0]); } -- cgit v1.2.3-59-g8ed1b