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

Commit ef672184 authored by Jeff DeCew's avatar Jeff DeCew
Browse files

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
parent 5cd0e659

packages/SystemUI/src/com/android/systemui/flags/Flags.java

+3 −0
Original line number Diff line number Diff line
@@ -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

packages/SystemUI/src/com/android/systemui/statusbar/notification/NotifPipelineFlags.kt

+4 −1
Original line number Diff line number Diff line
@@ -56,4 +56,7 @@ class NotifPipelineFlags @Inject constructor( 
    fun isSmartspaceDedupingEnabled(): Boolean =

            featureFlags.isEnabled(Flags.SMARTSPACE) &&

                    featureFlags.isEnabled(Flags.SMARTSPACE_DEDUPING)



    fun removeUnrankedNotifs(): Boolean =

        featureFlags.isEnabled(Flags.REMOVE_UNRANKED_NOTIFICATIONS)

}

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java

+6 −0
Original line number Diff line number Diff line
@@ -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());

    }

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

+5 −0
Original line number Diff line number Diff line
@@ -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]));

    }

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

+74 −0
Original line number Diff line number Diff line
@@ -1491,6 +1491,80 @@ public class NotifCollectionTest extends SysuiTestCase { 
        verify(mCollectionListener, never()).onEntryUpdated(any(), anyBoolean());

    }



    @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