Merge "Don't readd pending notifs to NEM's allNotifs list" into rvc-dev am: b937587289

Change-Id: Iab3da75cec001031112c2421b6533866fb0627f2

packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java

@@ -151,6 +151,16 @@ public class NotificationEntryManager implements
     @Override
     public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
         pw.println("NotificationEntryManager state:");
+        pw.println("  mAllNotifications=");
+        if (mAllNotifications.size() == 0) {
+            pw.println("null");
+        } else {
+            int i = 0;
+            for (NotificationEntry entry : mAllNotifications) {
+                dumpEntry(pw, "  ", i, entry);
+                i++;
+            }
+        }
         pw.print("  mPendingNotifications=");
         if (mPendingNotifications.size() == 0) {
             pw.println("null");
@@ -350,8 +360,8 @@ public class NotificationEntryManager implements
     private final NotificationHandler mNotifListener = new NotificationHandler() {
         @Override
         public void onNotificationPosted(StatusBarNotification sbn, RankingMap rankingMap) {
-            final boolean isUpdate = mActiveNotifications.containsKey(sbn.getKey());
+            final boolean isUpdateToInflatedNotif = mActiveNotifications.containsKey(sbn.getKey());
-            if (isUpdate) {
+            if (isUpdateToInflatedNotif) {
                 updateNotification(sbn, rankingMap);
             } else {
                 addNotification(sbn, rankingMap);
@@ -442,16 +452,12 @@ public class NotificationEntryManager implements
                 }
                 if (!lifetimeExtended) {
                     // At this point, we are guaranteed the notification will be removed
+                    abortExistingInflation(key, "removeNotification");
                     mAllNotifications.remove(pendingEntry);
+                    mLeakDetector.trackGarbage(pendingEntry);
                 }
             }
-        }
+        } else {
-
-        if (!lifetimeExtended) {
-            abortExistingInflation(key, "removeNotification");
-        }
-
-        if (entry != null) {
             // If a manager needs to keep the notification around for whatever reason, we
             // keep the notification
             boolean entryDismissed = entry.isRowDismissed();
@@ -469,6 +475,8 @@ public class NotificationEntryManager implements
 
             if (!lifetimeExtended) {
                 // At this point, we are guaranteed the notification will be removed
+                abortExistingInflation(key, "removeNotification");
+                mAllNotifications.remove(entry);
 
                 // Ensure any managers keeping the lifetime extended stop managing the entry
                 cancelLifetimeExtension(entry);
@@ -477,13 +485,10 @@ public class NotificationEntryManager implements
                     entry.removeRow();
                 }
 
-                mAllNotifications.remove(entry);
-
                 // Let's remove the children if this was a summary
                 handleGroupSummaryRemoved(key);
                 removeVisibleNotification(key);
                 updateNotifications("removeNotificationInternal");
-                mLeakDetector.trackGarbage(entry);
                 removedByUser |= entryDismissed;
 
                 mLogger.logNotifRemoved(entry.getKey(), removedByUser);
@@ -497,6 +502,7 @@ public class NotificationEntryManager implements
                 for (NotifCollectionListener listener : mNotifCollectionListeners) {
                     listener.onEntryCleanUp(entry);
                 }
+                mLeakDetector.trackGarbage(entry);
             }
         }
     }
@@ -556,17 +562,24 @@ public class NotificationEntryManager implements
         Ranking ranking = new Ranking();
         rankingMap.getRanking(key, ranking);
 
-        NotificationEntry entry = new NotificationEntry(
+        NotificationEntry entry = mPendingNotifications.get(key);
-                notification,
+        if (entry != null) {
-                ranking,
+            entry.setSbn(notification);
-                mFgsFeatureController.isForegroundServiceDismissalEnabled(),
+        } else {
-                SystemClock.uptimeMillis());
+            entry = new NotificationEntry(
+                    notification,
+                    ranking,
+                    mFgsFeatureController.isForegroundServiceDismissalEnabled(),
+                    SystemClock.uptimeMillis());
+            mAllNotifications.add(entry);
+            mLeakDetector.trackInstance(entry);
+        }
+
+        abortExistingInflation(key, "addNotification");
+
         for (NotifCollectionListener listener : mNotifCollectionListeners) {
             listener.onEntryBind(entry, notification);
         }
-        mAllNotifications.add(entry);
-
-        mLeakDetector.trackInstance(entry);
 
         for (NotifCollectionListener listener : mNotifCollectionListeners) {
             listener.onEntryInit(entry);
@@ -581,7 +594,6 @@ public class NotificationEntryManager implements
                             mInflationCallback);
         }
 
-        abortExistingInflation(key, "addNotification");
         mPendingNotifications.put(key, entry);
         mLogger.logNotifAdded(entry.getKey());
         for (NotificationEntryListener listener : mNotificationEntryListeners) {

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java

@@ -209,6 +209,28 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
         setUserSentiment(mSbn.getKey(), Ranking.USER_SENTIMENT_NEUTRAL);
     }
 
+    @Test
+    public void testAddNotification_noDuplicateEntriesCreated() {
+        // GIVEN a notification has been added
+        mEntryManager.addNotification(mSbn, mRankingMap);
+
+        // WHEN the same notification is added multiple times before the previous entry (with
+        // the same key) didn't finish inflating
+        mEntryManager.addNotification(mSbn, mRankingMap);
+        mEntryManager.addNotification(mSbn, mRankingMap);
+        mEntryManager.addNotification(mSbn, mRankingMap);
+
+        // THEN getAllNotifs() only contains exactly one notification with this key
+        int count = 0;
+        for (NotificationEntry entry : mEntryManager.getAllNotifs()) {
+            if (entry.getKey().equals(mSbn.getKey())) {
+                count++;
+            }
+        }
+        assertEquals("Should only be one entry with key=" + mSbn.getKey() + " in mAllNotifs. "
+                        + "Instead there are " + count, 1, count);
+    }
+
     @Test
     public void testAddNotification_setsUserSentiment() {
         mEntryManager.addNotification(mSbn, mRankingMap);