From 6e20050815772a210fab3818c0643f51d5ce1d2f Mon Sep 17 00:00:00 2001 From: Jay Aliomer Date: Tue, 16 Nov 2021 15:57:22 -0500 Subject: [PATCH] Use logical notif groups when dismissign summary by user Fixes: 205679661 Test: NotifCollectionTest Change-Id: Id3a418dd9c57aff9cb8327f5c6bd68651e74d3ac --- .../collection/NotifCollection.java | 41 ++++++++++++- .../OnUserInteractionCallbackImpl.java | 9 ++- .../notifcollection/NotifCollectionLogger.kt | 8 +++ .../collection/NotifCollectionTest.java | 59 ++++++++++++++----- 4 files changed, 95 insertions(+), 22 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 f36f430fc29bb..e097244fb7d7a 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 @@ -245,9 +245,15 @@ public class NotifCollection implements Dumpable { DismissedByUserStats stats = entriesToDismiss.get(i).second; requireNonNull(stats); - if (entry != mNotificationSet.get(entry.getKey())) { + NotificationEntry storedEntry = mNotificationSet.get(entry.getKey()); + if (storedEntry == null) { + mLogger.logNonExistentNotifDismissed(entry.getKey()); + continue; + } + if (entry != storedEntry) { throw mEulogizer.record( - new IllegalStateException("Invalid entry: " + entry.getKey())); + new IllegalStateException("Invalid entry: " + + "different stored and dismissed entries for " + entry.getKey())); } if (entry.getDismissState() == DISMISSED) { @@ -489,6 +495,37 @@ public class NotifCollection implements Dumpable { } } + /** + * Get the group summary entry + * @param group + * @return + */ + @Nullable + public NotificationEntry getGroupSummary(String group) { + return mNotificationSet + .values() + .stream() + .filter(it -> Objects.equals(it.getSbn().getGroup(), group)) + .filter(it -> it.getSbn().getNotification().isGroupSummary()) + .findFirst().orElse(null); + } + + /** + * Checks if the entry is the only child in the logical group + * @param entry + * @return + */ + public boolean isOnlyChildInGroup(NotificationEntry entry) { + String group = entry.getSbn().getGroup(); + return mNotificationSet.get(entry.getKey()) == entry + && mNotificationSet + .values() + .stream() + .filter(it -> Objects.equals(it.getSbn().getGroup(), group)) + .filter(it -> !it.getSbn().getNotification().isGroupSummary()) + .count() == 1; + } + private void applyRanking(@NonNull RankingMap rankingMap) { for (NotificationEntry entry : mNotificationSet.values()) { if (!isCanceled(entry)) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/OnUserInteractionCallbackImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/OnUserInteractionCallbackImpl.java index 66ec30d6ee079..f50038c08bd39 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/OnUserInteractionCallbackImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/OnUserInteractionCallbackImpl.java @@ -108,11 +108,10 @@ public class OnUserInteractionCallbackImpl implements OnUserInteractionCallback @Override @Nullable public NotificationEntry getGroupSummaryToDismiss(NotificationEntry entry) { - if (entry.getParent() != null - && entry.getParent().getSummary() != null - && mGroupMembershipManager.isOnlyChildInGroup(entry)) { - NotificationEntry groupSummary = entry.getParent().getSummary(); - return groupSummary.isClearable() ? groupSummary : null; + String group = entry.getSbn().getGroup(); + if (mNotifCollection.isOnlyChildInGroup(entry)) { + NotificationEntry summary = mNotifCollection.getGroupSummary(group); + if (summary != null && summary.isClearable()) return summary; } return null; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionLogger.kt index 1ebc66e4c6653..7bfb999ef53dc 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionLogger.kt @@ -81,6 +81,14 @@ class NotifCollectionLogger @Inject constructor( }) } + fun logNonExistentNotifDismissed(key: String) { + buffer.log(TAG, INFO, { + str1 = key + }, { + "DISMISSED Non Existent $str1" + }) + } + fun logChildDismissed(entry: NotificationEntry) { buffer.log(TAG, DEBUG, { str1 = entry.key 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 cf90cef6dae5f..544d90cb61fde 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 @@ -173,6 +173,41 @@ public class NotifCollectionTest extends SysuiTestCase { mNotifHandler.onNotificationsInitialized(); } + @Test + public void testGetGroupSummary() { + assertEquals(null, mCollection.getGroupSummary("group")); + NotifEvent summary = mNoMan.postNotif( + buildNotif(TEST_PACKAGE, 0) + .setGroup(mContext, "group") + .setGroupSummary(mContext, true)); + + final NotificationEntry entry = mCollection.getGroupSummary("group"); + assertEquals(summary.key, entry.getKey()); + assertEquals(summary.sbn, entry.getSbn()); + assertEquals(summary.ranking, entry.getRanking()); + } + + @Test + public void testIsOnlyChildInGroup() { + NotifEvent notif1 = mNoMan.postNotif( + buildNotif(TEST_PACKAGE, 1) + .setGroup(mContext, "group")); + final NotificationEntry entry = mCollection.getEntry(notif1.key); + assertTrue(mCollection.isOnlyChildInGroup(entry)); + + // summaries are not counted + mNoMan.postNotif( + buildNotif(TEST_PACKAGE, 0) + .setGroup(mContext, "group") + .setGroupSummary(mContext, true)); + assertTrue(mCollection.isOnlyChildInGroup(entry)); + + mNoMan.postNotif( + buildNotif(TEST_PACKAGE, 2) + .setGroup(mContext, "group")); + assertFalse(mCollection.isOnlyChildInGroup(entry)); + } + @Test public void testEventDispatchedWhenNotifPosted() { // WHEN a notification is posted @@ -192,6 +227,15 @@ public class NotifCollectionTest extends SysuiTestCase { assertEquals(notif1.ranking, entry.getRanking()); } + @Test + public void testCancelNonExistingNotification() { + NotifEvent notif = mNoMan.postNotif(buildNotif(TEST_PACKAGE2, 88, "barTag")); + NotificationEntry entry = mCollectionListener.getEntry(notif.key); + mCollection.dismissNotification(entry, defaultStats(entry)); + mCollection.dismissNotification(entry, defaultStats(entry)); + mCollection.dismissNotification(entry, defaultStats(entry)); + } + @Test public void testEventDispatchedWhenNotifBatchPosted() { // GIVEN a NotifCollection with one notif already posted @@ -649,21 +693,6 @@ public class NotifCollectionTest extends SysuiTestCase { // THEN an exception is thrown } - @Test(expected = IllegalStateException.class) - public void testDismissingNonExistentNotificationThrows() { - // GIVEN a collection that originally had three notifs, but where one was dismissed - NotifEvent notif1 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 47)); - NotifEvent notif2 = mNoMan.postNotif(buildNotif(TEST_PACKAGE2, 88)); - NotifEvent notif3 = mNoMan.postNotif(buildNotif(TEST_PACKAGE2, 99)); - NotificationEntry entry2 = mCollectionListener.getEntry(notif2.key); - mNoMan.retractNotif(notif2.sbn, REASON_UNKNOWN); - - // WHEN we try to dismiss a notification that isn't present - mCollection.dismissNotification(entry2, defaultStats(entry2)); - - // THEN an exception is thrown - } - @Test public void testGroupChildrenAreDismissedLocallyWhenSummaryIsDismissed() { // GIVEN a collection with two grouped notifs in it