From 6a2fc4a9080eeae9b1fb20f433117d7c3657151e Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Wed, 13 Jul 2022 13:35:54 -0700 Subject: [PATCH] Ignore updates to bubbles that aren't from the system The new notif pipeline code adds a concept of updates from the system vs from sysui. Currently bubbles is responding to all updates but I think we really only care about updates from the system (i.e. from NotificationManagerService). Bug: 238896626 Test: atest BubblesTest#testNonSystemUpdatesIgnored Test: atest --no-bazel-mode --iterations 20 PlatformScenarioTests:android.platform.test.scenario.sysui.bubble.ExpandAndDragToDismissTest (with vadim's CLs and a 2nd change to be uploaded) Change-Id: I3ffd6eb0f0a07448b2117924be514347c6e80b16 --- .../wm/shell/bubbles/BubbleController.java | 14 ++++++++----- .../com/android/wm/shell/bubbles/Bubbles.java | 3 ++- .../systemui/wmshell/BubblesManager.java | 9 ++++---- .../android/systemui/wmshell/BubblesTest.java | 21 ++++++++++++++++--- 4 files changed, 34 insertions(+), 13 deletions(-) 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 398261600dc7f..2b8c1eb455684 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 @@ -1082,7 +1082,8 @@ public class BubbleController implements ConfigurationChangeListener { } } - void inflateAndAdd(Bubble bubble, boolean suppressFlyout, boolean showInShade) { + @VisibleForTesting + public void inflateAndAdd(Bubble bubble, boolean suppressFlyout, boolean showInShade) { // Lazy init stack view when a bubble is created ensureStackViewCreated(); bubble.setInflateSynchronously(mInflateSynchronously); @@ -1111,7 +1112,10 @@ public class BubbleController implements ConfigurationChangeListener { } @VisibleForTesting - public void onEntryUpdated(BubbleEntry entry, boolean shouldBubbleUp) { + public void onEntryUpdated(BubbleEntry entry, boolean shouldBubbleUp, boolean fromSystem) { + if (!fromSystem) { + return; + } // shouldBubbleUp checks canBubble & for bubble metadata boolean shouldBubble = shouldBubbleUp && canLaunchInTaskView(mContext, entry); if (!shouldBubble && mBubbleData.hasAnyBubbleWithKey(entry.getKey())) { @@ -1174,7 +1178,7 @@ public class BubbleController implements ConfigurationChangeListener { mBubbleData.dismissBubbleWithKey(key, DISMISS_NO_BUBBLE_UP); } else if (entry != null && mTmpRanking.isBubble() && !isActive) { entry.setFlagBubble(true); - onEntryUpdated(entry, shouldBubbleUp); + onEntryUpdated(entry, shouldBubbleUp, /* fromSystem= */ true); } } } @@ -1773,9 +1777,9 @@ public class BubbleController implements ConfigurationChangeListener { } @Override - public void onEntryUpdated(BubbleEntry entry, boolean shouldBubbleUp) { + public void onEntryUpdated(BubbleEntry entry, boolean shouldBubbleUp, boolean fromSystem) { mMainExecutor.execute(() -> { - BubbleController.this.onEntryUpdated(entry, shouldBubbleUp); + BubbleController.this.onEntryUpdated(entry, shouldBubbleUp, fromSystem); }); } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/Bubbles.java b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/Bubbles.java index cf792cda91b5c..37b96ffe5cd1e 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/Bubbles.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/Bubbles.java @@ -171,8 +171,9 @@ public interface Bubbles { * * @param entry the {@link BubbleEntry} by the notification. * @param shouldBubbleUp {@code true} if this notification should bubble up. + * @param fromSystem {@code true} if this update is from NotificationManagerService. */ - void onEntryUpdated(BubbleEntry entry, boolean shouldBubbleUp); + void onEntryUpdated(BubbleEntry entry, boolean shouldBubbleUp, boolean fromSystem); /** * Called when new notification entry removed. diff --git a/packages/SystemUI/src/com/android/systemui/wmshell/BubblesManager.java b/packages/SystemUI/src/com/android/systemui/wmshell/BubblesManager.java index e22a896227ef6..4c762702892a0 100644 --- a/packages/SystemUI/src/com/android/systemui/wmshell/BubblesManager.java +++ b/packages/SystemUI/src/com/android/systemui/wmshell/BubblesManager.java @@ -405,8 +405,8 @@ public class BubblesManager implements Dumpable { } @Override - public void onEntryUpdated(NotificationEntry entry) { - BubblesManager.this.onEntryUpdated(entry); + public void onEntryUpdated(NotificationEntry entry, boolean fromSystem) { + BubblesManager.this.onEntryUpdated(entry, fromSystem); } @Override @@ -444,9 +444,10 @@ public class BubblesManager implements Dumpable { } } - void onEntryUpdated(NotificationEntry entry) { + void onEntryUpdated(NotificationEntry entry, boolean fromSystem) { + boolean shouldBubble = mNotificationInterruptStateProvider.shouldBubbleUp(entry); mBubbles.onEntryUpdated(notifToBubbleEntry(entry), - mNotificationInterruptStateProvider.shouldBubbleUp(entry)); + shouldBubble, fromSystem); } void onEntryRemoved(NotificationEntry entry) { 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 fee17c785ed28..25f83fcd4bcca 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java @@ -33,6 +33,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; @@ -621,7 +622,7 @@ public class BubblesTest extends SysuiTestCase { assertFalse(mBubbleData.getBubbleInStackWithKey(mRow.getKey()).showDot()); // Send update - mEntryListener.onEntryUpdated(mRow); + mEntryListener.onEntryUpdated(mRow, /* fromSystem= */ true); // Nothing should have changed // Notif is suppressed after expansion @@ -789,7 +790,7 @@ public class BubblesTest extends SysuiTestCase { @Test public void testAddNotif_notBubble() { mEntryListener.onEntryAdded(mNonBubbleNotifRow.getEntry()); - mEntryListener.onEntryUpdated(mNonBubbleNotifRow.getEntry()); + mEntryListener.onEntryUpdated(mNonBubbleNotifRow.getEntry(), /* fromSystem= */ true); assertThat(mBubbleController.hasBubbles()).isFalse(); } @@ -827,7 +828,7 @@ public class BubblesTest extends SysuiTestCase { NotificationListenerService.Ranking ranking = new RankingBuilder( mRow.getRanking()).setCanBubble(false).build(); mRow.setRanking(ranking); - mEntryListener.onEntryUpdated(mRow); + mEntryListener.onEntryUpdated(mRow, /* fromSystem= */ true); assertFalse(mBubbleController.hasBubbles()); verify(mDeleteIntent, never()).send(); @@ -1432,6 +1433,20 @@ public class BubblesTest extends SysuiTestCase { assertThat(mBubbleData.hasBubbleInStackWithKey(mBubbleEntry.getKey())).isFalse(); } + @Test + public void testNonSystemUpdatesIgnored() { + mEntryListener.onEntryAdded(mRow); + assertThat(mBubbleController.hasBubbles()).isTrue(); + + mEntryListener.onEntryUpdated(mRow, /* fromSystem= */ false); + mEntryListener.onEntryUpdated(mRow, /* fromSystem= */ false); + mEntryListener.onEntryUpdated(mRow, /* fromSystem= */ false); + + // 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()); + } + /** Creates a bubble using the userId and package. */ private Bubble createBubble(int userId, String pkg) { final UserHandle userHandle = new UserHandle(userId);