From bb23cbcb2e747e8f827593536a6ff97bb2b72eed Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Tue, 25 May 2021 16:35:37 -0700 Subject: [PATCH] Fix an issue where the bubble wasn't opening If a bubble had been dismissed, but the notif was still visible, when the user clicks on that notif, the onNotificationClick event can occur before onNotificationBubbleChanged. This means that we might remove/cancel a notification that is about to become a bubble, so when onNotificationBubbleChanged is called there isn't a record to create the bubble with. This CL fixes this by skipping the removal if it's a bubble notif where the bubble has been dismissed & the notif canBubble. Test: atest NotificationManagerServiceTest Bug: 189149153 Bug: 183732510 Change-Id: I366c3a317fb6d4c38e7547c31ff9d47dc9b4846e --- .../NotificationManagerService.java | 8 ++-- .../NotificationManagerServiceTest.java | 41 +++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 0bc28405eab3d..7e7c4eb005b8f 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -6815,12 +6815,14 @@ public class NotificationManagerService extends SystemService { mUsageStats.registerClickedByUser(r); } - if (mReason == REASON_LISTENER_CANCEL - && r.getNotification().isBubbleNotification()) { + if ((mReason == REASON_LISTENER_CANCEL + && r.getNotification().isBubbleNotification()) + || (mReason == REASON_CLICK && r.canBubble() + && r.isFlagBubbleRemoved())) { boolean isBubbleSuppressed = r.getNotification().getBubbleMetadata() != null && r.getNotification().getBubbleMetadata().isBubbleSuppressed(); mNotificationDelegate.onBubbleNotificationSuppressionChanged( - r.getKey(), true /* suppressed */, isBubbleSuppressed); + r.getKey(), true /* notifSuppressed */, isBubbleSuppressed); return; } 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 c33287c57377a..3869faedcd3c0 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -7223,6 +7223,47 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mNotificationRecordLogger.event(0)); } + /** + * When something is bubble'd and the bubble is dismissed, but the notification is still + * visible, clicking on the notification shouldn't auto-cancel it because clicking on + * it will produce a bubble. + */ + @Test + public void testNotificationBubbles_bubbleStays_whenClicked_afterBubbleDismissed() + throws Exception { + setUpPrefsForBubbles(PKG, mUid, + true /* global */, + BUBBLE_PREFERENCE_ALL /* app */, + true /* channel */); + + // GIVEN a notification that has the auto cancels flag (cancel on click) and is a bubble + final NotificationRecord nr = generateNotificationRecord(mTestNotificationChannel); + nr.getSbn().getNotification().flags |= FLAG_BUBBLE | FLAG_AUTO_CANCEL; + nr.setAllowBubble(true); + mService.addNotification(nr); + + // And the bubble is dismissed + mService.mNotificationDelegate.onNotificationBubbleChanged(nr.getKey(), + false /* isBubble */, 0 /* bubbleFlags */); + waitForIdle(); + assertTrue(nr.isFlagBubbleRemoved()); + + // WHEN we click the notification + final NotificationVisibility nv = NotificationVisibility.obtain(nr.getKey(), 1, 2, true); + mService.mNotificationDelegate.onNotificationClick(mUid, Binder.getCallingPid(), + nr.getKey(), nv); + waitForIdle(); + + // THEN the bubble should still exist + StatusBarNotification[] notifsAfter = mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifsAfter.length); + + // Check we got the click log + assertEquals(1, mNotificationRecordLogger.numCalls()); + assertEquals(NotificationRecordLogger.NotificationEvent.NOTIFICATION_CLICKED, + mNotificationRecordLogger.event(0)); + } + @Test public void testLoadDefaultApprovedServices_emptyResources() { TestableResources tr = mContext.getOrCreateTestableResources();