From c88902ca0b0cd27f45270848c004cb290ec7c80f Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Tue, 13 Jul 2021 13:32:07 -0700 Subject: [PATCH] Only load the overflow data once We would also try loading the overflow data if it was empty but this works poorly if it's becoming empty due to an app cancel because we could load contents that are about to be removed. I think this can safely work if we just load it based on the flag. Adds a test related to this that was only in BubblesTest to NewNotifPipelineBubblesTest as well Test: atest BubblesTest NewNotifPipelineBubblesTest Test: - have nothing in bubble overflow - post 3 bubbles, cancel all, get a bubble, look in the overflow => the 3 bubbles shouldn't be there Bug: 193560795 Change-Id: I1ec2d637281807ae3a2ec7430cff6627b074fa78 --- .../wm/shell/bubbles/BubbleController.java | 3 +- .../android/systemui/wmshell/BubblesTest.java | 25 ++++++- .../wmshell/NewNotifPipelineBubblesTest.java | 72 +++++++++++++++++++ 3 files changed, 97 insertions(+), 3 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 f3f66dc894ce8..28e2f7dd0a639 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 @@ -906,8 +906,7 @@ public class BubbleController { * Fills the overflow bubbles by loading them from disk. */ void loadOverflowBubblesFromDisk() { - if (!mBubbleData.getOverflowBubbles().isEmpty() && !mOverflowDataLoadNeeded) { - // we don't need to load overflow bubbles from disk if it is already in memory + if (!mOverflowDataLoadNeeded) { return; } mOverflowDataLoadNeeded = false; 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 d6492c610a4fa..a0b93ad25b8b3 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java @@ -24,6 +24,7 @@ import static android.service.notification.NotificationListenerService.REASON_CA import static android.service.notification.NotificationListenerService.REASON_GROUP_SUMMARY_CANCELED; import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn; +import static com.android.wm.shell.bubbles.Bubbles.DISMISS_NOTIF_CANCEL; import static com.google.common.truth.Truth.assertThat; @@ -441,7 +442,7 @@ public class BubblesTest extends SysuiTestCase { mRow.getKey(), Bubbles.DISMISS_USER_GESTURE); mBubbleController.removeBubble( - mRow.getKey(), Bubbles.DISMISS_NOTIF_CANCEL); + mRow.getKey(), DISMISS_NOTIF_CANCEL); verify(mNotificationEntryManager, times(1)).performRemoveNotification( eq(mRow.getSbn()), any(), anyInt()); assertThat(mBubbleData.getOverflowBubbles()).isEmpty(); @@ -1146,6 +1147,28 @@ public class BubblesTest extends SysuiTestCase { // Verify these are in the overflow assertThat(mBubbleData.getOverflowBubbleWithKey(mBubbleEntryUser11.getKey())).isNotNull(); assertThat(mBubbleData.getOverflowBubbleWithKey(mBubbleEntry2User11.getKey())).isNotNull(); + + // Would have loaded bubbles twice because of user switch + verify(mDataRepository, times(2)).loadBubbles(anyInt(), any()); + } + + /** + * Verifies we only load the overflow data once. + */ + @Test + public void testOverflowLoadedOnce() { + mBubbleController.updateBubble(mBubbleEntry); + mBubbleController.updateBubble(mBubbleEntry2); + mBubbleData.dismissAll(Bubbles.DISMISS_USER_GESTURE); + assertThat(mBubbleData.getOverflowBubbles().isEmpty()).isFalse(); + + mBubbleController.updateBubble(mBubbleEntry); + mBubbleController.updateBubble(mBubbleEntry2); + mBubbleController.removeBubble(mBubbleEntry.getKey(), DISMISS_NOTIF_CANCEL); + mBubbleController.removeBubble(mBubbleEntry2.getKey(), DISMISS_NOTIF_CANCEL); + assertThat(mBubbleData.getOverflowBubbles()).isEmpty(); + + verify(mDataRepository, times(1)).loadBubbles(anyInt(), any()); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/wmshell/NewNotifPipelineBubblesTest.java b/packages/SystemUI/tests/src/com/android/systemui/wmshell/NewNotifPipelineBubblesTest.java index db8a08cdc5e4a..b0dd73ada7cf0 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/wmshell/NewNotifPipelineBubblesTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/wmshell/NewNotifPipelineBubblesTest.java @@ -17,6 +17,7 @@ package com.android.systemui.wmshell; import static android.app.Notification.FLAG_BUBBLE; +import static android.service.notification.NotificationListenerService.REASON_APP_CANCEL; import static android.service.notification.NotificationListenerService.REASON_GROUP_SUMMARY_CANCELED; import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn; @@ -47,6 +48,7 @@ import android.content.pm.LauncherApps; import android.hardware.display.AmbientDisplayConfiguration; import android.os.Handler; import android.os.PowerManager; +import android.os.UserHandle; import android.service.dreams.IDreamManager; import android.service.notification.NotificationListenerService; import android.service.notification.ZenModeConfig; @@ -172,6 +174,10 @@ public class NewNotifPipelineBubblesTest extends SysuiTestCase { private ExpandableNotificationRow mNonBubbleNotifRow; private BubbleEntry mBubbleEntry; private BubbleEntry mBubbleEntry2; + + private BubbleEntry mBubbleEntryUser11; + private BubbleEntry mBubbleEntry2User11; + @Mock private Bubbles.BubbleExpandListener mBubbleExpandListener; @Mock @@ -241,6 +247,13 @@ public class NewNotifPipelineBubblesTest extends SysuiTestCase { mBubbleEntry = BubblesManager.notifToBubbleEntry(mRow); mBubbleEntry2 = BubblesManager.notifToBubbleEntry(mRow2); + UserHandle handle = mock(UserHandle.class); + when(handle.getIdentifier()).thenReturn(11); + mBubbleEntryUser11 = BubblesManager.notifToBubbleEntry( + mNotificationTestHelper.createBubble(handle)); + mBubbleEntry2User11 = BubblesManager.notifToBubbleEntry( + mNotificationTestHelper.createBubble(handle)); + mZenModeConfig.suppressedVisualEffects = 0; when(mZenModeController.getConfig()).thenReturn(mZenModeConfig); @@ -908,6 +921,65 @@ public class NewNotifPipelineBubblesTest extends SysuiTestCase { groupSummary.getEntry().getSbn().getGroupKey())); } + + /** + * Verifies that when the user changes, the bubbles in the overflow list is cleared. Doesn't + * test the loading from the repository which would be a nice thing to add. + */ + @Test + public void testOnUserChanged_overflowState() { + int firstUserId = mBubbleEntry.getStatusBarNotification().getUser().getIdentifier(); + int secondUserId = mBubbleEntryUser11.getStatusBarNotification().getUser().getIdentifier(); + + mBubbleController.updateBubble(mBubbleEntry); + mBubbleController.updateBubble(mBubbleEntry2); + assertTrue(mBubbleController.hasBubbles()); + mBubbleData.dismissAll(Bubbles.DISMISS_USER_GESTURE); + + // Verify these are in the overflow + assertThat(mBubbleData.getOverflowBubbleWithKey(mBubbleEntry.getKey())).isNotNull(); + assertThat(mBubbleData.getOverflowBubbleWithKey(mBubbleEntry2.getKey())).isNotNull(); + + // Switch users + mBubbleController.onUserChanged(secondUserId); + assertThat(mBubbleData.getOverflowBubbles()).isEmpty(); + + // Give this user some bubbles + mBubbleController.updateBubble(mBubbleEntryUser11); + mBubbleController.updateBubble(mBubbleEntry2User11); + assertTrue(mBubbleController.hasBubbles()); + mBubbleData.dismissAll(Bubbles.DISMISS_USER_GESTURE); + + // Verify these are in the overflow + assertThat(mBubbleData.getOverflowBubbleWithKey(mBubbleEntryUser11.getKey())).isNotNull(); + assertThat(mBubbleData.getOverflowBubbleWithKey(mBubbleEntry2User11.getKey())).isNotNull(); + + // Would have loaded bubbles twice because of user switch + verify(mDataRepository, times(2)).loadBubbles(anyInt(), any()); + } + + /** + * Verifies we only load the overflow data once. + */ + @Test + public void testOverflowLoadedOnce() { + when(mNotificationEntryManager.getPendingOrActiveNotif(mRow.getKey())) + .thenReturn(mRow); + when(mNotificationEntryManager.getPendingOrActiveNotif(mRow2.getKey())) + .thenReturn(mRow2); + + mEntryListener.onEntryAdded(mRow); + mEntryListener.onEntryAdded(mRow2); + mBubbleData.dismissAll(Bubbles.DISMISS_USER_GESTURE); + assertThat(mBubbleData.getOverflowBubbles()).isNotEmpty(); + + mEntryListener.onEntryRemoved(mRow, REASON_APP_CANCEL); + mEntryListener.onEntryRemoved(mRow2, REASON_APP_CANCEL); + assertThat(mBubbleData.getOverflowBubbles()).isEmpty(); + + verify(mDataRepository, times(1)).loadBubbles(anyInt(), any()); + } + /** * Sets the bubble metadata flags for this entry. These flags are normally set by * NotificationManagerService when the notification is sent, however, these tests do not