diff options
2 files changed, 75 insertions, 20 deletions
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 61d69f95c501..bdea24704ef5 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -307,11 +307,15 @@ public class NotificationManagerService extends SystemService { // used as a mutex for access to all active notifications & listeners final Object mNotificationLock = new Object(); + @GuardedBy("mNotificationLock") final ArrayList<NotificationRecord> mNotificationList = new ArrayList<NotificationRecord>(); + @GuardedBy("mNotificationLock") final ArrayMap<String, NotificationRecord> mNotificationsByKey = new ArrayMap<String, NotificationRecord>(); + @GuardedBy("mNotificationLock") final ArrayList<NotificationRecord> mEnqueuedNotifications = new ArrayList<>(); + @GuardedBy("mNotificationLock") final ArrayMap<Integer, ArrayMap<String, String>> mAutobundledSummaries = new ArrayMap<>(); final ArrayList<ToastRecord> mToastQueue = new ArrayList<ToastRecord>(); final ArrayMap<String, NotificationRecord> mSummaryByGroupKey = new ArrayMap<>(); @@ -2806,7 +2810,8 @@ public class NotificationManagerService extends SystemService { // Clear summary. final NotificationRecord removed = findNotificationByKeyLocked(summaries.remove(pkg)); if (removed != null) { - cancelNotificationLocked(removed, false, REASON_UNAUTOBUNDLED); + boolean wasPosted = removeFromNotificationListsLocked(removed); + cancelNotificationLocked(removed, false, REASON_UNAUTOBUNDLED, wasPosted); } } } @@ -3420,7 +3425,8 @@ public class NotificationManagerService extends SystemService { .setType(MetricsEvent.TYPE_CLOSE) .addTaggedData(MetricsEvent.NOTIFICATION_SNOOZED_CRITERIA, mSnoozeCriterionId == null ? 0 : 1)); - cancelNotificationLocked(r, false, REASON_SNOOZED); + boolean wasPosted = removeFromNotificationListsLocked(r); + cancelNotificationLocked(r, false, REASON_SNOOZED, wasPosted); updateLightsLocked(); if (mSnoozeCriterionId != null) { mNotificationAssistants.notifyAssistantSnoozedLocked(r.sbn, mSnoozeCriterionId); @@ -4206,15 +4212,18 @@ public class NotificationManagerService extends SystemService { manager.sendAccessibilityEvent(event); } + /** + * Removes all NotificationsRecords with the same key as the given notification record + * from both lists. Do not call this method while iterating over either list. + */ @GuardedBy("mNotificationLock") - private void cancelNotificationLocked(NotificationRecord r, boolean sendDelete, int reason) { - final String canceledKey = r.getKey(); - - // Remove from both lists, either list could have a separate Record for what is effectively - // the same notification. + private boolean removeFromNotificationListsLocked(NotificationRecord r) { + // Remove from both lists, either list could have a separate Record for what is + // effectively the same notification. boolean wasPosted = false; NotificationRecord recordInList = null; - if ((recordInList = findNotificationByListLocked(mNotificationList, r.getKey())) != null) { + if ((recordInList = findNotificationByListLocked(mNotificationList, r.getKey())) + != null) { mNotificationList.remove(recordInList); mNotificationsByKey.remove(recordInList.sbn.getKey()); wasPosted = true; @@ -4223,6 +4232,13 @@ public class NotificationManagerService extends SystemService { != null) { mEnqueuedNotifications.remove(recordInList); } + return wasPosted; + } + + @GuardedBy("mNotificationLock") + private void cancelNotificationLocked(NotificationRecord r, boolean sendDelete, int reason, + boolean wasPosted) { + final String canceledKey = r.getKey(); // Record caller. recordCallerLocked(r); @@ -4363,7 +4379,8 @@ public class NotificationManagerService extends SystemService { } // Cancel the notification. - cancelNotificationLocked(r, sendDelete, reason); + boolean wasPosted = removeFromNotificationListsLocked(r); + cancelNotificationLocked(r, sendDelete, reason, wasPosted); cancelGroupChildrenLocked(r, callingUid, callingPid, listenerName, sendDelete); updateLightsLocked(); @@ -4440,11 +4457,11 @@ public class NotificationManagerService extends SystemService { cancelAllNotificationsByListLocked(mNotificationList, callingUid, callingPid, pkg, true /*nullPkgIndicatesUserSwitch*/, channelId, flagChecker, false /*includeCurrentProfiles*/, userId, false /*sendDelete*/, reason, - listenerName); + listenerName, true /* wasPosted */); cancelAllNotificationsByListLocked(mEnqueuedNotifications, callingUid, callingPid, pkg, true /*nullPkgIndicatesUserSwitch*/, channelId, flagChecker, false /*includeCurrentProfiles*/, userId, - false /*sendDelete*/, reason, listenerName); + false /*sendDelete*/, reason, listenerName, false /* wasPosted */); mSnoozeHelper.cancel(userId, pkg); } } @@ -4460,7 +4477,7 @@ public class NotificationManagerService extends SystemService { private void cancelAllNotificationsByListLocked(ArrayList<NotificationRecord> notificationList, int callingUid, int callingPid, String pkg, boolean nullPkgIndicatesUserSwitch, String channelId, FlagChecker flagChecker, boolean includeCurrentProfiles, int userId, - boolean sendDelete, int reason, String listenerName) { + boolean sendDelete, int reason, String listenerName, boolean wasPosted) { ArrayList<NotificationRecord> canceledNotifications = null; for (int i = notificationList.size() - 1; i >= 0; --i) { NotificationRecord r = notificationList.get(i); @@ -4488,8 +4505,9 @@ public class NotificationManagerService extends SystemService { if (canceledNotifications == null) { canceledNotifications = new ArrayList<>(); } + notificationList.remove(i); canceledNotifications.add(r); - cancelNotificationLocked(r, sendDelete, reason); + cancelNotificationLocked(r, sendDelete, reason, wasPosted); } if (canceledNotifications != null) { final int M = canceledNotifications.size(); @@ -4548,11 +4566,11 @@ public class NotificationManagerService extends SystemService { cancelAllNotificationsByListLocked(mNotificationList, callingUid, callingPid, null, false /*nullPkgIndicatesUserSwitch*/, null, flagChecker, includeCurrentProfiles, userId, true /*sendDelete*/, reason, - listenerName); + listenerName, true); cancelAllNotificationsByListLocked(mEnqueuedNotifications, callingUid, callingPid, null, false /*nullPkgIndicatesUserSwitch*/, null, flagChecker, includeCurrentProfiles, userId, true /*sendDelete*/, - reason, listenerName); + reason, listenerName, false); mSnoozeHelper.cancel(userId, includeCurrentProfiles); } } @@ -4569,7 +4587,6 @@ public class NotificationManagerService extends SystemService { } String pkg = r.sbn.getPackageName(); - int userId = r.getUserId(); if (pkg == null) { if (DBG) Log.e(TAG, "No package for group summary: " + r.getKey()); @@ -4577,15 +4594,15 @@ public class NotificationManagerService extends SystemService { } cancelGroupChildrenByListLocked(mNotificationList, r, callingUid, callingPid, listenerName, - sendDelete); + sendDelete, true); cancelGroupChildrenByListLocked(mEnqueuedNotifications, r, callingUid, callingPid, - listenerName, sendDelete); + listenerName, sendDelete, false); } @GuardedBy("mNotificationLock") private void cancelGroupChildrenByListLocked(ArrayList<NotificationRecord> notificationList, NotificationRecord parentNotification, int callingUid, int callingPid, - String listenerName, boolean sendDelete) { + String listenerName, boolean sendDelete, boolean wasPosted) { final String pkg = parentNotification.sbn.getPackageName(); final int userId = parentNotification.getUserId(); final int reason = REASON_GROUP_SUMMARY_CANCELED; @@ -4597,7 +4614,8 @@ public class NotificationManagerService extends SystemService { && (childR.getFlags() & Notification.FLAG_FOREGROUND_SERVICE) == 0) { EventLogTags.writeNotificationCancel(callingUid, callingPid, pkg, childSbn.getId(), childSbn.getTag(), userId, 0, 0, reason, listenerName); - cancelNotificationLocked(childR, sendDelete, reason); + notificationList.remove(i); + cancelNotificationLocked(childR, sendDelete, reason, wasPosted); } } } diff --git a/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java index bd6e379cb913..46c536c76d46 100644 --- a/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -357,6 +357,43 @@ public class NotificationManagerServiceTest extends NotificationTestCase { } @Test + public void testCancelAllNotificationsMultipleEnqueuedDoesNotCrash() throws Exception { + final StatusBarNotification sbn = generateNotificationRecord(null).sbn; + for (int i = 0; i < 10; i++) { + mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag", + sbn.getId(), sbn.getNotification(), sbn.getUserId()); + } + mBinderService.cancelAllNotifications(PKG, sbn.getUserId()); + waitForIdle(); + } + + @Test + public void testCancelGroupSummaryMultipleEnqueuedChildrenDoesNotCrash() throws Exception { + final NotificationRecord parent = generateNotificationRecord( + mTestNotificationChannel, 1, "group1", true); + final NotificationRecord parentAsChild = generateNotificationRecord( + mTestNotificationChannel, 1, "group1", false); + final NotificationRecord child = generateNotificationRecord( + mTestNotificationChannel, 2, "group1", false); + + // fully post parent notification + mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag", + parent.sbn.getId(), parent.sbn.getNotification(), parent.sbn.getUserId()); + waitForIdle(); + + // enqueue the child several times + for (int i = 0; i < 10; i++) { + mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag", + child.sbn.getId(), child.sbn.getNotification(), child.sbn.getUserId()); + } + // make the parent a child, which will cancel the child notification + mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag", + parentAsChild.sbn.getId(), parentAsChild.sbn.getNotification(), + parentAsChild.sbn.getUserId()); + waitForIdle(); + } + + @Test public void testCancelAllNotifications_IgnoreForegroundService() throws Exception { final StatusBarNotification sbn = generateNotificationRecord(null).sbn; sbn.getNotification().flags |= Notification.FLAG_FOREGROUND_SERVICE; |