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
This commit is contained in:
@@ -68,6 +68,8 @@ class Bubble implements BubbleViewProvider {
|
||||
|
||||
/** 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 @@ class Bubble implements BubbleViewProvider {
|
||||
|
||||
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
|
||||
|
||||
@@ -729,8 +729,9 @@ public class BubbleController implements ConfigurationController.ConfigurationLi
|
||||
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 @@ public class BubbleController implements ConfigurationController.ConfigurationLi
|
||||
|
||||
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 @@ public class BubbleController implements ConfigurationController.ConfigurationLi
|
||||
* @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 @@ public class BubbleController implements ConfigurationController.ConfigurationLi
|
||||
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 @@ public class BubbleController implements ConfigurationController.ConfigurationLi
|
||||
|
||||
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 @@ public class BubbleController implements ConfigurationController.ConfigurationLi
|
||||
}
|
||||
}
|
||||
|
||||
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 @@ public class BubbleController implements ConfigurationController.ConfigurationLi
|
||||
}
|
||||
} 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 @@ public class BubbleController implements ConfigurationController.ConfigurationLi
|
||||
boolean clearedTask, boolean wasVisible) {
|
||||
for (Bubble b : mBubbleData.getBubbles()) {
|
||||
if (b.getDisplayId() == task.displayId) {
|
||||
expandStackAndSelectBubble(b.getKey());
|
||||
mBubbleData.setSelectedBubble(b);
|
||||
mBubbleData.setExpanded(true);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,7 +24,6 @@ import static com.android.systemui.bubbles.BubbleDebugConfig.TAG_WITH_CLASS_NAME
|
||||
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 @@ public class BubbleData {
|
||||
// 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 @@ public class BubbleData {
|
||||
}
|
||||
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 @@ public class BubbleData {
|
||||
bubble.setSuppressFlyout(suppressFlyout);
|
||||
doUpdate(bubble);
|
||||
}
|
||||
|
||||
if (bubble.shouldAutoExpand()) {
|
||||
bubble.setShouldAutoExpand(false);
|
||||
setSelectedBubbleInternal(bubble);
|
||||
if (!mExpanded) {
|
||||
setExpandedInternal(true);
|
||||
|
||||
@@ -146,14 +146,6 @@ public class NotificationInterruptStateProviderImpl implements NotificationInter
|
||||
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)) {
|
||||
|
||||
@@ -350,7 +350,6 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit
|
||||
}
|
||||
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 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit
|
||||
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 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit
|
||||
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 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit
|
||||
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));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -50,6 +50,7 @@ import android.hardware.face.FaceManager;
|
||||
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.shared.system.QuickStepContract;
|
||||
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 @@ public class BubbleControllerTest extends SysuiTestCase {
|
||||
mRemoveInterceptor.onNotificationRemoveRequested(
|
||||
mRow.getEntry().getKey(), mRow.getEntry(), REASON_APP_CANCEL);
|
||||
|
||||
mBubbleController.expandStackAndSelectBubble(key);
|
||||
mBubbleController.expandStackAndSelectBubble(mRow.getEntry());
|
||||
|
||||
assertTrue(mSysUiStateBubblesExpanded);
|
||||
}
|
||||
@@ -727,6 +729,9 @@ public class BubbleControllerTest extends SysuiTestCase {
|
||||
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());
|
||||
|
||||
@@ -46,6 +46,7 @@ import android.hardware.face.FaceManager;
|
||||
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.model.SysUiState;
|
||||
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 @@ public class NewNotifPipelineBubbleControllerTest extends SysuiTestCase {
|
||||
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());
|
||||
|
||||
@@ -282,7 +282,7 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
|
||||
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 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
|
||||
mNotificationActivityStarter.onNotificationClicked(sbn, mBubbleNotificationRow);
|
||||
|
||||
// Then
|
||||
verify(mBubbleController).expandStackAndSelectBubble(eq(sbn.getKey()));
|
||||
verify(mBubbleController).expandStackAndSelectBubble(mBubbleNotificationRow.getEntry());
|
||||
|
||||
verify(mShadeController, atLeastOnce()).collapsePanel();
|
||||
|
||||
@@ -343,7 +343,7 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
|
||||
mNotificationActivityStarter.onNotificationClicked(sbn, mBubbleNotificationRow);
|
||||
|
||||
// Then
|
||||
verify(mBubbleController).expandStackAndSelectBubble(eq(sbn.getKey()));
|
||||
verify(mBubbleController).expandStackAndSelectBubble(mBubbleNotificationRow.getEntry());
|
||||
|
||||
verify(mShadeController, atLeastOnce()).collapsePanel();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user