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
This commit is contained in:
Mady Mellor
2021-07-13 13:32:07 -07:00
parent 6154a87850
commit c88902ca0b
3 changed files with 97 additions and 3 deletions

View File

@@ -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;

View File

@@ -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());
}

View File

@@ -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