From 55524d2a78d4f9f55d509c4f167fed4cfc0135a8 Mon Sep 17 00:00:00 2001
From: Mady Mellor <madym@google.com>
Date: Fri, 22 Jul 2022 14:53:59 -0700
Subject: [PATCH] Don't bubble things when we shouldn't

I haven't been able to repro these issues outside of tests on
cuttlefish, but I think logically these fixes make sense & seem to
resolve the test issue.

- Ensure non-notifying updates to bubbles in the overflow don't
  clobber the FLAG_BUBBLE state
- onRankingUpdate should only bubble stuff up if it's not active and
  not in the overflow (previously just not active)

Bug: 238896626
Test: atest BubblesTest#testNonInterruptiveUpdate_doesntOverrideOverflowFlagBubble
Test: atest --no-bazel-mode --iterations 20 PlatformScenarioTests:android.platform.test.scenario.sysui.bubble.ExpandAndDragToDismissTest
      (with vadim's changes & another CL)
Change-Id: Id588c1130e62533a23dc931dfb4fe3c92125a905
---
 .../wm/shell/bubbles/BubbleController.java    |  5 +++-
 .../android/systemui/wmshell/BubblesTest.java | 29 +++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

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 2b8c1eb455684..9c3bc288a58f4 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
@@ -1061,6 +1061,9 @@ public class BubbleController implements ConfigurationChangeListener {
                 && 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);
         } else if (mBubbleData.isSuppressedWithLocusId(notif.getLocusId())) {
             // Update the bubble but don't promote it out of overflow
@@ -1176,7 +1179,7 @@ public class BubbleController implements ConfigurationChangeListener {
                 // notification, so that the bubble will be re-created if shouldBubbleUp returns
                 // true.
                 mBubbleData.dismissBubbleWithKey(key, DISMISS_NO_BUBBLE_UP);
-            } else if (entry != null && mTmpRanking.isBubble() && !isActive) {
+            } else if (entry != null && mTmpRanking.isBubble() && !isActiveOrInOverflow) {
                 entry.setFlagBubble(true);
                 onEntryUpdated(entry, shouldBubbleUp, /* fromSystem= */ true);
             }
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 25f83fcd4bcca..ff95969f1e53d 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,35 @@ 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 with FLAG_BUBBLE that the flag is removed.
+     */
+    @Test
+    public void testNonInterruptiveUpdate_doesntOverrideOverflowFlagBubble() {
+        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();
+        // Once it's in the overflow it's not actively a bubble (doesn't have FLAG_BUBBLE)
+        Bubble b = mBubbleData.getOverflowBubbleWithKey(mBubbleEntry.getKey());
+        assertThat(b.isBubble()).isFalse();
+
+        // Send a non-notifying update that has FLAG_BUBBLE
+        mRow.getSbn().getNotification().flags = FLAG_BUBBLE;
+        assertThat(mRow.getSbn().getNotification().isBubbleNotification()).isTrue();
+        mBubbleController.updateBubble(mBubbleEntry,
+                /* suppressFlyout= */ false, /* showInShade= */ true);
+
+        // Verify that it still doesn't have FLAG_BUBBLE because it's in the overflow.
+        b = mBubbleData.getOverflowBubbleWithKey(mBubbleEntry.getKey());
+        assertThat(b.isBubble()).isFalse();
+    }
+
     @Test
     public void testNonSystemUpdatesIgnored() {
         mEntryListener.onEntryAdded(mRow);