From 2e6396664dc7258dc3de9b1f38f7dd71d15eb007 Mon Sep 17 00:00:00 2001 From: Ned Burns Date: Thu, 6 Feb 2020 15:51:18 -0500 Subject: [PATCH] Don't auto-dismiss children with certain flags Typically, when a group summary is dismissed, NotificationManager will also dismiss that group's children. In SysUI, we anticipate this change by optimistically dismissing those children on our side before we get confirmation from NoMan. However, there are exceptions to this auto-dismissal behavior, namely if the child is part of a foreground service or a bubble. This CL adds in those exceptions to the SysUI side. Change-Id: Iff03846501af3736897bb61fe825cca911d7584a Test: atest --- .../collection/NotifCollection.java | 26 ++++- .../systemui/statusbar/SbnBuilder.java | 5 + .../collection/NotifCollectionTest.java | 103 ++++++++++++++++++ .../collection/NotificationEntryBuilder.java | 5 + 4 files changed, 137 insertions(+), 2 deletions(-) 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 38d8d979a4dae..ec3285f2b2411 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 @@ -44,6 +44,7 @@ import static java.util.Objects.requireNonNull; import android.annotation.IntDef; import android.annotation.MainThread; import android.annotation.Nullable; +import android.app.Notification; import android.os.RemoteException; import android.service.notification.NotificationListenerService.Ranking; import android.service.notification.NotificationListenerService.RankingMap; @@ -239,8 +240,7 @@ public class NotifCollection implements Dumpable { // Also mark any children as dismissed as system server will auto-dismiss them as well if (entry.getSbn().getNotification().isGroupSummary()) { for (NotificationEntry otherEntry : mNotificationSet.values()) { - if (otherEntry.getSbn().getGroupKey().equals(entry.getSbn().getGroupKey()) - && otherEntry.getDismissState() != DISMISSED) { + if (shouldAutoDismiss(otherEntry, entry.getSbn().getGroupKey())) { otherEntry.setDismissState(PARENT_DISMISSED); if (isCanceled(otherEntry)) { canceledEntries.add(otherEntry); @@ -544,6 +544,28 @@ public class NotifCollection implements Dumpable { return entry.getDismissState() != NOT_DISMISSED; } + /** + * When a group summary is dismissed, NotificationManager will also try to dismiss its children. + * Returns true if we think dismissing the group summary with group key + * dismissedGroupKey will cause NotificationManager to also dismiss + * entry. + * + * See NotificationManager.cancelGroupChildrenByListLocked() for corresponding code. + */ + private static boolean shouldAutoDismiss( + NotificationEntry entry, + String dismissedGroupKey) { + return entry.getSbn().getGroupKey().equals(dismissedGroupKey) + && !entry.getSbn().getNotification().isGroupSummary() + && !hasFlag(entry, Notification.FLAG_FOREGROUND_SERVICE) + && !hasFlag(entry, Notification.FLAG_BUBBLE) + && entry.getDismissState() != DISMISSED; + } + + private static boolean hasFlag(NotificationEntry entry, int flag) { + return (entry.getSbn().getNotification().flags & flag) != 0; + } + private void dispatchOnEntryInit(NotificationEntry entry) { mAmDispatchingToOtherCode = true; for (NotifCollectionListener listener : mNotifCollectionListeners) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/SbnBuilder.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/SbnBuilder.java index 26054027ed5fa..62f406ff835ac 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/SbnBuilder.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/SbnBuilder.java @@ -140,6 +140,11 @@ public class SbnBuilder { return this; } + public SbnBuilder setFlag(Context context, int mask, boolean value) { + modifyNotification(context).setFlag(mask, value); + return this; + } + public Notification.Builder modifyNotification(Context context) { if (mNotification != null) { mNotificationBuilder = new Notification.Builder(context, mNotification); 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 abc0f3ee8a527..96db16adb7dc6 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 @@ -46,9 +46,12 @@ import static org.mockito.Mockito.when; import static java.util.Objects.requireNonNull; import android.annotation.Nullable; +import android.app.Notification; import android.os.RemoteException; import android.service.notification.NotificationListenerService.Ranking; +import android.service.notification.NotificationListenerService.RankingMap; import android.service.notification.NotificationStats; +import android.service.notification.StatusBarNotification; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; import android.util.ArrayMap; @@ -733,6 +736,78 @@ public class NotifCollectionTest extends SysuiTestCase { assertEquals(NOT_DISMISSED, entry1.getDismissState()); } + @Test + public void testDismissingSummaryDoesNotDismissForegroundServiceChildren() { + // GIVEN a collection with three grouped notifs in it + CollectionEvent notif0 = postNotif( + buildNotif(TEST_PACKAGE, 0) + .setGroup(mContext, GROUP_1) + .setGroupSummary(mContext, true)); + CollectionEvent notif1 = postNotif( + buildNotif(TEST_PACKAGE, 1) + .setGroup(mContext, GROUP_1) + .setFlag(mContext, Notification.FLAG_FOREGROUND_SERVICE, true)); + CollectionEvent notif2 = postNotif( + buildNotif(TEST_PACKAGE, 2) + .setGroup(mContext, GROUP_1)); + + // WHEN the summary is dismissed + mCollection.dismissNotification(notif0.entry, defaultStats(notif0.entry)); + + // THEN the foreground service child is not dismissed + assertEquals(DISMISSED, notif0.entry.getDismissState()); + assertEquals(NOT_DISMISSED, notif1.entry.getDismissState()); + assertEquals(PARENT_DISMISSED, notif2.entry.getDismissState()); + } + + @Test + public void testDismissingSummaryDoesNotDismissBubbledChildren() { + // GIVEN a collection with three grouped notifs in it + CollectionEvent notif0 = postNotif( + buildNotif(TEST_PACKAGE, 0) + .setGroup(mContext, GROUP_1) + .setGroupSummary(mContext, true)); + CollectionEvent notif1 = postNotif( + buildNotif(TEST_PACKAGE, 1) + .setGroup(mContext, GROUP_1) + .setFlag(mContext, Notification.FLAG_BUBBLE, true)); + CollectionEvent notif2 = postNotif( + buildNotif(TEST_PACKAGE, 2) + .setGroup(mContext, GROUP_1)); + + // WHEN the summary is dismissed + mCollection.dismissNotification(notif0.entry, defaultStats(notif0.entry)); + + // THEN the bubbled child is not dismissed + assertEquals(DISMISSED, notif0.entry.getDismissState()); + assertEquals(NOT_DISMISSED, notif1.entry.getDismissState()); + assertEquals(PARENT_DISMISSED, notif2.entry.getDismissState()); + } + + @Test + public void testDismissingSummaryDoesNotDismissDuplicateSummaries() { + // GIVEN a group with a two summaries + CollectionEvent notif0 = postNotif( + buildNotif(TEST_PACKAGE, 0) + .setGroup(mContext, GROUP_1) + .setGroupSummary(mContext, true)); + CollectionEvent notif1 = postNotif( + buildNotif(TEST_PACKAGE, 1) + .setGroup(mContext, GROUP_1) + .setGroupSummary(mContext, true)); + CollectionEvent notif2 = postNotif( + buildNotif(TEST_PACKAGE, 2) + .setGroup(mContext, GROUP_1)); + + // WHEN the first summary is dismissed + mCollection.dismissNotification(notif0.entry, defaultStats(notif0.entry)); + + // THEN the second summary is not auto-dismissed (but the child is) + assertEquals(DISMISSED, notif0.entry.getDismissState()); + assertEquals(NOT_DISMISSED, notif1.entry.getDismissState()); + assertEquals(PARENT_DISMISSED, notif2.entry.getDismissState()); + } + @Test public void testLifetimeExtendersAreQueriedWhenNotifRemoved() { // GIVEN a couple notifications and a few lifetime extenders @@ -1000,6 +1075,13 @@ public class NotifCollectionTest extends SysuiTestCase { NotificationVisibility.obtain(entry.getKey(), 7, 2, true)); } + public CollectionEvent postNotif(NotificationEntryBuilder builder) { + clearInvocations(mCollectionListener); + NotifEvent rawEvent = mNoMan.postNotif(builder); + verify(mCollectionListener).onEntryAdded(mEntryCaptor.capture()); + return new CollectionEvent(rawEvent, requireNonNull(mEntryCaptor.getValue())); + } + private static class RecordingCollectionListener implements NotifCollectionListener { private final Map mLastSeenEntries = new ArrayMap<>(); @@ -1014,6 +1096,7 @@ public class NotifCollectionTest extends SysuiTestCase { @Override public void onEntryUpdated(NotificationEntry entry) { + mLastSeenEntries.put(entry.getKey(), entry); } @Override @@ -1098,6 +1181,26 @@ public class NotifCollectionTest extends SysuiTestCase { } } + /** + * Wrapper around {@link NotifEvent} that adds the NotificationEntry that the collection under + * test creates. + */ + private static class CollectionEvent { + public final String key; + public final StatusBarNotification sbn; + public final Ranking ranking; + public final RankingMap rankingMap; + public final NotificationEntry entry; + + private CollectionEvent(NotifEvent rawEvent, NotificationEntry entry) { + this.key = rawEvent.key; + this.sbn = rawEvent.sbn; + this.ranking = rawEvent.ranking; + this.rankingMap = rawEvent.rankingMap; + this.entry = entry; + } + } + private static final String TEST_PACKAGE = "com.android.test.collection"; private static final String TEST_PACKAGE2 = "com.android.test.collection2"; diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationEntryBuilder.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationEntryBuilder.java index 300ec189e31df..d2bb0119c0b34 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationEntryBuilder.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationEntryBuilder.java @@ -162,6 +162,11 @@ public class NotificationEntryBuilder { return this; } + public NotificationEntryBuilder setFlag(Context context, int mask, boolean value) { + mSbnBuilder.setFlag(context, mask, value); + return this; + } + /* Delegated to RankingBuilder */ public NotificationEntryBuilder setRank(int rank) {