Remove the bubble if the notification is no longer FLAG_BUBBLE
Previously if a notification that was a bubble got updated such that it
would fail our bubble criteria, we wouldn't actually remove the bubble we
would just stop updating that bubble. This CL fixes it so that we'll remove
the bubble in that case.
This CL also factors all of the 'shouldBubble' logic into Notification
InterruptionStateProvider.
Fixes: 128459529
Test: CTSVerifier tests in other CL; atest BubbleControllerTest
Change-Id: Ia2b53a25a28d53bd2bbaebe4fe89f50d8d46a9fb
diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
index acdcfb2..9168bf3 100644
--- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
+++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
@@ -83,7 +83,7 @@
@Retention(SOURCE)
@IntDef({DISMISS_USER_GESTURE, DISMISS_AGED, DISMISS_TASK_FINISHED, DISMISS_BLOCKED,
- DISMISS_NOTIF_CANCEL, DISMISS_ACCESSIBILITY_ACTION})
+ DISMISS_NOTIF_CANCEL, DISMISS_ACCESSIBILITY_ACTION, DISMISS_NO_LONGER_BUBBLE})
@interface DismissReason {}
static final int DISMISS_USER_GESTURE = 1;
@@ -92,6 +92,7 @@
static final int DISMISS_BLOCKED = 4;
static final int DISMISS_NOTIF_CANCEL = 5;
static final int DISMISS_ACCESSIBILITY_ACTION = 6;
+ static final int DISMISS_NO_LONGER_BUBBLE = 7;
static final int MAX_BUBBLES = 5; // TODO: actually enforce this
@@ -126,8 +127,7 @@
private final StatusBarWindowController mStatusBarWindowController;
private StatusBarStateListener mStatusBarStateListener;
- private final NotificationInterruptionStateProvider mNotificationInterruptionStateProvider =
- Dependency.get(NotificationInterruptionStateProvider.class);
+ private final NotificationInterruptionStateProvider mNotificationInterruptionStateProvider;
private INotificationManager mNotificationManagerService;
@@ -183,15 +183,19 @@
@Inject
public BubbleController(Context context, StatusBarWindowController statusBarWindowController,
- BubbleData data, ConfigurationController configurationController) {
+ BubbleData data, ConfigurationController configurationController,
+ NotificationInterruptionStateProvider interruptionStateProvider) {
this(context, statusBarWindowController, data, null /* synchronizer */,
- configurationController);
+ configurationController, interruptionStateProvider);
}
public BubbleController(Context context, StatusBarWindowController statusBarWindowController,
BubbleData data, @Nullable BubbleStackView.SurfaceSynchronizer synchronizer,
- ConfigurationController configurationController) {
+ ConfigurationController configurationController,
+ NotificationInterruptionStateProvider interruptionStateProvider) {
mContext = context;
+ mNotificationInterruptionStateProvider = interruptionStateProvider;
+
configurationController.addCallback(this /* configurationListener */);
mNotificationEntryManager = Dependency.get(NotificationEntryManager.class);
@@ -380,7 +384,7 @@
if (!areBubblesEnabled(mContext)) {
return;
}
- if (shouldAutoBubbleForFlags(mContext, entry) || shouldBubble(entry)) {
+ if (mNotificationInterruptionStateProvider.shouldBubbleUp(entry)) {
// TODO: handle group summaries?
updateShowInShadeForSuppressNotification(entry);
}
@@ -391,7 +395,7 @@
if (!areBubblesEnabled(mContext)) {
return;
}
- if (entry.isBubble() && mNotificationInterruptionStateProvider.shouldBubbleUp(entry)) {
+ if (mNotificationInterruptionStateProvider.shouldBubbleUp(entry)) {
updateBubble(entry);
}
}
@@ -401,8 +405,11 @@
if (!areBubblesEnabled(mContext)) {
return;
}
- if (mNotificationInterruptionStateProvider.shouldBubbleUp(entry)
- && alertAgain(entry, entry.notification.getNotification())) {
+ boolean shouldBubble = mNotificationInterruptionStateProvider.shouldBubbleUp(entry);
+ if (!shouldBubble && mBubbleData.hasBubbleWithKey(entry.key)) {
+ // It was previously a bubble but no longer a bubble -- lets remove it
+ removeBubble(entry.key, DISMISS_NO_LONGER_BUBBLE);
+ } else if (shouldBubble && alertAgain(entry, entry.notification.getNotification())) {
updateShowInShadeForSuppressNotification(entry);
entry.setBubbleDismissed(false); // updates come back as bubbles even if dismissed
updateBubble(entry);
@@ -529,17 +536,6 @@
}
/**
- * Whether the notification has been developer configured to bubble and is allowed by the user.
- */
- @VisibleForTesting
- protected boolean shouldBubble(NotificationEntry entry) {
- StatusBarNotification n = entry.notification;
- boolean hasOverlayIntent = n.getNotification().getBubbleMetadata() != null
- && n.getNotification().getBubbleMetadata().getIntent() != null;
- return hasOverlayIntent && entry.canBubble;
- }
-
- /**
* Whether the notification should automatically bubble or not. Gated by secure settings flags.
*/
@VisibleForTesting
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationInterruptionStateProvider.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationInterruptionStateProvider.java
index 059c3f6..926d4b6 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationInterruptionStateProvider.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationInterruptionStateProvider.java
@@ -147,7 +147,14 @@
* @return true if the entry should bubble up, false otherwise
*/
public boolean shouldBubbleUp(NotificationEntry entry) {
- StatusBarNotification sbn = entry.notification;
+ final StatusBarNotification sbn = entry.notification;
+ if (!entry.canBubble) {
+ if (DEBUG) {
+ Log.d(TAG, "No bubble up: not allowed to bubble: " + sbn.getKey());
+ }
+ return false;
+ }
+
if (!entry.isBubble()) {
if (DEBUG) {
Log.d(TAG, "No bubble up: notification " + sbn.getKey()
@@ -156,6 +163,15 @@
return false;
}
+ final Notification n = sbn.getNotification();
+ if (n.getBubbleMetadata() == null || n.getBubbleMetadata().getIntent() == null) {
+ if (DEBUG) {
+ Log.d(TAG, "No bubble up: notification: " + sbn.getKey()
+ + " doesn't have valid metadata");
+ }
+ return false;
+ }
+
if (!canHeadsUpCommon(entry)) {
return false;
}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/bubbles/BubbleControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/bubbles/BubbleControllerTest.java
index 40a5357..8b0ba94 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/bubbles/BubbleControllerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/bubbles/BubbleControllerTest.java
@@ -16,6 +16,7 @@
package com.android.systemui.bubbles;
+import static android.app.Notification.FLAG_BUBBLE;
import static android.content.Intent.FLAG_ACTIVITY_NEW_TASK;
import static com.google.common.truth.Truth.assertThat;
@@ -25,6 +26,7 @@
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.Mockito.atLeastOnce;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -47,15 +49,18 @@
import com.android.systemui.R;
import com.android.systemui.SysuiTestCase;
+import com.android.systemui.statusbar.NotificationPresenter;
import com.android.systemui.statusbar.NotificationTestHelper;
import com.android.systemui.statusbar.notification.NotificationEntryListener;
import com.android.systemui.statusbar.notification.NotificationEntryManager;
+import com.android.systemui.statusbar.notification.NotificationInterruptionStateProvider;
import com.android.systemui.statusbar.notification.collection.NotificationData;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;
import com.android.systemui.statusbar.phone.DozeParameters;
import com.android.systemui.statusbar.phone.StatusBarWindowController;
import com.android.systemui.statusbar.policy.ConfigurationController;
+import com.android.systemui.statusbar.policy.HeadsUpManager;
import org.junit.Before;
import org.junit.Test;
@@ -138,7 +143,7 @@
// Some bubbles want to suppress notifs
Notification.BubbleMetadata suppressNotifMetadata =
- getBuilder().setSuppressInitialNotification(true).build();
+ getBuilder().setSuppressNotification(true).build();
mSuppressNotifRow = mNotificationTestHelper.createBubble(suppressNotifMetadata,
FOREGROUND_TEST_PKG_NAME);
@@ -146,9 +151,15 @@
when(mNotificationEntryManager.getNotificationData()).thenReturn(mNotificationData);
when(mNotificationData.getChannel(mRow.getEntry().key)).thenReturn(mRow.getEntry().channel);
+ TestableNotificationInterruptionStateProvider interruptionStateProvider =
+ new TestableNotificationInterruptionStateProvider(mContext);
+ interruptionStateProvider.setUpWithPresenter(
+ mock(NotificationPresenter.class),
+ mock(HeadsUpManager.class),
+ mock(NotificationInterruptionStateProvider.HeadsUpSuppressor.class));
mBubbleData = new BubbleData(mContext);
mBubbleController = new TestableBubbleController(mContext, mStatusBarWindowController,
- mBubbleData, mConfigurationController);
+ mBubbleData, mConfigurationController, interruptionStateProvider);
mBubbleController.setBubbleStateChangeListener(mBubbleStateChangeListener);
mBubbleController.setExpandListener(mBubbleExpandListener);
@@ -487,12 +498,27 @@
verify(mDeleteIntent, times(2)).send();
}
+ @Test
+ public void testRemoveBubble_noLongerBubbleAfterUpdate()
+ throws PendingIntent.CanceledException {
+ mBubbleController.updateBubble(mRow.getEntry());
+ assertTrue(mBubbleController.hasBubbles());
+
+ mRow.getEntry().notification.getNotification().flags &= ~FLAG_BUBBLE;
+ mEntryListener.onPreEntryUpdated(mRow.getEntry());
+
+ assertFalse(mBubbleController.hasBubbles());
+ verify(mDeleteIntent, never()).send();
+ }
+
static class TestableBubbleController extends BubbleController {
// Let's assume surfaces can be synchronized immediately.
TestableBubbleController(Context context,
StatusBarWindowController statusBarWindowController, BubbleData data,
- ConfigurationController configurationController) {
- super(context, statusBarWindowController, data, Runnable::run, configurationController);
+ ConfigurationController configurationController,
+ NotificationInterruptionStateProvider interruptionStateProvider) {
+ super(context, statusBarWindowController, data, Runnable::run,
+ configurationController, interruptionStateProvider);
}
@Override
@@ -501,6 +527,15 @@
}
}
+ public static class TestableNotificationInterruptionStateProvider extends
+ NotificationInterruptionStateProvider {
+
+ public TestableNotificationInterruptionStateProvider(Context context) {
+ super(context);
+ mUseHeadsUp = true;
+ }
+ }
+
/**
* @return basic {@link android.app.Notification.BubbleMetadata.Builder}
*/
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationTestHelper.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationTestHelper.java
index 8cc1571..e4b90c5 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationTestHelper.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationTestHelper.java
@@ -178,7 +178,10 @@
Notification n = createNotification(false /* isGroupSummary */,
null /* groupKey */, bubbleMetadata);
n.flags |= FLAG_BUBBLE;
- return generateRow(n, pkg, UID, USER_HANDLE, 0 /* extraInflationFlags */, IMPORTANCE_HIGH);
+ ExpandableNotificationRow row = generateRow(n, pkg, UID, USER_HANDLE,
+ 0 /* extraInflationFlags */, IMPORTANCE_HIGH);
+ row.getEntry().canBubble = true;
+ return row;
}
/**