When applying a RankingMap that lacks an entry, remove that entry from the NotifCollection.

This fixes a bug after notification removed messages were dropped during moments of mass removal.  The NotifCollection was detecting this edge case but ignoring it.  Now we'll actually remove the notification and require an add event to get it back.

Note: this feature is still flagged off by default.

Bug: 216384850
Test: atest NotifCollectionTest
Change-Id: Id897c8761e40fd1adedd7c9e5a7bd2f9467d13ed
Merged-In: Id897c8761e40fd1adedd7c9e5a7bd2f9467d13ed

5 files changed, 92 insertions, 1 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/flags/Flags.java b/packages/SystemUI/src/com/android/systemui/flags/Flags.java
index 3f70d4ecc7a2..a54f4c2b62dd 100644
--- a/packages/SystemUI/src/com/android/systemui/flags/Flags.java
+++ b/packages/SystemUI/src/com/android/systemui/flags/Flags.java
@@ -60,6 +60,9 @@ public class Flags {
     public static final ResourceBooleanFlag NOTIFICATION_DRAG_TO_CONTENTS =
             new ResourceBooleanFlag(108, R.bool.config_notificationToContents);
 
+    public static final BooleanFlag REMOVE_UNRANKED_NOTIFICATIONS =
+            new BooleanFlag(109, false);
+
     /***************************************/
     // 200 - keyguard/lockscreen
 
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt
index 478f7aa8ce09..c4947ca2dd90 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt
@@ -56,4 +56,7 @@ class NotifPipelineFlags @Inject constructor(
     fun isSmartspaceDedupingEnabled(): Boolean =
             featureFlags.isEnabled(Flags.SMARTSPACE) &&
                     featureFlags.isEnabled(Flags.SMARTSPACE_DEDUPING)
-}
\ No newline at end of file
+
+    fun removeUnrankedNotifs(): Boolean =
+        featureFlags.isEnabled(Flags.REMOVE_UNRANKED_NOTIFICATIONS)
+}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java
index 38830c2e3300..e345aabc6215 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java
@@ -601,6 +601,12 @@ public class NotifCollection implements Dumpable {
         );
         mNotificationsWithoutRankings = currentEntriesWithoutRankings == null
                 ? Collections.emptySet() : currentEntriesWithoutRankings.keySet();
+        if (currentEntriesWithoutRankings != null && mNotifPipelineFlags.removeUnrankedNotifs()) {
+            for (NotificationEntry entry : currentEntriesWithoutRankings.values()) {
+                entry.mCancellationReason = REASON_UNKNOWN;
+                tryRemoveNotification(entry);
+            }
+        }
         mEventQueue.add(new RankingAppliedEvent());
     }
 
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NoManSimulator.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NoManSimulator.java
index 4507366e3073..ee7d55864931 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NoManSimulator.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NoManSimulator.java
@@ -85,6 +85,11 @@ public class NoManSimulator {
         mRankings.put(key, ranking);
     }
 
+    /** This is for testing error cases: b/216384850 */
+    public Ranking removeRankingWithoutEvent(String key) {
+        return mRankings.remove(key);
+    }
+
     private RankingMap buildRankingMap() {
         return new RankingMap(mRankings.values().toArray(new Ranking[0]));
     }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java
index 958d54230f1c..f286349971d2 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java
@@ -1492,6 +1492,80 @@ public class NotifCollectionTest extends SysuiTestCase {
     }
 
     @Test
+    public void testMissingRankingWhenRemovalFeatureIsDisabled() {
+        // GIVEN a pipeline with one two notifications
+        when(mNotifPipelineFlags.removeUnrankedNotifs()).thenReturn(false);
+        String key1 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 1, "myTag")).key;
+        String key2 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 2, "myTag")).key;
+        NotificationEntry entry1 = mCollectionListener.getEntry(key1);
+        NotificationEntry entry2 = mCollectionListener.getEntry(key2);
+        clearInvocations(mCollectionListener);
+
+        // GIVEN the message for removing key1 gets does not reach NotifCollection
+        Ranking ranking1 = mNoMan.removeRankingWithoutEvent(key1);
+        // WHEN the message for removing key2 arrives
+        mNoMan.retractNotif(entry2.getSbn(), REASON_APP_CANCEL);
+
+        // THEN only entry2 gets removed
+        verify(mCollectionListener).onEntryRemoved(eq(entry2), eq(REASON_APP_CANCEL));
+        verify(mCollectionListener).onEntryCleanUp(eq(entry2));
+        verify(mCollectionListener).onRankingApplied();
+        verifyNoMoreInteractions(mCollectionListener);
+        verify(mLogger).logMissingRankings(eq(List.of(entry1)), eq(1), any());
+        verify(mLogger, never()).logRecoveredRankings(any());
+        clearInvocations(mCollectionListener, mLogger);
+
+        // WHEN a ranking update includes key1 again
+        mNoMan.setRanking(key1, ranking1);
+        mNoMan.issueRankingUpdate();
+
+        // VERIFY that we do nothing but log the 'recovery'
+        verify(mCollectionListener).onRankingUpdate(any());
+        verify(mCollectionListener).onRankingApplied();
+        verifyNoMoreInteractions(mCollectionListener);
+        verify(mLogger, never()).logMissingRankings(any(), anyInt(), any());
+        verify(mLogger).logRecoveredRankings(eq(List.of(key1)));
+    }
+
+    @Test
+    public void testMissingRankingWhenRemovalFeatureIsEnabled() {
+        // GIVEN a pipeline with one two notifications
+        when(mNotifPipelineFlags.removeUnrankedNotifs()).thenReturn(true);
+        String key1 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 1, "myTag")).key;
+        String key2 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 2, "myTag")).key;
+        NotificationEntry entry1 = mCollectionListener.getEntry(key1);
+        NotificationEntry entry2 = mCollectionListener.getEntry(key2);
+        clearInvocations(mCollectionListener);
+
+        // GIVEN the message for removing key1 gets does not reach NotifCollection
+        Ranking ranking1 = mNoMan.removeRankingWithoutEvent(key1);
+        // WHEN the message for removing key2 arrives
+        mNoMan.retractNotif(entry2.getSbn(), REASON_APP_CANCEL);
+
+        // THEN both entry1 and entry2 get removed
+        verify(mCollectionListener).onEntryRemoved(eq(entry2), eq(REASON_APP_CANCEL));
+        verify(mCollectionListener).onEntryRemoved(eq(entry1), eq(REASON_UNKNOWN));
+        verify(mCollectionListener).onEntryCleanUp(eq(entry2));
+        verify(mCollectionListener).onEntryCleanUp(eq(entry1));
+        verify(mCollectionListener).onRankingApplied();
+        verifyNoMoreInteractions(mCollectionListener);
+        verify(mLogger).logMissingRankings(eq(List.of(entry1)), eq(1), any());
+        verify(mLogger, never()).logRecoveredRankings(any());
+        clearInvocations(mCollectionListener, mLogger);
+
+        // WHEN a ranking update includes key1 again
+        mNoMan.setRanking(key1, ranking1);
+        mNoMan.issueRankingUpdate();
+
+        // VERIFY that we do nothing but log the 'recovery'
+        verify(mCollectionListener).onRankingUpdate(any());
+        verify(mCollectionListener).onRankingApplied();
+        verifyNoMoreInteractions(mCollectionListener);
+        verify(mLogger, never()).logMissingRankings(any(), anyInt(), any());
+        verify(mLogger).logRecoveredRankings(eq(List.of(key1)));
+    }
+
+    @Test
     public void testRegisterFutureDismissal() throws RemoteException {
         // GIVEN a pipeline with one notification
         NotifEvent notifEvent = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 47, "myTag"));