From a8ddb7537dc30b3c144afd8ff0b3bed20aaeac2d Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Wed, 5 Aug 2020 12:51:20 -0700 Subject: [PATCH] Clean up AppStateTracker state on package removed Exempted bucket state was leaking post uninstall in a way that would persist till the next install of the same package. Also cleaning up defunct entries from other lists. Clarified bucket state to always use exempted instead of exempt in code. Test: atest FrameworksMockingServicesTests:AppStateTrackerTest Manual: 1. Install an app and put it under battery restriction 2. Install an app and put it on the power exemption list 3. Uninstall both of the apps 4. They should not appear under 'Exempted' or 'Restricted' packages list in `adb shell dumpsys alarm` Bug: 162763690 Bug: 156868557 Change-Id: I710edbb85d811ea0beced3b59fe89f59bcec6dd8 --- .../android/server/AppStateTrackerImpl.java | 117 ++++++++++-------- .../server/alarm/AlarmManagerService.java | 2 +- core/java/android/content/IntentFilter.java | 8 ++ .../android/server/AppStateTrackerTest.java | 64 +++++++++- 4 files changed, 137 insertions(+), 54 deletions(-) diff --git a/apex/jobscheduler/service/java/com/android/server/AppStateTrackerImpl.java b/apex/jobscheduler/service/java/com/android/server/AppStateTrackerImpl.java index 876d73a570af9..8e26052ee08bb 100644 --- a/apex/jobscheduler/service/java/com/android/server/AppStateTrackerImpl.java +++ b/apex/jobscheduler/service/java/com/android/server/AppStateTrackerImpl.java @@ -125,10 +125,11 @@ public class AppStateTrackerImpl implements AppStateTracker { private int[] mTempExemptAppIds = mPowerExemptAllAppIds; /** - * Per-user packages that are in the EXEMPT bucket. + * Per-user packages that are in the EXEMPTED bucket. */ @GuardedBy("mLock") - private final SparseSetArray mExemptBucketPackages = new SparseSetArray<>(); + @VisibleForTesting + final SparseSetArray mExemptedBucketPackages = new SparseSetArray<>(); @GuardedBy("mLock") final ArraySet mListeners = new ArraySet<>(); @@ -180,7 +181,7 @@ public class AppStateTrackerImpl implements AppStateTracker { int ALL_UNEXEMPTED = 3; int ALL_EXEMPTION_LIST_CHANGED = 4; int TEMP_EXEMPTION_LIST_CHANGED = 5; - int EXEMPT_BUCKET_CHANGED = 6; + int EXEMPTED_BUCKET_CHANGED = 6; int FORCE_ALL_CHANGED = 7; int FORCE_APP_STANDBY_FEATURE_FLAG_CHANGED = 8; @@ -195,7 +196,7 @@ public class AppStateTrackerImpl implements AppStateTracker { "ALL_UNEXEMPTED", "ALL_EXEMPTION_LIST_CHANGED", "TEMP_EXEMPTION_LIST_CHANGED", - "EXEMPT_BUCKET_CHANGED", + "EXEMPTED_BUCKET_CHANGED", "FORCE_ALL_CHANGED", "FORCE_APP_STANDBY_FEATURE_FLAG_CHANGED", @@ -338,9 +339,9 @@ public class AppStateTrackerImpl implements AppStateTracker { } /** - * This is called when the EXEMPT bucket is updated. + * This is called when the EXEMPTED bucket is updated. */ - private void onExemptBucketChanged(AppStateTrackerImpl sender) { + private void onExemptedBucketChanged(AppStateTrackerImpl sender) { // This doesn't happen very often, so just re-evaluate all jobs / alarms. updateAllJobs(); unblockAllUnrestrictedAlarms(); @@ -424,6 +425,38 @@ public class AppStateTrackerImpl implements AppStateTracker { mHandler = new MyHandler(looper); } + private final BroadcastReceiver mReceiver = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + final int userId = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, -1); + switch (intent.getAction()) { + case Intent.ACTION_USER_REMOVED: + if (userId > 0) { + mHandler.doUserRemoved(userId); + } + break; + case Intent.ACTION_BATTERY_CHANGED: + synchronized (mLock) { + mIsPluggedIn = (intent.getIntExtra(BatteryManager.EXTRA_PLUGGED, 0) != 0); + } + updateForceAllAppStandbyState(); + break; + case Intent.ACTION_PACKAGE_REMOVED: + if (!intent.getBooleanExtra(Intent.EXTRA_REPLACING, false)) { + final String pkgName = intent.getData().getSchemeSpecificPart(); + final int uid = intent.getIntExtra(Intent.EXTRA_UID, -1); + // No need to notify for state change as all the alarms and jobs should be + // removed too. + mExemptedBucketPackages.remove(userId, pkgName); + mRunAnyRestrictedPackages.remove(Pair.create(uid, pkgName)); + mActiveUids.delete(uid); + mForegroundUids.delete(uid); + } + break; + } + } + }; + /** * Call it when the system is ready. */ @@ -465,8 +498,11 @@ public class AppStateTrackerImpl implements AppStateTracker { IntentFilter filter = new IntentFilter(); filter.addAction(Intent.ACTION_USER_REMOVED); filter.addAction(Intent.ACTION_BATTERY_CHANGED); - filter.addAction(Intent.ACTION_PACKAGE_REMOVED); - mContext.registerReceiver(new MyReceiver(), filter); + mContext.registerReceiver(mReceiver, filter); + + filter = new IntentFilter(Intent.ACTION_PACKAGE_REMOVED); + filter.addDataScheme(IntentFilter.SCHEME_PACKAGE); + mContext.registerReceiver(mReceiver, filter); refreshForcedAppStandbyUidPackagesLocked(); @@ -693,30 +729,6 @@ public class AppStateTrackerImpl implements AppStateTracker { } } - private final class MyReceiver extends BroadcastReceiver { - @Override - public void onReceive(Context context, Intent intent) { - if (Intent.ACTION_USER_REMOVED.equals(intent.getAction())) { - final int userId = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, -1); - if (userId > 0) { - mHandler.doUserRemoved(userId); - } - } else if (Intent.ACTION_BATTERY_CHANGED.equals(intent.getAction())) { - synchronized (mLock) { - mIsPluggedIn = (intent.getIntExtra(BatteryManager.EXTRA_PLUGGED, 0) != 0); - } - updateForceAllAppStandbyState(); - } else if (Intent.ACTION_PACKAGE_REMOVED.equals(intent.getAction()) - && !intent.getBooleanExtra(Intent.EXTRA_REPLACING, false)) { - final int userId = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, -1); - final String pkgName = intent.getData().getSchemeSpecificPart(); - if (mExemptBucketPackages.remove(userId, pkgName)) { - mHandler.notifyExemptBucketChanged(); - } - } - } - } - final class StandbyTracker extends AppIdleStateChangeListener { @Override public void onAppIdleStateChanged(String packageName, int userId, boolean idle, @@ -728,12 +740,12 @@ public class AppStateTrackerImpl implements AppStateTracker { synchronized (mLock) { final boolean changed; if (bucket == UsageStatsManager.STANDBY_BUCKET_EXEMPTED) { - changed = mExemptBucketPackages.add(userId, packageName); + changed = mExemptedBucketPackages.add(userId, packageName); } else { - changed = mExemptBucketPackages.remove(userId, packageName); + changed = mExemptedBucketPackages.remove(userId, packageName); } if (changed) { - mHandler.notifyExemptBucketChanged(); + mHandler.notifyExemptedBucketChanged(); } } } @@ -755,7 +767,7 @@ public class AppStateTrackerImpl implements AppStateTracker { private static final int MSG_FORCE_ALL_CHANGED = 7; private static final int MSG_USER_REMOVED = 8; private static final int MSG_FORCE_APP_STANDBY_FEATURE_FLAG_CHANGED = 9; - private static final int MSG_EXEMPT_BUCKET_CHANGED = 10; + private static final int MSG_EXEMPTED_BUCKET_CHANGED = 10; private static final int MSG_ON_UID_STATE_CHANGED = 11; private static final int MSG_ON_UID_ACTIVE = 12; @@ -803,9 +815,9 @@ public class AppStateTrackerImpl implements AppStateTracker { obtainMessage(MSG_FORCE_APP_STANDBY_FEATURE_FLAG_CHANGED).sendToTarget(); } - public void notifyExemptBucketChanged() { - removeMessages(MSG_EXEMPT_BUCKET_CHANGED); - obtainMessage(MSG_EXEMPT_BUCKET_CHANGED).sendToTarget(); + public void notifyExemptedBucketChanged() { + removeMessages(MSG_EXEMPTED_BUCKET_CHANGED); + obtainMessage(MSG_EXEMPTED_BUCKET_CHANGED).sendToTarget(); } public void doUserRemoved(int userId) { @@ -888,11 +900,11 @@ public class AppStateTrackerImpl implements AppStateTracker { mStatLogger.logDurationStat(Stats.TEMP_EXEMPTION_LIST_CHANGED, start); return; - case MSG_EXEMPT_BUCKET_CHANGED: + case MSG_EXEMPTED_BUCKET_CHANGED: for (Listener l : cloneListeners()) { - l.onExemptBucketChanged(sender); + l.onExemptedBucketChanged(sender); } - mStatLogger.logDurationStat(Stats.EXEMPT_BUCKET_CHANGED, start); + mStatLogger.logDurationStat(Stats.EXEMPTED_BUCKET_CHANGED, start); return; case MSG_FORCE_ALL_CHANGED: @@ -1005,7 +1017,7 @@ public class AppStateTrackerImpl implements AppStateTracker { } cleanUpArrayForUser(mActiveUids, removedUserId); cleanUpArrayForUser(mForegroundUids, removedUserId); - mExemptBucketPackages.remove(removedUserId); + mExemptedBucketPackages.remove(removedUserId); } } @@ -1148,7 +1160,7 @@ public class AppStateTrackerImpl implements AppStateTracker { } final int userId = UserHandle.getUserId(uid); if (mAppStandbyInternal.isAppIdleEnabled() && !mAppStandbyInternal.isInParole() - && mExemptBucketPackages.contains(userId, packageName)) { + && mExemptedBucketPackages.contains(userId, packageName)) { return false; } return mForceAllAppsStandby; @@ -1301,14 +1313,14 @@ public class AppStateTrackerImpl implements AppStateTracker { pw.println("Exempted bucket packages:"); pw.increaseIndent(); - for (int i = 0; i < mExemptBucketPackages.size(); i++) { + for (int i = 0; i < mExemptedBucketPackages.size(); i++) { pw.print("User "); - pw.print(mExemptBucketPackages.keyAt(i)); + pw.print(mExemptedBucketPackages.keyAt(i)); pw.println(); pw.increaseIndent(); - for (int j = 0; j < mExemptBucketPackages.sizeAt(i); j++) { - pw.print(mExemptBucketPackages.valueAt(i, j)); + for (int j = 0; j < mExemptedBucketPackages.sizeAt(i); j++) { + pw.print(mExemptedBucketPackages.valueAt(i, j)); pw.println(); } pw.decreaseIndent(); @@ -1384,12 +1396,13 @@ public class AppStateTrackerImpl implements AppStateTracker { proto.write(AppStateTrackerProto.TEMP_POWER_SAVE_EXEMPT_APP_IDS, appId); } - for (int i = 0; i < mExemptBucketPackages.size(); i++) { - for (int j = 0; j < mExemptBucketPackages.sizeAt(i); j++) { + for (int i = 0; i < mExemptedBucketPackages.size(); i++) { + for (int j = 0; j < mExemptedBucketPackages.sizeAt(i); j++) { final long token2 = proto.start(AppStateTrackerProto.EXEMPTED_BUCKET_PACKAGES); - proto.write(ExemptedPackage.USER_ID, mExemptBucketPackages.keyAt(i)); - proto.write(ExemptedPackage.PACKAGE_NAME, mExemptBucketPackages.valueAt(i, j)); + proto.write(ExemptedPackage.USER_ID, mExemptedBucketPackages.keyAt(i)); + proto.write(ExemptedPackage.PACKAGE_NAME, + mExemptedBucketPackages.valueAt(i, j)); proto.end(token2); } diff --git a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java index 4194fd0068c20..e2f13c5608797 100644 --- a/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java +++ b/apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java @@ -4310,7 +4310,7 @@ public class AlarmManagerService extends SystemService { filter.addAction(Intent.ACTION_PACKAGE_REMOVED); filter.addAction(Intent.ACTION_PACKAGE_RESTARTED); filter.addAction(Intent.ACTION_QUERY_PACKAGE_RESTART); - filter.addDataScheme("package"); + filter.addDataScheme(IntentFilter.SCHEME_PACKAGE); getContext().registerReceiver(this, filter); // Register for events related to sdcard installation. IntentFilter sdFilter = new IntentFilter(); diff --git a/core/java/android/content/IntentFilter.java b/core/java/android/content/IntentFilter.java index ee9bd3d259fb8..7fe29a920931e 100644 --- a/core/java/android/content/IntentFilter.java +++ b/core/java/android/content/IntentFilter.java @@ -276,6 +276,14 @@ public class IntentFilter implements Parcelable { */ public static final String SCHEME_HTTPS = "https"; + /** + * Package scheme + * + * @see #addDataScheme(String) + * @hide + */ + public static final String SCHEME_PACKAGE = "package"; + /** * The value to indicate a wildcard for incoming match arguments. * @hide diff --git a/services/tests/mockingservicestests/src/com/android/server/AppStateTrackerTest.java b/services/tests/mockingservicestests/src/com/android/server/AppStateTrackerTest.java index 2f5e8839504b7..3220dff553d3a 100644 --- a/services/tests/mockingservicestests/src/com/android/server/AppStateTrackerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/AppStateTrackerTest.java @@ -48,6 +48,7 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; +import android.net.Uri; import android.os.BatteryManager; import android.os.Handler; import android.os.Looper; @@ -282,7 +283,7 @@ public class AppStateTrackerTest { eq(ServiceType.FORCE_ALL_APPS_STANDBY), powerSaveObserverCaptor.capture()); - verify(mMockContext).registerReceiver( + verify(mMockContext, times(2)).registerReceiver( receiverCaptor.capture(), any(IntentFilter.class)); verify(mMockAppStandbyInternal).addListener( appIdleStateChangeListenerCaptor.capture()); @@ -1242,6 +1243,67 @@ public class AppStateTrackerTest { assertTrue(instance.isForceAllAppsStandbyEnabled()); } + @Test + public void testStateClearedOnPackageRemoved() throws Exception { + final AppStateTrackerTestable instance = newInstance(); + callStart(instance); + + instance.mActiveUids.put(UID_1, true); + instance.mForegroundUids.put(UID_2, true); + instance.mRunAnyRestrictedPackages.add(Pair.create(UID_1, PACKAGE_1)); + instance.mExemptedBucketPackages.add(UserHandle.getUserId(UID_2), PACKAGE_2); + + // Replace PACKAGE_1, nothing should change + Intent packageRemoved = new Intent(Intent.ACTION_PACKAGE_REMOVED) + .putExtra(Intent.EXTRA_USER_HANDLE, UserHandle.getUserId(UID_1)) + .putExtra(Intent.EXTRA_UID, UID_1) + .putExtra(Intent.EXTRA_REPLACING, true) + .setData(Uri.fromParts(IntentFilter.SCHEME_PACKAGE, PACKAGE_1, null)); + mReceiver.onReceive(mMockContext, packageRemoved); + + assertEquals(1, instance.mActiveUids.size()); + assertEquals(1, instance.mForegroundUids.size()); + assertEquals(1, instance.mRunAnyRestrictedPackages.size()); + assertEquals(1, instance.mExemptedBucketPackages.size()); + + // Replace PACKAGE_2, nothing should change + packageRemoved = new Intent(Intent.ACTION_PACKAGE_REMOVED) + .putExtra(Intent.EXTRA_USER_HANDLE, UserHandle.getUserId(UID_2)) + .putExtra(Intent.EXTRA_UID, UID_2) + .putExtra(Intent.EXTRA_REPLACING, true) + .setData(Uri.fromParts(IntentFilter.SCHEME_PACKAGE, PACKAGE_2, null)); + mReceiver.onReceive(mMockContext, packageRemoved); + + assertEquals(1, instance.mActiveUids.size()); + assertEquals(1, instance.mForegroundUids.size()); + assertEquals(1, instance.mRunAnyRestrictedPackages.size()); + assertEquals(1, instance.mExemptedBucketPackages.size()); + + // Remove PACKAGE_1 + packageRemoved = new Intent(Intent.ACTION_PACKAGE_REMOVED) + .putExtra(Intent.EXTRA_USER_HANDLE, UserHandle.getUserId(UID_1)) + .putExtra(Intent.EXTRA_UID, UID_1) + .setData(Uri.fromParts(IntentFilter.SCHEME_PACKAGE, PACKAGE_1, null)); + mReceiver.onReceive(mMockContext, packageRemoved); + + assertEquals(0, instance.mActiveUids.size()); + assertEquals(1, instance.mForegroundUids.size()); + assertEquals(0, instance.mRunAnyRestrictedPackages.size()); + assertEquals(1, instance.mExemptedBucketPackages.size()); + + // Remove PACKAGE_2 + packageRemoved = new Intent(Intent.ACTION_PACKAGE_REMOVED) + .putExtra(Intent.EXTRA_USER_HANDLE, UserHandle.getUserId(UID_2)) + .putExtra(Intent.EXTRA_UID, UID_2) + .setData(Uri.fromParts(IntentFilter.SCHEME_PACKAGE, PACKAGE_2, null)); + mReceiver.onReceive(mMockContext, packageRemoved); + + assertEquals(0, instance.mActiveUids.size()); + assertEquals(0, instance.mForegroundUids.size()); + assertEquals(0, instance.mRunAnyRestrictedPackages.size()); + assertEquals(0, instance.mExemptedBucketPackages.size()); + } + static int[] array(int... appIds) { Arrays.sort(appIds); return appIds;