Fix some things with overflow, persistence, & visibility
1) Bubbles that are persisted no longer have an entry in
some cases this wasn't being handled correctly (i.e.
persisted bubbles would never autoexpand)
2) Removed the custom work to promote something from the
overflow -- in most cases we should be able to update
flagBubble & rely on the subsequent notification update
which minimizes the # of code paths
3) mPendingBubbles makes more sense as a HashMap
4) The visibility fix: If the bubble switched from a
shortcut to a pending intent, we would skip making the
view visible
Fixes: 158480978
Fixes: 157755108
Test: atest BubbleControllerTest
Test: manual - create a shortcut bubble
- reboot the device
- promote that bubble from the overflow
=> ensure that it auto-expands
- collapse the stack
- receive an update for that bubble but
now it's a pendingIntent
- open the stack
=> ensure that the view is visible
Change-Id: I37dc780e9a66b9e2f2ae46b5386dcd291dc0e0ab
diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/Bubble.java b/packages/SystemUI/src/com/android/systemui/bubbles/Bubble.java
index 97a7304..7f78ddf 100644
--- a/packages/SystemUI/src/com/android/systemui/bubbles/Bubble.java
+++ b/packages/SystemUI/src/com/android/systemui/bubbles/Bubble.java
@@ -303,10 +303,10 @@
mDotPath = info.dotPath;
if (mExpandedView != null) {
- mExpandedView.update(/* bubble */ this);
+ mExpandedView.update(this /* bubble */);
}
if (mIconView != null) {
- mIconView.setRenderedBubble(/* bubble */ this);
+ mIconView.setRenderedBubble(this /* bubble */);
}
}
@@ -548,13 +548,13 @@
}
private boolean shouldSuppressNotification() {
- if (mEntry == null) return false;
+ if (mEntry == null) return true;
return mEntry.getBubbleMetadata() != null
&& mEntry.getBubbleMetadata().isNotificationSuppressed();
}
boolean shouldAutoExpand() {
- if (mEntry == null) return false;
+ if (mEntry == null) return mShouldAutoExpand;
Notification.BubbleMetadata metadata = mEntry.getBubbleMetadata();
return (metadata != null && metadata.getAutoExpandBubble()) || mShouldAutoExpand;
}
diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
index cf793f0..278f3e5 100644
--- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
+++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
@@ -156,6 +156,7 @@
private final ShadeController mShadeController;
private final FloatingContentCoordinator mFloatingContentCoordinator;
private final BubbleDataRepository mDataRepository;
+ private BubbleLogger mLogger = new BubbleLoggerImpl();
private BubbleData mBubbleData;
private ScrimView mBubbleScrim;
@@ -317,6 +318,7 @@
BubbleDataRepository dataRepository,
SysUiState sysUiState,
INotificationManager notificationManager,
+ @Nullable IStatusBarService statusBarService,
WindowManager windowManager) {
dumpManager.registerDumpable(TAG, this);
mContext = context;
@@ -387,8 +389,10 @@
mSurfaceSynchronizer = synchronizer;
mWindowManager = windowManager;
- mBarService = IStatusBarService.Stub.asInterface(
- ServiceManager.getService(Context.STATUS_BAR_SERVICE));
+ mBarService = statusBarService == null
+ ? IStatusBarService.Stub.asInterface(
+ ServiceManager.getService(Context.STATUS_BAR_SERVICE))
+ : statusBarService;
mBubbleScrim = new ScrimView(mContext);
@@ -894,9 +898,11 @@
}
void promoteBubbleFromOverflow(Bubble bubble) {
+ mLogger.log(bubble, BubbleLogger.Event.BUBBLE_OVERFLOW_REMOVE_BACK_TO_STACK);
bubble.setInflateSynchronously(mInflateSynchronously);
- setIsBubble(bubble, /* isBubble */ true);
- mBubbleData.promoteBubbleFromOverflow(bubble, mStackView, mBubbleIconFactory);
+ bubble.setShouldAutoExpand(true);
+ bubble.markUpdatedAt(System.currentTimeMillis());
+ setIsBubble(bubble, true /* isBubble */);
}
/**
@@ -910,20 +916,17 @@
Bubble bubble = mBubbleData.getBubbleInStackWithKey(key);
if (bubble != null) {
mBubbleData.setSelectedBubble(bubble);
+ mBubbleData.setExpanded(true);
} else {
bubble = mBubbleData.getOverflowBubbleWithKey(key);
if (bubble != null) {
- bubble.setShouldAutoExpand(true);
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);
- updateBubble(entry, true /* suppressFlyout */, false /* showInShade */);
+ setIsBubble(entry, true /* isBubble */, true /* autoExpand */);
}
}
-
- mBubbleData.setExpanded(true);
}
/**
@@ -967,13 +970,17 @@
}
void updateBubble(NotificationEntry notif, boolean suppressFlyout, boolean showInShade) {
- // Lazy init stack view when a bubble is created
- ensureStackViewCreated();
// If this is an interruptive notif, mark that it's interrupted
if (notif.getImportance() >= NotificationManager.IMPORTANCE_HIGH) {
notif.setInterruption();
}
- Bubble bubble = mBubbleData.getOrCreateBubble(notif);
+ Bubble bubble = mBubbleData.getOrCreateBubble(notif, null /* persistedBubble */);
+ inflateAndAdd(bubble, suppressFlyout, showInShade);
+ }
+
+ void inflateAndAdd(Bubble bubble, boolean suppressFlyout, boolean showInShade) {
+ // Lazy init stack view when a bubble is created
+ ensureStackViewCreated();
bubble.setInflateSynchronously(mInflateSynchronously);
bubble.inflate(
b -> {
@@ -1119,7 +1126,8 @@
}
}
- private void setIsBubble(@NonNull final NotificationEntry entry, final boolean isBubble) {
+ private void setIsBubble(@NonNull final NotificationEntry entry, final boolean isBubble,
+ final boolean autoExpand) {
Objects.requireNonNull(entry);
if (isBubble) {
entry.getSbn().getNotification().flags |= FLAG_BUBBLE;
@@ -1127,7 +1135,12 @@
entry.getSbn().getNotification().flags &= ~FLAG_BUBBLE;
}
try {
- mBarService.onNotificationBubbleChanged(entry.getKey(), isBubble, 0);
+ int flags = 0;
+ if (autoExpand) {
+ flags = Notification.BubbleMetadata.FLAG_SUPPRESS_NOTIFICATION;
+ flags |= Notification.BubbleMetadata.FLAG_AUTO_EXPAND_BUBBLE;
+ }
+ mBarService.onNotificationBubbleChanged(entry.getKey(), isBubble, flags);
} catch (RemoteException e) {
// Bad things have happened
}
@@ -1141,13 +1154,15 @@
b.disable(FLAG_BUBBLE);
}
if (b.getEntry() != null) {
- setIsBubble(b.getEntry(), isBubble);
+ // Updating the entry to be a bubble will trigger our normal update flow
+ setIsBubble(b.getEntry(), isBubble, b.shouldAutoExpand());
} else {
- try {
- mBarService.onNotificationBubbleChanged(b.getKey(), isBubble, 0);
- } catch (RemoteException e) {
- // Bad things have happened
- }
+ // If we have no entry to update, it's a persisted bubble so
+ // we need to add it to the stack ourselves
+ Bubble bubble = mBubbleData.getOrCreateBubble(null, b /* persistedBubble */);
+ inflateAndAdd(bubble, bubble.shouldAutoExpand() /* suppressFlyout */,
+ !bubble.shouldAutoExpand() /* showInShade */);
+
}
}
diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java
index 996a555..24d44d5 100644
--- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java
+++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java
@@ -115,7 +115,7 @@
/** Bubbles that aged out to overflow. */
private final List<Bubble> mOverflowBubbles;
/** Bubbles that are being loaded but haven't been added to the stack just yet. */
- private final List<Bubble> mPendingBubbles;
+ private final HashMap<String, Bubble> mPendingBubbles;
private Bubble mSelectedBubble;
private boolean mShowingOverflow;
private boolean mExpanded;
@@ -151,7 +151,7 @@
mContext = context;
mBubbles = new ArrayList<>();
mOverflowBubbles = new ArrayList<>();
- mPendingBubbles = new ArrayList<>();
+ mPendingBubbles = new HashMap<>();
mStateChange = new Update(mBubbles, mOverflowBubbles);
mMaxBubbles = mContext.getResources().getInteger(R.integer.bubbles_max_rendered);
mMaxOverflowBubbles = mContext.getResources().getInteger(R.integer.bubbles_max_overflow);
@@ -203,62 +203,46 @@
dispatchPendingChanges();
}
- public void promoteBubbleFromOverflow(Bubble bubble, BubbleStackView stack,
- BubbleIconFactory factory) {
- if (DEBUG_BUBBLE_DATA) {
- Log.d(TAG, "promoteBubbleFromOverflow: " + bubble);
- }
- mLogger.log(bubble, BubbleLogger.Event.BUBBLE_OVERFLOW_REMOVE_BACK_TO_STACK);
- moveOverflowBubbleToPending(bubble);
- // Preserve new order for next repack, which sorts by last updated time.
- bubble.inflate(
- b -> {
- b.setShouldAutoExpand(true);
- b.markUpdatedAt(mTimeSource.currentTimeMillis());
- notificationEntryUpdated(bubble, false /* suppressFlyout */,
- true /* showInShade */);
- },
- mContext, stack, factory, false /* skipInflation */);
- }
-
void setShowingOverflow(boolean showingOverflow) {
mShowingOverflow = showingOverflow;
}
- private void moveOverflowBubbleToPending(Bubble b) {
- mOverflowBubbles.remove(b);
- mPendingBubbles.add(b);
- }
-
/**
* Constructs a new bubble or returns an existing one. Does not add new bubbles to
* bubble data, must go through {@link #notificationEntryUpdated(Bubble, boolean, boolean)}
* for that.
+ *
+ * @param entry The notification entry to use, only null if it's a bubble being promoted from
+ * the overflow that was persisted over reboot.
+ * @param persistedBubble The bubble to use, only non-null if it's a bubble being promoted from
+ * the overflow that was persisted over reboot.
*/
- Bubble getOrCreateBubble(NotificationEntry entry) {
- String key = entry.getKey();
- Bubble bubble = getBubbleInStackWithKey(entry.getKey());
- if (bubble != null) {
- bubble.setEntry(entry);
- } else {
- bubble = getOverflowBubbleWithKey(key);
- if (bubble != null) {
- moveOverflowBubbleToPending(bubble);
- bubble.setEntry(entry);
- return bubble;
+ Bubble getOrCreateBubble(NotificationEntry entry, Bubble persistedBubble) {
+ String key = entry != null ? entry.getKey() : persistedBubble.getKey();
+ Bubble bubbleToReturn = getBubbleInStackWithKey(key);
+
+ if (bubbleToReturn == null) {
+ bubbleToReturn = getOverflowBubbleWithKey(key);
+ if (bubbleToReturn != null) {
+ // Promoting from overflow
+ mOverflowBubbles.remove(bubbleToReturn);
+ } else if (mPendingBubbles.containsKey(key)) {
+ // Update while it was pending
+ bubbleToReturn = mPendingBubbles.get(key);
+ } else if (entry != null) {
+ // New bubble
+ bubbleToReturn = new Bubble(entry, mSuppressionListener);
+ } else {
+ // Persisted bubble being promoted
+ bubbleToReturn = persistedBubble;
}
- // Check for it in pending
- for (int i = 0; i < mPendingBubbles.size(); i++) {
- Bubble b = mPendingBubbles.get(i);
- if (b.getKey().equals(entry.getKey())) {
- b.setEntry(entry);
- return b;
- }
- }
- bubble = new Bubble(entry, mSuppressionListener);
- mPendingBubbles.add(bubble);
}
- return bubble;
+
+ if (entry != null) {
+ bubbleToReturn.setEntry(entry);
+ }
+ mPendingBubbles.put(key, bubbleToReturn);
+ return bubbleToReturn;
}
/**
@@ -270,7 +254,7 @@
if (DEBUG_BUBBLE_DATA) {
Log.d(TAG, "notificationEntryUpdated: " + bubble);
}
- mPendingBubbles.remove(bubble); // No longer pending once we're here
+ mPendingBubbles.remove(bubble.getKey()); // No longer pending once we're here
Bubble prevBubble = getBubbleInStackWithKey(bubble.getKey());
suppressFlyout |= bubble.getEntry() == null
|| !bubble.getEntry().getRanking().visuallyInterruptive();
@@ -408,10 +392,8 @@
Log.d(TAG, "doRemove: " + key);
}
// If it was pending remove it
- for (int i = 0; i < mPendingBubbles.size(); i++) {
- if (mPendingBubbles.get(i).getKey().equals(key)) {
- mPendingBubbles.remove(mPendingBubbles.get(i));
- }
+ if (mPendingBubbles.containsKey(key)) {
+ mPendingBubbles.remove(key);
}
int indexToRemove = indexForKey(key);
if (indexToRemove == -1) {
diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleExpandedView.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleExpandedView.java
index c3dcc0b..db79802 100644
--- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleExpandedView.java
+++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleExpandedView.java
@@ -490,7 +490,7 @@
if (DEBUG_BUBBLE_EXPANDED_VIEW) {
Log.d(TAG, "update: bubble=" + (bubble != null ? bubble.getKey() : "null"));
}
- boolean isNew = mBubble == null;
+ boolean isNew = mBubble == null || didBackingContentChange(bubble);
if (isNew || bubble != null && bubble.getKey().equals(mBubble.getKey())) {
mBubble = bubble;
mSettingsIcon.setContentDescription(getResources().getString(
@@ -523,6 +523,12 @@
}
}
+ private boolean didBackingContentChange(Bubble newBubble) {
+ boolean prevWasIntentBased = mBubble != null && mPendingIntent != null;
+ boolean newIsIntentBased = newBubble.getBubbleIntent() != null;
+ return prevWasIntentBased != newIsIntentBased;
+ }
+
/**
* Lets activity view know it should be shown / populated with activity content.
*/
diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/dagger/BubbleModule.java b/packages/SystemUI/src/com/android/systemui/bubbles/dagger/BubbleModule.java
index d1d07f6..097932e 100644
--- a/packages/SystemUI/src/com/android/systemui/bubbles/dagger/BubbleModule.java
+++ b/packages/SystemUI/src/com/android/systemui/bubbles/dagger/BubbleModule.java
@@ -20,6 +20,7 @@
import android.content.Context;
import android.view.WindowManager;
+import com.android.internal.statusbar.IStatusBarService;
import com.android.systemui.bubbles.BubbleController;
import com.android.systemui.bubbles.BubbleData;
import com.android.systemui.bubbles.BubbleDataRepository;
@@ -70,6 +71,7 @@
BubbleDataRepository bubbleDataRepository,
SysUiState sysUiState,
INotificationManager notifManager,
+ IStatusBarService statusBarService,
WindowManager windowManager) {
return new BubbleController(
context,
@@ -91,6 +93,7 @@
bubbleDataRepository,
sysUiState,
notifManager,
+ statusBarService,
windowManager);
}
}