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

Commit 0248b5c8 authored by Jeff DeCew's avatar Jeff DeCew
Browse files

NotifCollection.dismissNotifications will now remove hidden summaries. 

Previously we would take the list of notifications exactly, but that left room that a notification's summary, if it was not included in the list, would be left in the shade.  We now check if each entry is the sole logic child of a single summary, and if so we include that summary in the dismissal (assuming it was not already included), and generate the necessary stats object.

Bug: 355967751
Flag: com.android.systemui.notifications_dismiss_pruned_summaries
Test: atest NotifCollectionTest
Change-Id: Id3eda2f7a36227e4d5a921888735dd898d33a61a
parent 4eb9e0fa

packages/SystemUI/aconfig/systemui.aconfig

+7 −0
Original line number Diff line number Diff line
@@ -128,6 +128,13 @@ flag { 
    bug: "308623704"

}



flag {

    name: "notifications_dismiss_pruned_summaries"

    namespace: "systemui"

    description: "NotifCollection.dismissNotifications will now dismiss summaries that are pruned from the shade."

    bug: "355967751"

}



flag {

   name: "notification_transparent_header_fix"

   namespace: "systemui"

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

+47 −10
Original line number Diff line number Diff line
@@ -39,6 +39,7 @@ import static android.service.notification.NotificationListenerService.REASON_TI 
import static android.service.notification.NotificationListenerService.REASON_UNAUTOBUNDLED;

import static android.service.notification.NotificationListenerService.REASON_USER_STOPPED;



import static com.android.systemui.Flags.notificationsDismissPrunedSummaries;

import static com.android.systemui.statusbar.notification.NotificationUtils.logKey;

import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.DISMISSED;

import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.NOT_DISMISSED;
@@ -69,6 +70,7 @@ import androidx.annotation.Nullable; 


import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.statusbar.IStatusBarService;

import com.android.internal.statusbar.NotificationVisibility;

import com.android.systemui.Dumpable;

import com.android.systemui.dagger.SysUISingleton;

import com.android.systemui.dagger.qualifiers.Background;
@@ -111,6 +113,7 @@ import java.util.Collection; 
import java.util.Collections;

import java.util.Comparator;

import java.util.HashMap;

import java.util.HashSet;

import java.util.List;

import java.util.Map;

import java.util.Objects;
@@ -277,6 +280,10 @@ public class NotifCollection implements Dumpable, PipelineDumpable { 
        Assert.isMainThread();

        checkForReentrantCall();



        if (notificationsDismissPrunedSummaries()) {

            entriesToDismiss = includeSummariesToDismiss(entriesToDismiss);

        }



        final int entryCount = entriesToDismiss.size();

        final List<NotificationEntry> entriesToLocallyDismiss = new ArrayList<>();

        for (int i = 0; i < entriesToDismiss.size(); i++) {
@@ -336,6 +343,36 @@ public class NotifCollection implements Dumpable, PipelineDumpable { 
        dispatchEventsAndRebuildList("dismissNotifications");

    }



    private List<Pair<NotificationEntry, DismissedByUserStats>> includeSummariesToDismiss(

            List<Pair<NotificationEntry, DismissedByUserStats>> entriesToDismiss) {

        final HashSet<NotificationEntry> entriesSet = new HashSet<>(entriesToDismiss.size());

        for (Pair<NotificationEntry, DismissedByUserStats> entryToStats : entriesToDismiss) {

            entriesSet.add(entryToStats.first);

        }



        final List<Pair<NotificationEntry, DismissedByUserStats>> entriesPlusSummaries =

                new ArrayList<>(entriesToDismiss.size() + 1);

        for (Pair<NotificationEntry, DismissedByUserStats> entryToStats : entriesToDismiss) {

            entriesPlusSummaries.add(entryToStats);

            NotificationEntry summary = fetchSummaryToDismiss(entryToStats.first);

            if (summary != null && !entriesSet.contains(summary)) {

                DismissedByUserStats currentStats = entryToStats.second;

                NotificationVisibility summaryVisibility = NotificationVisibility.obtain(

                        summary.getKey(),

                        summary.getRanking().getRank(),

                        currentStats.notificationVisibility.count,

                        /* visible= */ false);

                DismissedByUserStats summaryStats = new DismissedByUserStats(

                        currentStats.dismissalSurface,

                        currentStats.dismissalSentiment,

                        summaryVisibility

                );

                entriesPlusSummaries.add(new Pair<>(summary, summaryStats));

            }

        }

        return entriesPlusSummaries;

    }



    /**

     * Dismisses a single notification on behalf of the user.

     */
@@ -1062,6 +1099,16 @@ public class NotifCollection implements Dumpable, PipelineDumpable { 
        }

    }



    @Nullable

    private NotificationEntry fetchSummaryToDismiss(NotificationEntry entry) {

        if (isOnlyChildInGroup(entry)) {

            String group = entry.getSbn().getGroupKey();

            NotificationEntry summary = getGroupSummary(group);

            if (summary != null && isDismissable(summary)) return summary;

        }

        return null;

    }



    /** A single method interface that callers can pass in when registering future dismissals */

    public interface DismissedByUserStatsCreator {

        DismissedByUserStats createDismissedByUserStats(NotificationEntry entry);
@@ -1092,16 +1139,6 @@ public class NotifCollection implements Dumpable, PipelineDumpable { 
                    + ">";

        }



        @Nullable

        private NotificationEntry fetchSummaryToDismiss(NotificationEntry entry) {

            if (isOnlyChildInGroup(entry)) {

                String group = entry.getSbn().getGroupKey();

                NotificationEntry summary = getGroupSummary(group);

                if (summary != null && isDismissable(summary)) return summary;

            }

            return null;

        }



        /** called when the entry has been removed from the collection */

        public void onSystemServerCancel(@CancellationReason int cancellationReason) {

            Assert.isMainThread();

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

+43 −1
Original line number Diff line number Diff line
@@ -45,6 +45,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean; 
import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.clearInvocations;

import static org.mockito.Mockito.doReturn;

import static org.mockito.Mockito.inOrder;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.never;
@@ -63,6 +64,7 @@ import android.app.NotificationChannel; 
import android.app.NotificationManager;

import android.os.Handler;

import android.os.RemoteException;

import android.platform.test.annotations.EnableFlags;

import android.service.notification.NotificationListenerService.Ranking;

import android.service.notification.NotificationListenerService.RankingMap;

import android.service.notification.StatusBarNotification;
@@ -77,6 +79,7 @@ import androidx.test.filters.SmallTest; 


import com.android.internal.statusbar.IStatusBarService;

import com.android.internal.statusbar.NotificationVisibility;

import com.android.systemui.Flags;

import com.android.systemui.SysuiTestCase;

import com.android.systemui.dump.DumpManager;

import com.android.systemui.dump.LogBufferEulogizer;
@@ -129,6 +132,7 @@ public class NotifCollectionTest extends SysuiTestCase { 
    @Mock private GroupCoalescer mGroupCoalescer;

    @Spy private RecordingCollectionListener mCollectionListener;

    @Mock private CollectionReadyForBuildListener mBuildListener;

    @Mock private NotificationDismissibilityProvider mDismissibilityProvider;



    @Spy private RecordingLifetimeExtender mExtender1 = new RecordingLifetimeExtender("Extender1");

    @Spy private RecordingLifetimeExtender mExtender2 = new RecordingLifetimeExtender("Extender2");
@@ -160,6 +164,7 @@ public class NotifCollectionTest extends SysuiTestCase { 
        allowTestableLooperAsMainThread();



        when(mEulogizer.record(any(Exception.class))).thenAnswer(i -> i.getArguments()[0]);

        doReturn(Boolean.TRUE).when(mDismissibilityProvider).isDismissable(any());



        mListenerInOrder = inOrder(mCollectionListener);
@@ -172,7 +177,7 @@ public class NotifCollectionTest extends SysuiTestCase { 
                mBgExecutor,

                mEulogizer,

                mock(DumpManager.class),

                mock(NotificationDismissibilityProvider.class));

                mDismissibilityProvider);

        mCollection.attach(mGroupCoalescer);

        mCollection.addCollectionListener(mCollectionListener);

        mCollection.setBuildListener(mBuildListener);
@@ -1378,6 +1383,43 @@ public class NotifCollectionTest extends SysuiTestCase { 
        assertEquals(List.of(mInterceptor1, mInterceptor2), entry2.mDismissInterceptors);

    }



    @Test

    @EnableFlags(Flags.FLAG_NOTIFICATIONS_DISMISS_PRUNED_SUMMARIES)

    public void testDismissNotificationsIncludesPrunedParents() {

        // GIVEN a collection with 2 groups; one has a single child, one has two.

        mCollection.addNotificationDismissInterceptor(mInterceptor1);



        NotifEvent notif1summary = mNoMan.postNotif(

                buildNotif(TEST_PACKAGE, 1, "notif1summary").setGroup(mContext, "group1")

                        .setGroupSummary(mContext, true));

        NotifEvent notif1child = mNoMan.postNotif(

                buildNotif(TEST_PACKAGE, 1, "notif1child").setGroup(mContext, "group1"));

        NotifEvent notif2summary = mNoMan.postNotif(

                buildNotif(TEST_PACKAGE2, 2, "notif2summary").setGroup(mContext, "group2")

                        .setGroupSummary(mContext, true));

        NotifEvent notif2child1 = mNoMan.postNotif(

                buildNotif(TEST_PACKAGE2, 2, "notif2child1").setGroup(mContext, "group2"));

        NotifEvent notif2child2 = mNoMan.postNotif(

                buildNotif(TEST_PACKAGE2, 2, "notif2child2").setGroup(mContext, "group2"));

        NotificationEntry entry1summary = mCollectionListener.getEntry(notif1summary.key);

        NotificationEntry entry1child = mCollectionListener.getEntry(notif1child.key);

        NotificationEntry entry2summary = mCollectionListener.getEntry(notif2summary.key);

        NotificationEntry entry2child1 = mCollectionListener.getEntry(notif2child1.key);

        NotificationEntry entry2child2 = mCollectionListener.getEntry(notif2child2.key);



        // WHEN one child from each group are manually dismissed together

        mCollection.dismissNotifications(

                List.of(new Pair<>(entry1child, defaultStats(entry1child)),

                        new Pair<>(entry2child1, defaultStats(entry2child1))));



        // THEN the summary for the singleton child is dismissed, but not the other summary

        verify(mInterceptor1).shouldInterceptDismissal(entry1summary);

        verify(mInterceptor1).shouldInterceptDismissal(entry1child);

        verify(mInterceptor1, never()).shouldInterceptDismissal(entry2summary);

        verify(mInterceptor1).shouldInterceptDismissal(entry2child1);

        verify(mInterceptor1, never()).shouldInterceptDismissal(entry2child2);

    }



    @Test

    public void testDismissAllNotificationsCallsRebuildOnce() {

        // GIVEN a collection with a couple notifications