Fix nomanservice notificationrecord leak

Test: runtest systemui-notification
Change-Id: Ic5761b917edd000bf73b3291b0ca95bbb0b9bedb
Fixes: 63585648

diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index 57605bb..a7eb2c6 100644 (file)
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -520,7 +520,8 @@ public class NotificationManagerService extends SystemService {
         }
     }
 
-    private final NotificationDelegate mNotificationDelegate = new NotificationDelegate() {
+    @VisibleForTesting
+    final NotificationDelegate mNotificationDelegate = new NotificationDelegate() {
 
         @Override
         public void onSetDisabled(int status) {
@@ -1012,6 +1013,25 @@ public class NotificationManagerService extends SystemService {
     }
 
     @VisibleForTesting
+    int getNotificationRecordCount() {
+        synchronized (mNotificationLock) {
+            int count = mNotificationList.size() + mNotificationsByKey.size()
+                    + mSummaryByGroupKey.size() + mEnqueuedNotifications.size();
+            // subtract duplicates
+            for (NotificationRecord posted : mNotificationList) {
+                if (mNotificationsByKey.containsKey(posted.getKey())) {
+                    count--;
+                }
+                if (posted.sbn.isGroup() && posted.getNotification().isGroupSummary()) {
+                    count --;
+                }
+            }
+
+            return count;
+        }
+    }
+
+    @VisibleForTesting
     void addNotification(NotificationRecord r) {
         mNotificationList.add(r);
         mNotificationsByKey.put(r.sbn.getKey(), r);
@@ -4553,6 +4573,7 @@ public class NotificationManagerService extends SystemService {
                 canceledNotifications = new ArrayList<>();
             }
             notificationList.remove(i);
+            mNotificationsByKey.remove(r.getKey());
             canceledNotifications.add(r);
             cancelNotificationLocked(r, sendDelete, reason, wasPosted);
         }
@@ -4662,6 +4683,7 @@ public class NotificationManagerService extends SystemService {
                 EventLogTags.writeNotificationCancel(callingUid, callingPid, pkg, childSbn.getId(),
                         childSbn.getTag(), userId, 0, 0, reason, listenerName);
                 notificationList.remove(i);
+                mNotificationsByKey.remove(childR.getKey());
                 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 2e96f6f..8b3fb18 100644 (file)
--- a/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java
+++ b/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -314,9 +314,9 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
         mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag", 0,
                 generateNotificationRecord(null).getNotification(), 0);
         waitForIdle();
-        StatusBarNotification[] notifs =
-                mBinderService.getActiveNotifications(PKG);
+        StatusBarNotification[] notifs = mBinderService.getActiveNotifications(PKG);
         assertEquals(1, notifs.length);
+        assertEquals(1, mNotificationManagerService.getNotificationRecordCount());
     }
 
     @Test
@@ -328,6 +328,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
         StatusBarNotification[] notifs =
                 mBinderService.getActiveNotifications(PKG);
         assertEquals(0, notifs.length);
+        assertEquals(0, mNotificationManagerService.getNotificationRecordCount());
     }
 
     @Test
@@ -342,6 +343,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
         StatusBarNotification[] notifs =
                 mBinderService.getActiveNotifications(PKG);
         assertEquals(0, notifs.length);
+        assertEquals(0, mNotificationManagerService.getNotificationRecordCount());
     }
 
     @Test
@@ -354,6 +356,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
         StatusBarNotification[] notifs =
                 mBinderService.getActiveNotifications(sbn.getPackageName());
         assertEquals(0, notifs.length);
+        assertEquals(0, mNotificationManagerService.getNotificationRecordCount());
     }
 
     @Test
@@ -366,6 +369,43 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
         StatusBarNotification[] notifs =
                 mBinderService.getActiveNotifications(sbn.getPackageName());
         assertEquals(0, notifs.length);
+        assertEquals(0, mNotificationManagerService.getNotificationRecordCount());
+    }
+
+    @Test
+    public void testUserInitiatedClearAll_noLeak() throws Exception {
+        final NotificationRecord n = generateNotificationRecord(
+                mTestNotificationChannel, 1, "group", true);
+
+        mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag",
+                n.sbn.getId(), n.sbn.getNotification(), n.sbn.getUserId());
+        waitForIdle();
+
+        mNotificationManagerService.mNotificationDelegate.onClearAll(uid, Binder.getCallingPid(),
+                n.getUserId());
+        waitForIdle();
+        StatusBarNotification[] notifs =
+                mBinderService.getActiveNotifications(n.sbn.getPackageName());
+        assertEquals(0, notifs.length);
+        assertEquals(0, mNotificationManagerService.getNotificationRecordCount());
+    }
+
+    @Test
+    public void testCancelAllNotificationsCancelsChildren() throws Exception {
+        final NotificationRecord parent = generateNotificationRecord(
+                mTestNotificationChannel, 1, "group1", true);
+        final NotificationRecord child = generateNotificationRecord(
+                mTestNotificationChannel, 2, "group1", false);
+
+        mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag",
+                parent.sbn.getId(), parent.sbn.getNotification(), parent.sbn.getUserId());
+        mBinderService.enqueueNotificationWithTag(PKG, "opPkg", "tag",
+                child.sbn.getId(), child.sbn.getNotification(), child.sbn.getUserId());
+        waitForIdle();
+
+        mBinderService.cancelAllNotifications(PKG, parent.sbn.getUserId());
+        waitForIdle();
+        assertEquals(0, mNotificationManagerService.getNotificationRecordCount());
     }
 
     @Test
@@ -377,6 +417,8 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
         }
         mBinderService.cancelAllNotifications(PKG, sbn.getUserId());
         waitForIdle();
+
+        assertEquals(0, mNotificationManagerService.getNotificationRecordCount());
     }
 
     @Test
@@ -403,6 +445,8 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
                 parentAsChild.sbn.getId(), parentAsChild.sbn.getNotification(),
                 parentAsChild.sbn.getUserId());
         waitForIdle();
+
+        assertEquals(0, mNotificationManagerService.getNotificationRecordCount());
     }
 
     @Test
@@ -416,6 +460,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
         StatusBarNotification[] notifs =
                 mBinderService.getActiveNotifications(sbn.getPackageName());
         assertEquals(1, notifs.length);
+        assertEquals(1, mNotificationManagerService.getNotificationRecordCount());
     }
 
     @Test
@@ -429,6 +474,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
         StatusBarNotification[] notifs =
                 mBinderService.getActiveNotifications(sbn.getPackageName());
         assertEquals(1, notifs.length);
+        assertEquals(1, mNotificationManagerService.getNotificationRecordCount());
     }
 
     @Test
@@ -441,6 +487,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
         StatusBarNotification[] notifs =
                 mBinderService.getActiveNotifications(sbn.getPackageName());
         assertEquals(0, notifs.length);
+        assertEquals(0, mNotificationManagerService.getNotificationRecordCount());
     }
 
     @Test
@@ -454,6 +501,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
         StatusBarNotification[] notifs =
                 mBinderService.getActiveNotifications(sbn.getPackageName());
         assertEquals(1, notifs.length);
+        assertEquals(1, mNotificationManagerService.getNotificationRecordCount());
     }
 
     @Test
@@ -483,6 +531,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
         mBinderService.cancelNotificationWithTag(PKG, "tag", sbn.getId(), sbn.getUserId());
         waitForIdle();
         assertEquals(0, mBinderService.getActiveNotifications(sbn.getPackageName()).length);
+        assertEquals(0, mNotificationManagerService.getNotificationRecordCount());
     }
 
     @Test