Always open bubble for bubble notifs + fix issue
Clicking on the notification will only redirect to the
bubble if the notification has FLAG_BUBBLE. This changes
the logic so that it will happen if the notification is
allowed to bubble.
This also includes a fix for an issue where we would
remove the bubble (and notification) incorrectly on updates:
Previously we'd always remove a bubble if the notif was
updated & no longer had FLAG_BUBBLE. There are some
situations where a notif might not have the flag but also
shouldn't be removed (e.g. bubble dismissed). This CL alters
it so that the bubble is removed when canBubble is false
instead of FLAG_BUBBLE being removed.
Test: atest SystemUITests
Bug: 149033291
Bug: 152883583
Change-Id: If9c61295d09dfa9f36360b02bcd66967bcb7d667
diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/Bubble.java b/packages/SystemUI/src/com/android/systemui/bubbles/Bubble.java
index 24ef324..ecd8b45 100644
--- a/packages/SystemUI/src/com/android/systemui/bubbles/Bubble.java
+++ b/packages/SystemUI/src/com/android/systemui/bubbles/Bubble.java
@@ -68,6 +68,8 @@
/** Whether flyout text should be suppressed, regardless of any other flags or state. */
private boolean mSuppressFlyout;
+ /** Whether this bubble should auto expand regardless of the normal flag, used for overflow. */
+ private boolean mShouldAutoExpand;
// Items that are typically loaded later
private String mAppName;
@@ -470,7 +472,11 @@
boolean shouldAutoExpand() {
Notification.BubbleMetadata metadata = mEntry.getBubbleMetadata();
- return metadata != null && metadata.getAutoExpandBubble();
+ return (metadata != null && metadata.getAutoExpandBubble()) || mShouldAutoExpand;
+ }
+
+ void setShouldAutoExpand(boolean shouldAutoExpand) {
+ mShouldAutoExpand = shouldAutoExpand;
}
@Override
diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
index eb4ba6f..f0f28fd 100644
--- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
+++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
@@ -729,8 +729,9 @@
mNotificationEntryManager.getActiveNotificationsForCurrentUser()) {
if (savedBubbleKeys.contains(e.getKey())
&& mNotificationInterruptStateProvider.shouldBubbleUp(e)
+ && e.isBubble()
&& canLaunchInActivityView(mContext, e)) {
- updateBubble(e, /* suppressFlyout= */ true);
+ updateBubble(e, true /* suppressFlyout */, false /* showInShade */);
}
}
// Finally, remove the entries for this user now that bubbles are restored.
@@ -844,25 +845,34 @@
void promoteBubbleFromOverflow(Bubble bubble) {
bubble.setInflateSynchronously(mInflateSynchronously);
- setIsBubble(bubble, /* isBubble */ true);
+ setIsBubble(bubble.getEntry(), /* isBubble */ true);
mBubbleData.promoteBubbleFromOverflow(bubble, mStackView, mBubbleIconFactory);
}
/**
* Request the stack expand if needed, then select the specified Bubble as current.
+ * If no bubble exists for this entry, one is created.
*
- * @param notificationKey the notification key for the bubble to be selected
+ * @param entry the notification for the bubble to be selected
*/
- public void expandStackAndSelectBubble(String notificationKey) {
- Bubble bubble = mBubbleData.getBubbleInStackWithKey(notificationKey);
- if (bubble == null) {
- bubble = mBubbleData.getOverflowBubbleWithKey(notificationKey);
- if (bubble != null) {
- mBubbleData.promoteBubbleFromOverflow(bubble, mStackView, mBubbleIconFactory);
- }
- } else if (bubble.getEntry().isBubble()){
+ public void expandStackAndSelectBubble(NotificationEntry entry) {
+ String key = entry.getKey();
+ Bubble bubble = mBubbleData.getBubbleInStackWithKey(key);
+ if (bubble != null) {
mBubbleData.setSelectedBubble(bubble);
+ } else {
+ bubble = mBubbleData.getOverflowBubbleWithKey(key);
+ if (bubble != null) {
+ promoteBubbleFromOverflow(bubble);
+ } else if (entry.canBubble()) {
+ // It can bubble but it's not -- it got aged out of the overflow before it
+ // was dismissed or opened, make it a bubble again.
+ setIsBubble(entry, true);
+ bubble.setShouldAutoExpand(true);
+ updateBubble(entry, true /* suppressFlyout */, false /* showInShade */);
+ }
}
+
mBubbleData.setExpanded(true);
}
@@ -882,11 +892,7 @@
* @param notif the notification associated with this bubble.
*/
void updateBubble(NotificationEntry notif) {
- updateBubble(notif, false /* suppressFlyout */);
- }
-
- void updateBubble(NotificationEntry notif, boolean suppressFlyout) {
- updateBubble(notif, suppressFlyout, true /* showInShade */);
+ updateBubble(notif, false /* suppressFlyout */, true /* showInShade */);
}
void updateBubble(NotificationEntry notif, boolean suppressFlyout, boolean showInShade) {
@@ -901,7 +907,8 @@
bubble.setInflateSynchronously(mInflateSynchronously);
bubble.inflate(
b -> {
- mBubbleData.notificationEntryUpdated(b, suppressFlyout, showInShade);
+ mBubbleData.notificationEntryUpdated(b, suppressFlyout,
+ showInShade);
if (bubble.getBubbleIntent() == null) {
return;
}
@@ -979,18 +986,20 @@
private void onEntryAdded(NotificationEntry entry) {
if (mNotificationInterruptStateProvider.shouldBubbleUp(entry)
+ && entry.isBubble()
&& canLaunchInActivityView(mContext, entry)) {
updateBubble(entry);
}
}
private void onEntryUpdated(NotificationEntry entry) {
+ // shouldBubbleUp checks canBubble & for bubble metadata
boolean shouldBubble = mNotificationInterruptStateProvider.shouldBubbleUp(entry)
&& canLaunchInActivityView(mContext, entry);
if (!shouldBubble && mBubbleData.hasAnyBubbleWithKey(entry.getKey())) {
// It was previously a bubble but no longer a bubble -- lets remove it
removeBubble(entry, DISMISS_NO_LONGER_BUBBLE);
- } else if (shouldBubble) {
+ } else if (shouldBubble && entry.isBubble()) {
updateBubble(entry);
}
}
@@ -1036,14 +1045,14 @@
}
}
- private void setIsBubble(Bubble b, boolean isBubble) {
+ private void setIsBubble(NotificationEntry entry, boolean isBubble) {
if (isBubble) {
- b.getEntry().getSbn().getNotification().flags |= FLAG_BUBBLE;
+ entry.getSbn().getNotification().flags |= FLAG_BUBBLE;
} else {
- b.getEntry().getSbn().getNotification().flags &= ~FLAG_BUBBLE;
+ entry.getSbn().getNotification().flags &= ~FLAG_BUBBLE;
}
try {
- mBarService.onNotificationBubbleChanged(b.getKey(), isBubble, 0);
+ mBarService.onNotificationBubbleChanged(entry.getKey(), isBubble, 0);
} catch (RemoteException e) {
// Bad things have happened
}
@@ -1092,7 +1101,7 @@
}
} else {
if (bubble.getEntry().isBubble() && bubble.showInShade()) {
- setIsBubble(bubble, /* isBubble */ false);
+ setIsBubble(bubble.getEntry(), false /* isBubble */);
}
if (bubble.getEntry().getRow() != null) {
bubble.getEntry().getRow().updateBubbleButton();
@@ -1327,7 +1336,8 @@
boolean clearedTask, boolean wasVisible) {
for (Bubble b : mBubbleData.getBubbles()) {
if (b.getDisplayId() == task.displayId) {
- expandStackAndSelectBubble(b.getKey());
+ mBubbleData.setSelectedBubble(b);
+ mBubbleData.setExpanded(true);
return;
}
}
diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java
index 43f65de..35647b0 100644
--- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java
+++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java
@@ -24,7 +24,6 @@
import android.app.Notification;
import android.app.PendingIntent;
import android.content.Context;
-import android.service.notification.NotificationListenerService;
import android.util.Log;
import android.util.Pair;
import android.view.View;
@@ -123,8 +122,6 @@
// State tracked during an operation -- keeps track of what listener events to dispatch.
private Update mStateChange;
- private NotificationListenerService.Ranking mTmpRanking;
-
private TimeSource mTimeSource = System::currentTimeMillis;
@Nullable
@@ -210,15 +207,14 @@
}
moveOverflowBubbleToPending(bubble);
// Preserve new order for next repack, which sorts by last updated time.
- bubble.markUpdatedAt(mTimeSource.currentTimeMillis());
bubble.inflate(
b -> {
- notificationEntryUpdated(bubble, /* suppressFlyout */
- false, /* showInShade */ true);
- setSelectedBubble(bubble);
+ b.setShouldAutoExpand(true);
+ b.markUpdatedAt(mTimeSource.currentTimeMillis());
+ notificationEntryUpdated(bubble, false /* suppressFlyout */,
+ true /* showInShade */);
},
mContext, stack, factory);
- dispatchPendingChanges();
}
void setShowingOverflow(boolean showingOverflow) {
@@ -284,7 +280,9 @@
bubble.setSuppressFlyout(suppressFlyout);
doUpdate(bubble);
}
+
if (bubble.shouldAutoExpand()) {
+ bubble.setShouldAutoExpand(false);
setSelectedBubbleInternal(bubble);
if (!mExpanded) {
setExpandedInternal(true);
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImpl.java
index da31fe0..71f6dac 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/NotificationInterruptStateProviderImpl.java
@@ -146,14 +146,6 @@
return false;
}
- if (!entry.isBubble()) {
- if (DEBUG) {
- Log.d(TAG, "No bubble up: notification " + sbn.getKey()
- + " is bubble? " + entry.isBubble());
- }
- return false;
- }
-
if (entry.getBubbleMetadata() == null
|| (entry.getBubbleMetadata().getShortcutId() == null
&& entry.getBubbleMetadata().getIntent() == null)) {
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java
index 64e5f0a..7bcfb46 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java
@@ -350,7 +350,6 @@
}
Intent fillInIntent = null;
NotificationEntry entry = row.getEntry();
- final boolean isBubble = entry.isBubble();
CharSequence remoteInputText = null;
if (!TextUtils.isEmpty(entry.remoteInputText)) {
remoteInputText = entry.remoteInputText;
@@ -359,14 +358,15 @@
fillInIntent = new Intent().putExtra(Notification.EXTRA_REMOTE_INPUT_DRAFT,
remoteInputText.toString());
}
- if (isBubble) {
+ final boolean canBubble = entry.canBubble();
+ if (canBubble) {
mLogger.logExpandingBubble(notificationKey);
- expandBubbleStackOnMainThread(notificationKey);
+ expandBubbleStackOnMainThread(entry);
} else {
startNotificationIntent(
intent, fillInIntent, entry, row, wasOccluded, isActivityIntent);
}
- if (isActivityIntent || isBubble) {
+ if (isActivityIntent || canBubble) {
mAssistManagerLazy.get().hideAssist();
}
if (shouldCollapse()) {
@@ -381,7 +381,7 @@
rank, count, true, location);
mClickNotifier.onNotificationClick(notificationKey, nv);
- if (!isBubble) {
+ if (!canBubble) {
if (parentToCancelFinal != null) {
// TODO: (b/145659174) remove - this cancels the parent if the notification clicked
// on will auto-cancel and is the only child in the group. This won't be
@@ -398,12 +398,12 @@
mIsCollapsingToShowActivityOverLockscreen = false;
}
- private void expandBubbleStackOnMainThread(String notificationKey) {
+ private void expandBubbleStackOnMainThread(NotificationEntry entry) {
if (Looper.getMainLooper().isCurrentThread()) {
- mBubbleController.expandStackAndSelectBubble(notificationKey);
+ mBubbleController.expandStackAndSelectBubble(entry);
} else {
mMainThreadHandler.post(
- () -> mBubbleController.expandStackAndSelectBubble(notificationKey));
+ () -> mBubbleController.expandStackAndSelectBubble(entry));
}
}
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 96e868d..9b377ca 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/bubbles/BubbleControllerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/bubbles/BubbleControllerTest.java
@@ -50,6 +50,7 @@
import android.os.Handler;
import android.os.PowerManager;
import android.service.dreams.IDreamManager;
+import android.service.notification.NotificationListenerService;
import android.service.notification.ZenModeConfig;
import android.testing.AndroidTestingRunner;
import android.testing.TestableLooper;
@@ -68,6 +69,7 @@
import com.android.systemui.statusbar.FeatureFlags;
import com.android.systemui.statusbar.NotificationLockscreenUserManager;
import com.android.systemui.statusbar.NotificationRemoveInterceptor;
+import com.android.systemui.statusbar.RankingBuilder;
import com.android.systemui.statusbar.SuperStatusBarViewFactory;
import com.android.systemui.statusbar.SysuiStatusBarStateController;
import com.android.systemui.statusbar.notification.NotificationEntryListener;
@@ -674,7 +676,7 @@
mRemoveInterceptor.onNotificationRemoveRequested(
mRow.getEntry().getKey(), mRow.getEntry(), REASON_APP_CANCEL);
- mBubbleController.expandStackAndSelectBubble(key);
+ mBubbleController.expandStackAndSelectBubble(mRow.getEntry());
assertTrue(mSysUiStateBubblesExpanded);
}
@@ -727,6 +729,9 @@
assertTrue(mBubbleController.hasBubbles());
mRow.getEntry().getSbn().getNotification().flags &= ~FLAG_BUBBLE;
+ NotificationListenerService.Ranking ranking = new RankingBuilder(
+ mRow.getEntry().getRanking()).setCanBubble(false).build();
+ mRow.getEntry().setRanking(ranking);
mEntryListener.onPreEntryUpdated(mRow.getEntry());
assertFalse(mBubbleController.hasBubbles());
diff --git a/packages/SystemUI/tests/src/com/android/systemui/bubbles/NewNotifPipelineBubbleControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/bubbles/NewNotifPipelineBubbleControllerTest.java
index 73b8760..b18d67b 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/bubbles/NewNotifPipelineBubbleControllerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/bubbles/NewNotifPipelineBubbleControllerTest.java
@@ -46,6 +46,7 @@
import android.os.Handler;
import android.os.PowerManager;
import android.service.dreams.IDreamManager;
+import android.service.notification.NotificationListenerService;
import android.service.notification.ZenModeConfig;
import android.testing.AndroidTestingRunner;
import android.testing.TestableLooper;
@@ -62,6 +63,7 @@
import com.android.systemui.plugins.statusbar.StatusBarStateController;
import com.android.systemui.statusbar.FeatureFlags;
import com.android.systemui.statusbar.NotificationLockscreenUserManager;
+import com.android.systemui.statusbar.RankingBuilder;
import com.android.systemui.statusbar.SuperStatusBarViewFactory;
import com.android.systemui.statusbar.SysuiStatusBarStateController;
import com.android.systemui.statusbar.notification.NotificationEntryManager;
@@ -640,6 +642,9 @@
assertTrue(mBubbleController.hasBubbles());
mRow.getEntry().getSbn().getNotification().flags &= ~FLAG_BUBBLE;
+ NotificationListenerService.Ranking ranking = new RankingBuilder(
+ mRow.getEntry().getRanking()).setCanBubble(false).build();
+ mRow.getEntry().setRanking(ranking);
mEntryListener.onEntryUpdated(mRow.getEntry());
assertFalse(mBubbleController.hasBubbles());
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java
index 53a1773..acdb2c5 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java
@@ -282,7 +282,7 @@
mNotificationActivityStarter.onNotificationClicked(sbn, mBubbleNotificationRow);
// Then
- verify(mBubbleController).expandStackAndSelectBubble(eq(sbn.getKey()));
+ verify(mBubbleController).expandStackAndSelectBubble(eq(mBubbleNotificationRow.getEntry()));
// This is called regardless, and simply short circuits when there is nothing to do.
verify(mShadeController, atLeastOnce()).collapsePanel();
@@ -313,7 +313,7 @@
mNotificationActivityStarter.onNotificationClicked(sbn, mBubbleNotificationRow);
// Then
- verify(mBubbleController).expandStackAndSelectBubble(eq(sbn.getKey()));
+ verify(mBubbleController).expandStackAndSelectBubble(mBubbleNotificationRow.getEntry());
verify(mShadeController, atLeastOnce()).collapsePanel();
@@ -343,7 +343,7 @@
mNotificationActivityStarter.onNotificationClicked(sbn, mBubbleNotificationRow);
// Then
- verify(mBubbleController).expandStackAndSelectBubble(eq(sbn.getKey()));
+ verify(mBubbleController).expandStackAndSelectBubble(mBubbleNotificationRow.getEntry());
verify(mShadeController, atLeastOnce()).collapsePanel();