From db0a602465561183c920c5b0596e8051d076baa4 Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Wed, 13 May 2020 11:34:50 -0700 Subject: [PATCH 1/2] canBubble only true when it can actually bubble If the notif is unable to present as a bubble: - set the record#canBubble value to false - null out bubble metadata if present (we show bubble button if there is metadata, so shouldn't show it if it can't present for some reason) Because bubble metadata is now null'd out we don't have the shortcutId to check / update the tracked shortcut list, so instead if it's null check for that entry in the list and remove it. Also fixes one spot where we weren't unregistering the listener before and adds a test for it. Test: atest ShortcutHelperTest BubbleExtractorTest NotificationManagerServiceTest Bug: 152883583 Change-Id: I29e993c01a8fb9fcdf2386b18a94adc712fe101f --- .../server/notification/BubbleExtractor.java | 27 +++++++--- .../server/notification/ShortcutHelper.java | 40 +++++++++++---- .../notification/BubbleExtractorTest.java | 51 ++++++++++++++++--- .../NotificationManagerServiceTest.java | 10 ++-- .../notification/ShortcutHelperTest.java | 50 ++++++++++++++++-- 5 files changed, 143 insertions(+), 35 deletions(-) diff --git a/services/core/java/com/android/server/notification/BubbleExtractor.java b/services/core/java/com/android/server/notification/BubbleExtractor.java index d7d413c2ffb03..0fa339f65cdb5 100644 --- a/services/core/java/com/android/server/notification/BubbleExtractor.java +++ b/services/core/java/com/android/server/notification/BubbleExtractor.java @@ -75,9 +75,19 @@ public class BubbleExtractor implements NotificationSignalExtractor { mConfig.getBubblePreference( record.getSbn().getPackageName(), record.getSbn().getUid()); NotificationChannel recordChannel = record.getChannel(); + boolean canPresentAsBubble = canPresentAsBubble(record) + && !mActivityManager.isLowRamDevice() + && record.isConversation() + && (record.getNotification().flags & FLAG_FOREGROUND_SERVICE) == 0; - if (!mConfig.bubblesEnabled() || bubblePreference == BUBBLE_PREFERENCE_NONE) { + if (!mConfig.bubblesEnabled() + || bubblePreference == BUBBLE_PREFERENCE_NONE + || !canPresentAsBubble) { record.setAllowBubble(false); + if (!canPresentAsBubble) { + // clear out bubble metadata since it can't be used + record.getNotification().setBubbleMetadata(null); + } } else if (recordChannel == null) { // the app is allowed but there's no channel to check record.setAllowBubble(true); @@ -86,14 +96,15 @@ public class BubbleExtractor implements NotificationSignalExtractor { } else if (bubblePreference == BUBBLE_PREFERENCE_SELECTED) { record.setAllowBubble(recordChannel.canBubble()); } + if (DBG) { + Slog.d(TAG, "record: " + record.getKey() + + " appPref: " + bubblePreference + + " canBubble: " + record.canBubble() + + " canPresentAsBubble: " + canPresentAsBubble + + " flagRemoved: " + record.isFlagBubbleRemoved()); + } - final boolean fulfillsPolicy = record.canBubble() - && record.isConversation() - && !mActivityManager.isLowRamDevice() - && (record.getNotification().flags & FLAG_FOREGROUND_SERVICE) == 0; - final boolean applyFlag = fulfillsPolicy - && canPresentAsBubble(record) - && !record.isFlagBubbleRemoved(); + final boolean applyFlag = record.canBubble() && !record.isFlagBubbleRemoved(); if (applyFlag) { record.getNotification().flags |= FLAG_BUBBLE; } else { diff --git a/services/core/java/com/android/server/notification/ShortcutHelper.java b/services/core/java/com/android/server/notification/ShortcutHelper.java index e79d33fa5f7aa..0fa522cf51dee 100644 --- a/services/core/java/com/android/server/notification/ShortcutHelper.java +++ b/services/core/java/com/android/server/notification/ShortcutHelper.java @@ -28,6 +28,7 @@ import android.content.pm.ShortcutServiceInternal; import android.os.Binder; import android.os.Handler; import android.os.UserHandle; +import android.text.TextUtils; import android.util.Slog; import com.android.internal.annotations.VisibleForTesting; @@ -111,6 +112,15 @@ public class ShortcutHelper { } if (!foundShortcut) { bubbleKeysToRemove.add(shortcutBubbles.get(shortcutId)); + shortcutBubbles.remove(shortcutId); + if (shortcutBubbles.isEmpty()) { + mActiveShortcutBubbles.remove(packageName); + if (mLauncherAppsCallbackRegistered + && mActiveShortcutBubbles.isEmpty()) { + mLauncherAppsService.unregisterCallback(mLauncherAppsCallback); + mLauncherAppsCallbackRegistered = false; + } + } } } } @@ -209,15 +219,16 @@ public class ShortcutHelper { * @param removedNotification true if this notification is being removed * @param handler handler to register the callback with */ - void maybeListenForShortcutChangesForBubbles(NotificationRecord r, boolean removedNotification, + void maybeListenForShortcutChangesForBubbles(NotificationRecord r, + boolean removedNotification, Handler handler) { final String shortcutId = r.getNotification().getBubbleMetadata() != null ? r.getNotification().getBubbleMetadata().getShortcutId() : null; - if (shortcutId == null) { - return; - } - if (r.getNotification().isBubbleNotification() && !removedNotification) { + if (!removedNotification + && !TextUtils.isEmpty(shortcutId) + && r.getShortcutInfo() != null + && r.getShortcutInfo().getId().equals(shortcutId)) { // Must track shortcut based bubbles in case the shortcut is removed HashMap packageBubbles = mActiveShortcutBubbles.get( r.getSbn().getPackageName()); @@ -235,10 +246,21 @@ public class ShortcutHelper { HashMap packageBubbles = mActiveShortcutBubbles.get( r.getSbn().getPackageName()); if (packageBubbles != null) { - packageBubbles.remove(shortcutId); - } - if (packageBubbles != null && packageBubbles.isEmpty()) { - mActiveShortcutBubbles.remove(r.getSbn().getPackageName()); + if (!TextUtils.isEmpty(shortcutId)) { + packageBubbles.remove(shortcutId); + } else { + // Check if there was a matching entry + for (String pkgShortcutId : packageBubbles.keySet()) { + String entryKey = packageBubbles.get(pkgShortcutId); + if (r.getKey().equals(entryKey)) { + // No longer has shortcut id so remove it + packageBubbles.remove(pkgShortcutId); + } + } + } + if (packageBubbles.isEmpty()) { + mActiveShortcutBubbles.remove(r.getSbn().getPackageName()); + } } if (mLauncherAppsCallbackRegistered && mActiveShortcutBubbles.isEmpty()) { mLauncherAppsService.unregisterCallback(mLauncherAppsCallback); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/BubbleExtractorTest.java b/services/tests/uiservicestests/src/com/android/server/notification/BubbleExtractorTest.java index 38b71b707196f..13457f0a284cc 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/BubbleExtractorTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/BubbleExtractorTest.java @@ -26,6 +26,8 @@ import static android.content.pm.ActivityInfo.RESIZE_MODE_RESIZEABLE; import static android.content.pm.ActivityInfo.RESIZE_MODE_UNRESIZEABLE; import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertNotNull; +import static junit.framework.Assert.assertNull; import static junit.framework.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -166,6 +168,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { setUpBubblesEnabled(true /* feature */, BUBBLE_PREFERENCE_ALL /* app */, ALLOW_BUBBLE_OFF /* channel */); + when(mActivityManager.isLowRamDevice()).thenReturn(false); + setUpShortcutBubble(true /* isValid */); NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); @@ -178,6 +182,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { setUpBubblesEnabled(true /* feature */, BUBBLE_PREFERENCE_ALL /* app */, DEFAULT_ALLOW_BUBBLE /* channel */); + when(mActivityManager.isLowRamDevice()).thenReturn(false); + setUpShortcutBubble(true /* isValid */); NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); @@ -190,6 +196,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { setUpBubblesEnabled(true /* feature */, BUBBLE_PREFERENCE_ALL /* app */, ALLOW_BUBBLE_ON /* channel */); + when(mActivityManager.isLowRamDevice()).thenReturn(false); + setUpShortcutBubble(true /* isValid */); NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); @@ -202,6 +210,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { setUpBubblesEnabled(false /* feature */, BUBBLE_PREFERENCE_ALL /* app */, ALLOW_BUBBLE_ON /* channel */); + when(mActivityManager.isLowRamDevice()).thenReturn(false); + setUpShortcutBubble(true /* isValid */); NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); @@ -215,6 +225,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { setUpBubblesEnabled(true /* feature */, BUBBLE_PREFERENCE_NONE /* app */, ALLOW_BUBBLE_ON /* channel */); + when(mActivityManager.isLowRamDevice()).thenReturn(false); + setUpShortcutBubble(true /* isValid */); NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); @@ -228,6 +240,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { setUpBubblesEnabled(true /* feature */, BUBBLE_PREFERENCE_NONE /* app */, DEFAULT_ALLOW_BUBBLE /* channel */); + when(mActivityManager.isLowRamDevice()).thenReturn(false); + setUpShortcutBubble(true /* isValid */); NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); @@ -241,6 +255,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { setUpBubblesEnabled(true /* feature */, BUBBLE_PREFERENCE_SELECTED /* app */, DEFAULT_ALLOW_BUBBLE /* channel */); + when(mActivityManager.isLowRamDevice()).thenReturn(false); + setUpShortcutBubble(true /* isValid */); NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); @@ -254,6 +270,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { setUpBubblesEnabled(true /* feature */, BUBBLE_PREFERENCE_SELECTED /* app */, ALLOW_BUBBLE_OFF /* channel */); + when(mActivityManager.isLowRamDevice()).thenReturn(false); + setUpShortcutBubble(true /* isValid */); NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); @@ -267,6 +285,9 @@ public class BubbleExtractorTest extends UiServiceTestCase { setUpBubblesEnabled(true /* feature */, BUBBLE_PREFERENCE_SELECTED /* app */, ALLOW_BUBBLE_ON /* channel */); + when(mActivityManager.isLowRamDevice()).thenReturn(false); + setUpShortcutBubble(true /* isValid */); + NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); @@ -279,6 +300,9 @@ public class BubbleExtractorTest extends UiServiceTestCase { setUpBubblesEnabled(false /* feature */, BUBBLE_PREFERENCE_SELECTED /* app */, ALLOW_BUBBLE_ON /* channel */); + when(mActivityManager.isLowRamDevice()).thenReturn(false); + setUpShortcutBubble(true /* isValid */); + NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); @@ -305,6 +329,7 @@ public class BubbleExtractorTest extends UiServiceTestCase { mBubbleExtractor.process(r); assertTrue(r.canBubble()); + assertNotNull(r.getNotification().getBubbleMetadata()); assertFalse(r.getNotification().isBubbleNotification()); } @@ -320,6 +345,7 @@ public class BubbleExtractorTest extends UiServiceTestCase { mBubbleExtractor.process(r); assertTrue(r.canBubble()); + assertNotNull(r.getNotification().getBubbleMetadata()); assertTrue(r.getNotification().isBubbleNotification()); } @@ -335,6 +361,7 @@ public class BubbleExtractorTest extends UiServiceTestCase { mBubbleExtractor.process(r); assertTrue(r.canBubble()); + assertNotNull(r.getNotification().getBubbleMetadata()); assertTrue(r.getNotification().isBubbleNotification()); } @@ -350,7 +377,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { r.setShortcutInfo(null); mBubbleExtractor.process(r); - assertTrue(r.canBubble()); + assertFalse(r.canBubble()); + assertNull(r.getNotification().getBubbleMetadata()); assertFalse(r.getNotification().isBubbleNotification()); } @@ -366,7 +394,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { r.setShortcutInfo(null); mBubbleExtractor.process(r); - assertTrue(r.canBubble()); + assertFalse(r.canBubble()); + assertNull(r.getNotification().getBubbleMetadata()); assertFalse(r.getNotification().isBubbleNotification()); } @@ -381,7 +410,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); - assertTrue(r.canBubble()); + assertFalse(r.canBubble()); + assertNull(r.getNotification().getBubbleMetadata()); assertFalse(r.getNotification().isBubbleNotification()); } @@ -395,7 +425,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { NotificationRecord r = getNotificationRecord(false /* bubble */); mBubbleExtractor.process(r); - assertTrue(r.canBubble()); + assertFalse(r.canBubble()); + assertNull(r.getNotification().getBubbleMetadata()); assertFalse(r.getNotification().isBubbleNotification()); } @@ -414,7 +445,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { mBubbleExtractor.process(r); - assertTrue(r.canBubble()); + assertFalse(r.canBubble()); + assertNull(r.getNotification().getBubbleMetadata()); assertFalse(r.getNotification().isBubbleNotification()); } @@ -429,7 +461,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); - assertTrue(r.canBubble()); + assertFalse(r.canBubble()); + assertNull(r.getNotification().getBubbleMetadata()); assertFalse(r.getNotification().isBubbleNotification()); } @@ -445,7 +478,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); - assertTrue(r.canBubble()); + assertFalse(r.canBubble()); + assertNull(r.getNotification().getBubbleMetadata()); assertFalse(r.getNotification().isBubbleNotification()); } @@ -462,7 +496,8 @@ public class BubbleExtractorTest extends UiServiceTestCase { NotificationRecord r = getNotificationRecord(true /* bubble */); mBubbleExtractor.process(r); - assertTrue(r.canBubble()); + assertFalse(r.canBubble()); + assertNull(r.getNotification().getBubbleMetadata()); assertFalse(r.getNotification().isBubbleNotification()); } } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index ced780475fb7d..2bea491949dec 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -6135,8 +6135,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { "tag", mUid, 0, nb.build(), new UserHandle(mUid), null, 0); NotificationRecord nr = new NotificationRecord(mContext, sbn, mTestNotificationChannel); - - // Test: Send the bubble notification mBinderService.enqueueNotificationWithTag(PKG, PKG, nr.getSbn().getTag(), nr.getSbn().getId(), nr.getSbn().getNotification(), nr.getSbn().getUserId()); @@ -6168,12 +6166,12 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { verify(mLauncherApps, times(1)).unregisterCallback(launcherAppsCallback.getValue()); // We're no longer a bubble - Notification notif2 = mService.getNotificationRecord( - nr.getSbn().getKey()).getNotification(); - assertFalse(notif2.isBubbleNotification()); + NotificationRecord notif2 = mService.getNotificationRecord( + nr.getSbn().getKey()); + assertNull(notif2.getShortcutInfo()); + assertFalse(notif2.getNotification().isBubbleNotification()); } - @Test public void testNotificationBubbles_shortcut_stopListeningWhenNotifRemoved() throws RemoteException { diff --git a/services/tests/uiservicestests/src/com/android/server/notification/ShortcutHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/ShortcutHelperTest.java index eb2d9fed197fb..c700a090fa2e6 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ShortcutHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ShortcutHelperTest.java @@ -48,6 +48,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.util.ArrayList; +import java.util.Collections; import java.util.List; @SmallTest @@ -73,6 +74,8 @@ public class ShortcutHelperTest extends UiServiceTestCase { StatusBarNotification mSbn; @Mock Notification.BubbleMetadata mBubbleMetadata; + @Mock + ShortcutInfo mShortcutInfo; ShortcutHelper mShortcutHelper; @@ -86,13 +89,13 @@ public class ShortcutHelperTest extends UiServiceTestCase { when(mNr.getSbn()).thenReturn(mSbn); when(mSbn.getPackageName()).thenReturn(PKG); when(mNr.getNotification()).thenReturn(mNotif); + when(mNr.getShortcutInfo()).thenReturn(mShortcutInfo); + when(mShortcutInfo.getId()).thenReturn(SHORTCUT_ID); when(mNotif.getBubbleMetadata()).thenReturn(mBubbleMetadata); when(mBubbleMetadata.getShortcutId()).thenReturn(SHORTCUT_ID); } private LauncherApps.Callback addShortcutBubbleAndVerifyListener() { - when(mNotif.isBubbleNotification()).thenReturn(true); - mShortcutHelper.maybeListenForShortcutChangesForBubbles(mNr, false /* removed */, null /* handler */); @@ -124,12 +127,12 @@ public class ShortcutHelperTest extends UiServiceTestCase { } @Test - public void testBubbleNoLongerBubble_listenerRemoved() { + public void testBubbleNoLongerHasBubbleMetadata_listenerRemoved() { // First set it up to listen addShortcutBubbleAndVerifyListener(); // Then make it not a bubble - when(mNotif.isBubbleNotification()).thenReturn(false); + when(mNotif.getBubbleMetadata()).thenReturn(null); mShortcutHelper.maybeListenForShortcutChangesForBubbles(mNr, false /* removed */, null /* handler */); @@ -137,6 +140,45 @@ public class ShortcutHelperTest extends UiServiceTestCase { verify(mLauncherApps, times(1)).unregisterCallback(any()); } + @Test + public void testBubbleNoLongerHasShortcutId_listenerRemoved() { + // First set it up to listen + addShortcutBubbleAndVerifyListener(); + + // Clear out shortcutId + when(mBubbleMetadata.getShortcutId()).thenReturn(null); + mShortcutHelper.maybeListenForShortcutChangesForBubbles(mNr, + false /* removed */, + null /* handler */); + + verify(mLauncherApps, times(1)).unregisterCallback(any()); + } + + @Test + public void testNotifNoLongerHasShortcut_listenerRemoved() { + // First set it up to listen + addShortcutBubbleAndVerifyListener(); + + // Clear out shortcutId + when(mNr.getShortcutInfo()).thenReturn(null); + mShortcutHelper.maybeListenForShortcutChangesForBubbles(mNr, + false /* removed */, + null /* handler */); + + verify(mLauncherApps, times(1)).unregisterCallback(any()); + } + + @Test + public void testOnShortcutsChanged_listenerRemoved() { + // First set it up to listen + LauncherApps.Callback callback = addShortcutBubbleAndVerifyListener(); + + // App shortcuts are removed: + callback.onShortcutsChanged(PKG, Collections.emptyList(), mock(UserHandle.class)); + + verify(mLauncherApps, times(1)).unregisterCallback(any()); + } + @Test public void testListenerNotifiedOnShortcutRemoved() { LauncherApps.Callback callback = addShortcutBubbleAndVerifyListener(); From 76343014ce9f66d5d343eac1828563962aa69dcc Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Wed, 13 May 2020 11:02:50 -0700 Subject: [PATCH 2/2] 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 --- .../com/android/systemui/bubbles/Bubble.java | 8 ++- .../systemui/bubbles/BubbleController.java | 58 +++++++++++-------- .../android/systemui/bubbles/BubbleData.java | 14 ++--- ...otificationInterruptStateProviderImpl.java | 8 --- .../StatusBarNotificationActivityStarter.java | 16 ++--- .../bubbles/BubbleControllerTest.java | 7 ++- .../NewNotifPipelineBubbleControllerTest.java | 5 ++ ...tusBarNotificationActivityStarterTest.java | 6 +- 8 files changed, 69 insertions(+), 53 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/Bubble.java b/packages/SystemUI/src/com/android/systemui/bubbles/Bubble.java index 24ef324842d9a..ecd8b4585609c 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 @@ 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 diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java index eb4ba6f682af8..f0f28fd9f28d4 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 @@ 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; } } diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java index 43f65ded29cb8..35647b0bb2f1a 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 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); 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 da31fe03c9e7b..71f6dac4e234f 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 @@ 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)) { 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 64e5f0a8184ef..7bcfb466d7d93 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 @@ 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)); } } 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 96e868d2a6180..9b377ca3ec90b 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.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()); 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 73b8760198630..b18d67bf27263 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.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()); 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 53a1773b1bd18..acdb2c59dc0c2 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 @@ 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();