Merge "[Notifications] Notify NotifCollectionListeners when a pending entry has been removed." into sc-v2-dev

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

@@ -305,9 +305,6 @@ public class NotificationEntryManager implements
             NotificationEntry entry = mPendingNotifications.get(key);
             entry.abortTask();
             mPendingNotifications.remove(key);
-            for (NotifCollectionListener listener : mNotifCollectionListeners) {
-                listener.onEntryCleanUp(entry);
-            }
             mLogger.logInflationAborted(key, "pending", reason);
         }
         NotificationEntry addedEntry = getActiveNotificationUnfiltered(key);
@@ -477,6 +474,18 @@ public class NotificationEntryManager implements
                 if (!lifetimeExtended) {
                     // At this point, we are guaranteed the notification will be removed
                     abortExistingInflation(key, "removeNotification");
+                    // Fix for b/201097913: NotifCollectionListener#onEntryRemoved specifies that
+                    //   #onEntryRemoved should be called when a notification is cancelled,
+                    //   regardless of whether the notification was pending or active.
+                    // Note that mNotificationEntryListeners are NOT notified of #onEntryRemoved
+                    //   because for that interface, #onEntryRemoved should only be called for
+                    //   active entries, NOT pending ones.
+                    for (NotifCollectionListener listener : mNotifCollectionListeners) {
+                        listener.onEntryRemoved(pendingEntry, REASON_UNKNOWN);
+                    }
+                    for (NotifCollectionListener listener : mNotifCollectionListeners) {
+                        listener.onEntryCleanUp(pendingEntry);
+                    }
                     mAllNotifications.remove(pendingEntry);
                     mLeakDetector.trackGarbage(pendingEntry);
                 }

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

@@ -32,6 +32,7 @@ import static org.junit.Assert.assertFalse;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.doAnswer;
@@ -81,6 +82,7 @@ import com.android.systemui.statusbar.notification.collection.NotificationRankin
 import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinder;
 import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy;
 import com.android.systemui.statusbar.notification.collection.notifcollection.DismissedByUserStats;
+import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener;
 import com.android.systemui.statusbar.notification.collection.provider.HighPriorityProvider;
 import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier;
 import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;
@@ -94,6 +96,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
+import org.mockito.ArgumentMatcher;
 import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
@@ -120,6 +123,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
     @Mock private KeyguardEnvironment mEnvironment;
     @Mock private ExpandableNotificationRow mRow;
     @Mock private NotificationEntryListener mEntryListener;
+    @Mock private NotifCollectionListener mNotifCollectionListener;
     @Mock private NotificationRemoveInterceptor mRemoveInterceptor;
     @Mock private HeadsUpManager mHeadsUpManager;
     @Mock private RankingMap mRankingMap;
@@ -215,6 +219,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
                         mEnvironment));
         mEntryManager.setUpWithPresenter(mPresenter);
         mEntryManager.addNotificationEntryListener(mEntryListener);
+        mEntryManager.addCollectionListener(mNotifCollectionListener);
         mEntryManager.addNotificationRemoveInterceptor(mRemoveInterceptor);
 
         setUserSentiment(mSbn.getKey(), Ranking.USER_SENTIMENT_NEUTRAL);
@@ -318,13 +323,20 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
                 eq(mEntry), any(), eq(false) /* removedByUser */, eq(UNDEFINED_DISMISS_REASON));
     }
 
+    /** Regression test for b/201097913. */
     @Test
-    public void testRemoveNotification_whilePending() {
+    public void testRemoveNotification_whilePending_onlyCollectionListenerNotified() {
+        // Add and then remove a pending entry (entry that hasn't been inflated).
         mEntryManager.addNotification(mSbn, mRankingMap);
         mEntryManager.removeNotification(mSbn.getKey(), mRankingMap, UNDEFINED_DISMISS_REASON);
 
+        // Verify that only the listener for the NEW pipeline is notified.
+        // Old pipeline:
         verify(mEntryListener, never()).onEntryRemoved(
-                eq(mEntry), any(), eq(false /* removedByUser */), eq(UNDEFINED_DISMISS_REASON));
+                argThat(matchEntryOnSbn()), any(), anyBoolean(), anyInt());
+        // New pipeline:
+        verify(mNotifCollectionListener).onEntryRemoved(
+                argThat(matchEntryOnSbn()), anyInt());
     }
 
     @Test
@@ -639,6 +651,11 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
                     PendingIntent.FLAG_IMMUTABLE)).build();
     }
 
+    // TODO(b/201321631): Update more tests to use this function instead of eq(mEntry).
+    private ArgumentMatcher<NotificationEntry> matchEntryOnSbn() {
+        return e -> e.getSbn().equals(mSbn);
+    }
+
     private static class FakeNotificationLifetimeExtender implements NotificationLifetimeExtender {
         private NotificationSafeToRemoveCallback mCallback;
         private boolean mExtendLifetimes = true;