From 22f2f076d0d223f403b983224c8d45cadf926179 Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Thu, 18 Apr 2019 13:26:18 -0700 Subject: [PATCH] When summary is dismissed, don't remove bubble children Adds to onNotificationRemoveRequested to factor in: * summary dismissal: in this case the remove isn't intercepted, however, any bubble children in the children of the group are updated to not be visible in the shade * summary cancel: in this case if the summary is canceled the bubble children are also canceled * REASON_GROUP_SUMMARY_CANCELED - this is the indicator that the child is being removed because the summary was dismissed, in this case treat the bubble like its notification has been dismissed Adds to NotificationManagerService so that NoMan won't remove any notifs that are FLAG_BUBBLE because of a group summary removal. Test: atest NotificationManagerServiceTest Manual Tests: 1) User Dismiss - add enough bubbles to auto-bundle the notifications - swipe away the summary => Note that the bubbles still exist but now the dot is gone 2) Clear all (repeat above but clear all rather than swipe away) 3) Click on group - add a group with auto-cancel enabled - click on the group summary => Bubbles from that group should remain but the dot should be gone Bug: 130250809 Change-Id: I2728152665817a0166cee72d969cbfdabc47067d --- .../systemui/bubbles/BubbleController.java | 95 ++++++++++++++-- .../android/systemui/bubbles/BubbleData.java | 17 +++ .../bubbles/BubbleControllerTest.java | 20 +++- .../NotificationManagerService.java | 16 ++- .../NotificationManagerServiceTest.java | 101 +++++++++++++++++- 5 files changed, 232 insertions(+), 17 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java index 7ef22c206dd37..ca4c56f1e1e64 100644 --- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java +++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java @@ -16,12 +16,15 @@ package com.android.systemui.bubbles; +import static android.app.Notification.FLAG_AUTOGROUP_SUMMARY; import static android.app.Notification.FLAG_BUBBLE; import static android.content.pm.ActivityInfo.DOCUMENT_LAUNCH_ALWAYS; import static android.service.notification.NotificationListenerService.REASON_APP_CANCEL; import static android.service.notification.NotificationListenerService.REASON_APP_CANCEL_ALL; import static android.service.notification.NotificationListenerService.REASON_CANCEL; import static android.service.notification.NotificationListenerService.REASON_CANCEL_ALL; +import static android.service.notification.NotificationListenerService.REASON_CLICK; +import static android.service.notification.NotificationListenerService.REASON_GROUP_SUMMARY_CANCELED; import static android.view.Display.DEFAULT_DISPLAY; import static android.view.Display.INVALID_DISPLAY; import static android.view.View.INVISIBLE; @@ -82,6 +85,7 @@ import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.NotificationInterruptionStateProvider; import com.android.systemui.statusbar.notification.collection.NotificationData; import com.android.systemui.statusbar.notification.collection.NotificationEntry; +import com.android.systemui.statusbar.phone.NotificationGroupManager; import com.android.systemui.statusbar.phone.StatusBarWindowController; import com.android.systemui.statusbar.policy.ConfigurationController; import com.android.systemui.statusbar.policy.ZenModeController; @@ -90,6 +94,7 @@ import java.io.FileDescriptor; import java.io.PrintWriter; import java.lang.annotation.Retention; import java.lang.annotation.Target; +import java.util.ArrayList; import java.util.List; import javax.inject.Inject; @@ -109,7 +114,7 @@ public class BubbleController implements ConfigurationController.ConfigurationLi @Retention(SOURCE) @IntDef({DISMISS_USER_GESTURE, DISMISS_AGED, DISMISS_TASK_FINISHED, DISMISS_BLOCKED, DISMISS_NOTIF_CANCEL, DISMISS_ACCESSIBILITY_ACTION, DISMISS_NO_LONGER_BUBBLE, - DISMISS_USER_CHANGED}) + DISMISS_USER_CHANGED, DISMISS_GROUP_CANCELLED}) @Target({FIELD, LOCAL_VARIABLE, PARAMETER}) @interface DismissReason {} @@ -121,6 +126,7 @@ public class BubbleController implements ConfigurationController.ConfigurationLi static final int DISMISS_ACCESSIBILITY_ACTION = 6; static final int DISMISS_NO_LONGER_BUBBLE = 7; static final int DISMISS_USER_CHANGED = 8; + static final int DISMISS_GROUP_CANCELLED = 9; public static final int MAX_BUBBLES = 5; // TODO: actually enforce this @@ -133,6 +139,7 @@ public class BubbleController implements ConfigurationController.ConfigurationLi private BubbleStateChangeListener mStateChangeListener; private BubbleExpandListener mExpandListener; @Nullable private BubbleStackView.SurfaceSynchronizer mSurfaceSynchronizer; + private final NotificationGroupManager mNotificationGroupManager; private BubbleData mBubbleData; @Nullable private BubbleStackView mStackView; @@ -211,10 +218,11 @@ public class BubbleController implements ConfigurationController.ConfigurationLi BubbleData data, ConfigurationController configurationController, NotificationInterruptionStateProvider interruptionStateProvider, ZenModeController zenModeController, - NotificationLockscreenUserManager notifUserManager) { + NotificationLockscreenUserManager notifUserManager, + NotificationGroupManager groupManager) { this(context, statusBarWindowController, data, null /* synchronizer */, configurationController, interruptionStateProvider, zenModeController, - notifUserManager); + notifUserManager, groupManager); } public BubbleController(Context context, StatusBarWindowController statusBarWindowController, @@ -222,7 +230,8 @@ public class BubbleController implements ConfigurationController.ConfigurationLi ConfigurationController configurationController, NotificationInterruptionStateProvider interruptionStateProvider, ZenModeController zenModeController, - NotificationLockscreenUserManager notifUserManager) { + NotificationLockscreenUserManager notifUserManager, + NotificationGroupManager groupManager) { mContext = context; mNotificationInterruptionStateProvider = interruptionStateProvider; mNotifUserManager = notifUserManager; @@ -251,6 +260,7 @@ public class BubbleController implements ConfigurationController.ConfigurationLi mNotificationEntryManager = Dependency.get(NotificationEntryManager.class); mNotificationEntryManager.addNotificationEntryListener(mEntryListener); mNotificationEntryManager.setNotificationRemoveInterceptor(mRemoveInterceptor); + mNotificationGroupManager = groupManager; mStatusBarWindowController = statusBarWindowController; mStatusBarStateListener = new StatusBarStateListener(); @@ -500,24 +510,38 @@ public class BubbleController implements ConfigurationController.ConfigurationLi new NotificationRemoveInterceptor() { @Override public boolean onNotificationRemoveRequested(String key, int reason) { - if (!mBubbleData.hasBubbleWithKey(key)) { + NotificationEntry entry = mNotificationEntryManager.getNotificationData().get(key); + String groupKey = entry != null ? entry.notification.getGroupKey() : null; + ArrayList bubbleChildren = mBubbleData.getBubblesInGroup(groupKey); + + boolean inBubbleData = mBubbleData.hasBubbleWithKey(key); + boolean isSummary = entry != null + && entry.notification.getNotification().isGroupSummary(); + boolean isSummaryOfBubbles = isSummary && bubbleChildren != null + && !bubbleChildren.isEmpty(); + + if (!inBubbleData && !isSummaryOfBubbles) { return false; } - Bubble bubble = mBubbleData.getBubbleWithKey(key); - NotificationEntry entry = bubble.getEntry(); final boolean isClearAll = reason == REASON_CANCEL_ALL; - final boolean isUserDimiss = reason == REASON_CANCEL; + final boolean isUserDimiss = reason == REASON_CANCEL || reason == REASON_CLICK; final boolean isAppCancel = reason == REASON_APP_CANCEL || reason == REASON_APP_CANCEL_ALL; + final boolean isSummaryCancel = reason == REASON_GROUP_SUMMARY_CANCELED; // Need to check for !appCancel here because the notification may have // previously been dismissed & entry.isRowDismissed would still be true boolean userRemovedNotif = (entry.isRowDismissed() && !isAppCancel) - || isClearAll || isUserDimiss; + || isClearAll || isUserDimiss || isSummaryCancel; + + if (isSummaryOfBubbles) { + return handleSummaryRemovalInterception(entry, userRemovedNotif); + } // The bubble notification sticks around in the data as long as the bubble is // not dismissed and the app hasn't cancelled the notification. + Bubble bubble = mBubbleData.getBubbleWithKey(key); boolean bubbleExtended = entry.isBubble() && userRemovedNotif; if (bubbleExtended) { bubble.setShowInShadeWhenBubble(false); @@ -536,6 +560,43 @@ public class BubbleController implements ConfigurationController.ConfigurationLi } }; + private boolean handleSummaryRemovalInterception(NotificationEntry summary, + boolean userRemovedNotif) { + String groupKey = summary.notification.getGroupKey(); + ArrayList bubbleChildren = mBubbleData.getBubblesInGroup(groupKey); + + if (userRemovedNotif) { + // If it's a user dismiss we mark the children to be hidden from the shade. + for (int i = 0; i < bubbleChildren.size(); i++) { + Bubble bubbleChild = bubbleChildren.get(i); + // As far as group manager is concerned, once a child is no longer shown + // in the shade, it is essentially removed. + mNotificationGroupManager.onEntryRemoved(bubbleChild.getEntry()); + bubbleChild.setShowInShadeWhenBubble(false); + bubbleChild.setShowBubbleDot(false); + if (mStackView != null) { + mStackView.updateDotVisibility(bubbleChild.getKey()); + } + } + // And since all children are removed, remove the summary. + mNotificationGroupManager.onEntryRemoved(summary); + + // If the summary was auto-generated we don't need to keep that notification around + // because apps can't cancel it; so we only intercept & suppress real summaries. + boolean isAutogroupSummary = (summary.notification.getNotification().flags + & FLAG_AUTOGROUP_SUMMARY) != 0; + return !isAutogroupSummary; + } else { + // Remove any associated bubble children. + for (int i = 0; i < bubbleChildren.size(); i++) { + Bubble bubbleChild = bubbleChildren.get(i); + mBubbleData.notificationEntryRemoved(bubbleChild.getEntry(), + DISMISS_GROUP_CANCELLED); + } + return false; + } + } + @SuppressWarnings("FieldCanBeLocal") private final NotificationEntryListener mEntryListener = new NotificationEntryListener() { @Override @@ -597,7 +658,9 @@ public class BubbleController implements ConfigurationController.ConfigurationLi } // Do removals, if any. - for (Pair removed : update.removedBubbles) { + ArrayList> removedBubbles = + new ArrayList<>(update.removedBubbles); + for (Pair removed : removedBubbles) { final Bubble bubble = removed.first; @DismissReason final int reason = removed.second; mStackView.removeBubble(bubble); @@ -622,6 +685,18 @@ public class BubbleController implements ConfigurationController.ConfigurationLi // Bad things have happened } } + + // Check if summary should be removed from NoManGroup + NotificationEntry summary = mNotificationGroupManager.getLogicalGroupSummary( + bubble.getEntry().notification); + if (summary != null) { + ArrayList summaryChildren = + mNotificationGroupManager.getLogicalChildren(summary.notification); + if (summaryChildren == null || summaryChildren.isEmpty()) { + mNotificationEntryManager.performRemoveNotification( + summary.notification, UNDEFINED_DISMISS_REASON); + } + } } } diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java index 49bab1d5bcf8d..269be95dbf8cd 100644 --- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java +++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java @@ -228,6 +228,23 @@ public class BubbleData { dispatchPendingChanges(); } + /** + * Retrieves any bubbles that are part of the notification group represented by the provided + * group key. + */ + ArrayList getBubblesInGroup(@Nullable String groupKey) { + ArrayList bubbleChildren = new ArrayList<>(); + if (groupKey == null) { + return bubbleChildren; + } + for (Bubble b : mBubbles) { + if (groupKey.equals(b.getEntry().notification.getGroupKey())) { + bubbleChildren.add(b); + } + } + return bubbleChildren; + } + private void doAdd(Bubble bubble) { if (DEBUG_BUBBLE_DATA) { Log.d(TAG, "doAdd: " + bubble); diff --git a/packages/SystemUI/tests/src/com/android/systemui/bubbles/BubbleControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/bubbles/BubbleControllerTest.java index 0df4f0cea7f3c..e13acce1a3a76 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/bubbles/BubbleControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/bubbles/BubbleControllerTest.java @@ -70,6 +70,7 @@ import com.android.systemui.statusbar.notification.collection.NotificationData; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; import com.android.systemui.statusbar.phone.DozeParameters; +import com.android.systemui.statusbar.phone.NotificationGroupManager; import com.android.systemui.statusbar.phone.StatusBarWindowController; import com.android.systemui.statusbar.policy.ConfigurationController; import com.android.systemui.statusbar.policy.HeadsUpManager; @@ -91,6 +92,8 @@ public class BubbleControllerTest extends SysuiTestCase { @Mock private NotificationEntryManager mNotificationEntryManager; @Mock + private NotificationGroupManager mNotificationGroupManager; + @Mock private WindowManager mWindowManager; @Mock private IActivityManager mActivityManager; @@ -154,6 +157,7 @@ public class BubbleControllerTest extends SysuiTestCase { // Return non-null notification data from the NEM when(mNotificationEntryManager.getNotificationData()).thenReturn(mNotificationData); + when(mNotificationData.get(mRow.getEntry().key)).thenReturn(mRow.getEntry()); when(mNotificationData.getChannel(mRow.getEntry().key)).thenReturn(mRow.getEntry().channel); mZenModeConfig.suppressedVisualEffects = 0; @@ -168,9 +172,14 @@ public class BubbleControllerTest extends SysuiTestCase { mock(HeadsUpManager.class), mock(NotificationInterruptionStateProvider.HeadsUpSuppressor.class)); mBubbleData = new BubbleData(mContext); - mBubbleController = new TestableBubbleController(mContext, mStatusBarWindowController, - mBubbleData, mConfigurationController, interruptionStateProvider, - mZenModeController, mLockscreenUserManager); + mBubbleController = new TestableBubbleController(mContext, + mStatusBarWindowController, + mBubbleData, + mConfigurationController, + interruptionStateProvider, + mZenModeController, + mLockscreenUserManager, + mNotificationGroupManager); mBubbleController.setBubbleStateChangeListener(mBubbleStateChangeListener); mBubbleController.setExpandListener(mBubbleExpandListener); @@ -596,10 +605,11 @@ public class BubbleControllerTest extends SysuiTestCase { ConfigurationController configurationController, NotificationInterruptionStateProvider interruptionStateProvider, ZenModeController zenModeController, - NotificationLockscreenUserManager lockscreenUserManager) { + NotificationLockscreenUserManager lockscreenUserManager, + NotificationGroupManager groupManager) { super(context, statusBarWindowController, data, Runnable::run, configurationController, interruptionStateProvider, zenModeController, - lockscreenUserManager); + lockscreenUserManager, groupManager); } } diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index abb59bd868607..c681241532d29 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -5249,12 +5249,26 @@ public class NotificationManagerService extends SystemService { return; } + // Bubbled children get to stick around if the summary was manually cancelled + // (user removed) from systemui. + FlagChecker childrenFlagChecker = null; + if (mReason == REASON_CANCEL + || mReason == REASON_CLICK + || mReason == REASON_CANCEL_ALL) { + childrenFlagChecker = (flags) -> { + if ((flags & FLAG_BUBBLE) != 0) { + return false; + } + return true; + }; + } + // Cancel the notification. boolean wasPosted = removeFromNotificationListsLocked(r); cancelNotificationLocked( r, mSendDelete, mReason, mRank, mCount, wasPosted, listenerName); cancelGroupChildrenLocked(r, mCallingUid, mCallingPid, listenerName, - mSendDelete, null); + mSendDelete, childrenFlagChecker); updateLightsLocked(); } else { // No notification was found, assume that it is snoozed and cancel it. diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index d68ef453e63c2..b5bc413414e84 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -20,6 +20,7 @@ import static android.app.ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREG import static android.app.ActivityManager.RunningAppProcessInfo.IMPORTANCE_GONE; import static android.app.ActivityManager.RunningAppProcessInfo.IMPORTANCE_VISIBLE; import static android.app.Notification.CATEGORY_CALL; +import static android.app.Notification.FLAG_AUTO_CANCEL; import static android.app.Notification.FLAG_BUBBLE; import static android.app.Notification.FLAG_FOREGROUND_SERVICE; import static android.app.NotificationManager.EXTRA_BLOCKED_STATE; @@ -439,14 +440,22 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { return sbn; } + private NotificationRecord generateNotificationRecord(NotificationChannel channel, int id, String groupKey, boolean isSummary) { + return generateNotificationRecord(channel, id, groupKey, isSummary, false /* isBubble */); + } + + private NotificationRecord generateNotificationRecord(NotificationChannel channel, int id, + String groupKey, boolean isSummary, boolean isBubble) { Notification.Builder nb = new Notification.Builder(mContext, channel.getId()) .setContentTitle("foo") .setSmallIcon(android.R.drawable.sym_def_app_icon) .setGroup(groupKey) .setGroupSummary(isSummary); - + if (isBubble) { + nb.setBubbleMetadata(getBasicBubbleMetadataBuilder().build()); + } StatusBarNotification sbn = new StatusBarNotification(PKG, PKG, id, "tag", mUid, 0, nb.build(), new UserHandle(mUid), null, 0); return new NotificationRecord(mContext, sbn, channel); @@ -542,6 +551,52 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { .setIcon(Icon.createWithResource(mContext, android.R.drawable.sym_def_app_icon)); } + private NotificationRecord addGroupWithBubblesAndValidateAdded(boolean summaryAutoCancel) + throws RemoteException { + + // Notification that has bubble metadata + NotificationRecord nrBubble = generateNotificationRecord(mTestNotificationChannel, 1, + "BUBBLE_GROUP", false /* isSummary */, true /* isBubble */); + + // Make the package foreground so that we're allowed to be a bubble + when(mActivityManager.getPackageImportance(nrBubble.sbn.getPackageName())).thenReturn( + IMPORTANCE_FOREGROUND); + + mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", + nrBubble.sbn.getId(), nrBubble.sbn.getNotification(), nrBubble.sbn.getUserId()); + waitForIdle(); + + // Make sure we are a bubble + StatusBarNotification[] notifsAfter = mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifsAfter.length); + assertTrue((notifsAfter[0].getNotification().flags & FLAG_BUBBLE) != 0); + + // Plain notification without bubble metadata + NotificationRecord nrPlain = generateNotificationRecord(mTestNotificationChannel, 2, + "BUBBLE_GROUP", false /* isSummary */, false /* isBubble */); + mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", + nrPlain.sbn.getId(), nrPlain.sbn.getNotification(), nrPlain.sbn.getUserId()); + waitForIdle(); + + notifsAfter = mBinderService.getActiveNotifications(PKG); + assertEquals(2, notifsAfter.length); + + // Summary notification for both of those + NotificationRecord nrSummary = generateNotificationRecord(mTestNotificationChannel, 3, + "BUBBLE_GROUP", true /* isSummary */, false /* isBubble */); + if (summaryAutoCancel) { + nrSummary.getNotification().flags |= FLAG_AUTO_CANCEL; + } + mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", + nrSummary.sbn.getId(), nrSummary.sbn.getNotification(), nrSummary.sbn.getUserId()); + waitForIdle(); + + notifsAfter = mBinderService.getActiveNotifications(PKG); + assertEquals(3, notifsAfter.length); + + return nrSummary; + } + @Test public void testCreateNotificationChannels_SingleChannel() throws Exception { final NotificationChannel channel = @@ -5192,4 +5247,48 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { assertTrue(notif.getBubbleMetadata().getAutoExpandBubble()); assertTrue(notif.getBubbleMetadata().isNotificationSuppressed()); } + + @Test + public void testNotificationBubbles_bubbleChildrenStay_whenGroupSummaryDismissed() + throws Exception { + // Bubbles are allowed! + setUpPrefsForBubbles(true /* global */, true /* app */, true /* channel */); + + NotificationRecord nrSummary = addGroupWithBubblesAndValidateAdded( + true /* summaryAutoCancel */); + + // Dismiss summary + final NotificationVisibility nv = NotificationVisibility.obtain(nrSummary.getKey(), 1, 2, + true); + mService.mNotificationDelegate.onNotificationClear(mUid, 0, PKG, nrSummary.sbn.getTag(), + nrSummary.sbn.getId(), nrSummary.getUserId(), nrSummary.getKey(), + NotificationStats.DISMISSAL_SHADE, + NotificationStats.DISMISS_SENTIMENT_NEUTRAL, nv); + waitForIdle(); + + // The bubble should still exist + StatusBarNotification[] notifsAfter = mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifsAfter.length); + } + + @Test + public void testNotificationBubbles_bubbleChildrenStay_whenGroupSummaryClicked() + throws Exception { + // Bubbles are allowed! + setUpPrefsForBubbles(true /* global */, true /* app */, true /* channel */); + + NotificationRecord nrSummary = addGroupWithBubblesAndValidateAdded( + true /* summaryAutoCancel */); + + // Click summary + final NotificationVisibility nv = NotificationVisibility.obtain(nrSummary.getKey(), 1, 2, + true); + mService.mNotificationDelegate.onNotificationClick(mUid, Binder.getCallingPid(), + nrSummary.getKey(), nv); + waitForIdle(); + + // The bubble should still exist + StatusBarNotification[] notifsAfter = mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifsAfter.length); + } }