From 5c5b6218e41cf3fafae745715a931ca9f4043707 Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Thu, 28 Jul 2022 10:27:42 -0700 Subject: [PATCH] If the update is non-interruptive don't re-bubble it I haven't been able to reproduce this outside of tests on acloud devices, but if you visit a bubble for the first time (causes an update to the notif) and then dismiss it sometimes the user dismiss can happen before we get the update back from notification manager service and the bubble will get re-bubbled incorrectly. To fix this, if a bubble is in the overflow (or active) and there's an update to the notification that is non-interruptive, we shouldn't re-bubble that content because there wasn't a new message, just some change on the notification. Bug: 238896626 Test: atest BubblesTest Test: atest --no-bazel-mode --iterations 20 PlatformScenarioTests:android.platform.test.scenario.sysui.bubble.ExpandAndDragToDismissTest (with vadim's changes & other CLc) Change-Id: If0b7b3d3d1875ddd1df6a1412db8c42456de35c5 --- .../com/android/wm/shell/bubbles/Bubble.java | 5 ++ .../wm/shell/bubbles/BubbleController.java | 26 ++++++++-- .../android/systemui/wmshell/BubblesTest.java | 51 +++++++++++++++++++ 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/Bubble.java b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/Bubble.java index 31fc6a5be5895..2c02006c8ca5d 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/Bubble.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/Bubble.java @@ -452,6 +452,7 @@ public class Bubble implements BubbleViewProvider { */ void setEntry(@NonNull final BubbleEntry entry) { Objects.requireNonNull(entry); + boolean showingDotPreviously = showDot(); mLastUpdated = entry.getStatusBarNotification().getPostTime(); mIsBubble = entry.getStatusBarNotification().getNotification().isBubbleNotification(); mPackageName = entry.getStatusBarNotification().getPackageName(); @@ -498,6 +499,10 @@ public class Bubble implements BubbleViewProvider { mShouldSuppressNotificationDot = entry.shouldSuppressNotificationDot(); mShouldSuppressNotificationList = entry.shouldSuppressNotificationList(); mShouldSuppressPeek = entry.shouldSuppressPeek(); + if (showingDotPreviously != showDot()) { + // This will update the UI if needed + setShowDot(showDot()); + } } @Nullable diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleController.java index 9c3bc288a58f4..8771ceb71d989 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleController.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleController.java @@ -1055,21 +1055,28 @@ public class BubbleController implements ConfigurationChangeListener { public void updateBubble(BubbleEntry notif, boolean suppressFlyout, boolean showInShade) { // If this is an interruptive notif, mark that it's interrupted mSysuiProxy.setNotificationInterruption(notif.getKey()); - if (!notif.getRanking().isTextChanged() + boolean isNonInterruptiveNotExpanding = !notif.getRanking().isTextChanged() && (notif.getBubbleMetadata() != null - && !notif.getBubbleMetadata().getAutoExpandBubble()) + && !notif.getBubbleMetadata().getAutoExpandBubble()); + if (isNonInterruptiveNotExpanding && mBubbleData.hasOverflowBubbleWithKey(notif.getKey())) { // Update the bubble but don't promote it out of overflow Bubble b = mBubbleData.getOverflowBubbleWithKey(notif.getKey()); if (notif.isBubble()) { notif.setFlagBubble(false); } - b.setEntry(notif); + updateNotNotifyingEntry(b, notif, showInShade); + } else if (mBubbleData.hasAnyBubbleWithKey(notif.getKey()) + && isNonInterruptiveNotExpanding) { + Bubble b = mBubbleData.getAnyBubbleWithkey(notif.getKey()); + if (b != null) { + updateNotNotifyingEntry(b, notif, showInShade); + } } else if (mBubbleData.isSuppressedWithLocusId(notif.getLocusId())) { // Update the bubble but don't promote it out of overflow Bubble b = mBubbleData.getSuppressedBubbleWithKey(notif.getKey()); if (b != null) { - b.setEntry(notif); + updateNotNotifyingEntry(b, notif, showInShade); } } else { Bubble bubble = mBubbleData.getOrCreateBubble(notif, null /* persistedBubble */); @@ -1079,12 +1086,23 @@ public class BubbleController implements ConfigurationChangeListener { if (bubble.shouldAutoExpand()) { bubble.setShouldAutoExpand(false); } + mImpl.mCachedState.updateBubbleSuppressedState(bubble); } else { inflateAndAdd(bubble, suppressFlyout, showInShade); } } } + void updateNotNotifyingEntry(Bubble b, BubbleEntry entry, boolean showInShade) { + boolean isBubbleSelected = Objects.equals(b, mBubbleData.getSelectedBubble()); + boolean isBubbleExpandedAndSelected = isStackExpanded() && isBubbleSelected; + b.setEntry(entry); + boolean suppress = isBubbleExpandedAndSelected || !showInShade || !b.showInShade(); + b.setSuppressNotification(suppress); + b.setShowDot(!isBubbleExpandedAndSelected); + mImpl.mCachedState.updateBubbleSuppressedState(b); + } + @VisibleForTesting public void inflateAndAdd(Bubble bubble, boolean suppressFlyout, boolean showInShade) { // Lazy init stack view when a bubble is created diff --git a/packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java b/packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java index ff95969f1e53d..18acf3f6ce531 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java @@ -1433,6 +1433,57 @@ public class BubblesTest extends SysuiTestCase { assertThat(mBubbleData.hasBubbleInStackWithKey(mBubbleEntry.getKey())).isFalse(); } + /** + * Verifies that if a bubble is in the overflow and a non-interruptive notification update + * comes in for it, it stays in the overflow but the entry is updated. + */ + @Test + public void testNonInterruptiveUpdate_doesntBubbleFromOverflow() { + mEntryListener.onEntryAdded(mRow); + mEntryListener.onEntryUpdated(mRow, /* fromSystem= */ true); + assertBubbleNotificationNotSuppressedFromShade(mBubbleEntry); + + // Dismiss the bubble so it's in the overflow + mBubbleController.removeBubble( + mRow.getKey(), Bubbles.DISMISS_USER_GESTURE); + assertThat(mBubbleData.hasOverflowBubbleWithKey(mRow.getKey())).isTrue(); + + // Update the entry to not show in shade + setMetadataFlags(mRow, + Notification.BubbleMetadata.FLAG_SUPPRESS_NOTIFICATION, /* enableFlag= */ true); + mBubbleController.updateBubble(mBubbleEntry, + /* suppressFlyout= */ false, /* showInShade= */ true); + + // Check that the update was applied - shouldn't be show in shade + assertBubbleNotificationSuppressedFromShade(mBubbleEntry); + // Check that it wasn't inflated (1 because it would've been inflated via onEntryAdded) + verify(mBubbleController, times(1)).inflateAndAdd( + any(Bubble.class), anyBoolean(), anyBoolean()); + } + + /** + * Verifies that if a bubble is active, and a non-interruptive notification update comes in for + * it, it doesn't trigger a new inflate and add for that bubble. + */ + @Test + public void testNonInterruptiveUpdate_doesntTriggerInflate() { + mEntryListener.onEntryAdded(mRow); + mEntryListener.onEntryUpdated(mRow, /* fromSystem= */ true); + assertBubbleNotificationNotSuppressedFromShade(mBubbleEntry); + + // Update the entry to not show in shade + setMetadataFlags(mRow, + Notification.BubbleMetadata.FLAG_SUPPRESS_NOTIFICATION, /* enableFlag= */ true); + mBubbleController.updateBubble(mBubbleEntry, + /* suppressFlyout= */ false, /* showInShade= */ true); + + // Check that the update was applied - shouldn't be show in shade + assertBubbleNotificationSuppressedFromShade(mBubbleEntry); + // Check that it wasn't inflated (1 because it would've been inflated via onEntryAdded) + verify(mBubbleController, times(1)).inflateAndAdd( + any(Bubble.class), anyBoolean(), anyBoolean()); + } + /** * Verifies that if a bubble is in the overflow and a non-interruptive notification update * comes in for it with FLAG_BUBBLE that the flag is removed.