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

Commit 0839c02b authored by Julia Reynolds's avatar Julia Reynolds
Browse files

Fix two IndexOutOfBoundsException crashes. 

Do not remove (sometimes multiple) items from lists
you are iterating over.

Test: runtest systemui-notification
Change-Id: I130cc63ae2f5721e7b434006f4306e0b1eaef77d
Fixes: 62622503
parent 1a01d129

services/core/java/com/android/server/notification/NotificationManagerService.java

+38 −20
Original line number Diff line number Diff line
@@ -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);

            }

        }

    }

services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java

+37 −0
Original line number Diff line number Diff line
@@ -356,6 +356,43 @@ public class NotificationManagerServiceTest extends NotificationTestCase { 
        assertEquals(0, notifs.length);

    }



    @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;