From ef2ef6c909f28a494f812019ee672db4531725be Mon Sep 17 00:00:00 2001 From: Ned Burns Date: Wed, 2 Jan 2019 16:48:08 -0500 Subject: [PATCH 1/3] Change onRemoteEntry() to only fire when entries are removed Conceptually, this should be a paired match with onNotificationAdded(). Previously we were firing this method even if the notification was no longer present (already removed) or if it hadn't been added yet (was in pending). Test: atest Change-Id: I613f60aa8cf4e1aeb7bb13ff5883a221c9b623c6 --- ...ForegroundServiceNotificationListener.java | 7 +--- .../statusbar/NotificationMediaManager.java | 10 ++---- .../NotificationRemoteInputManager.java | 5 +-- .../NotificationAlertingManager.java | 8 ++--- .../NotificationEntryListener.java | 8 +---- .../NotificationEntryManager.java | 17 ++++++---- .../logging/NotificationLogger.java | 10 ++---- .../NotificationGroupAlertTransferHelper.java | 5 +-- .../phone/StatusBarNotificationPresenter.java | 8 ++--- .../ForegroundServiceControllerTest.java | 2 +- .../NotificationEntryManagerTest.java | 32 ++++++++++++++++--- .../logging/NotificationLoggerTest.java | 5 --- ...ificationGroupAlertTransferHelperTest.java | 3 +- 13 files changed, 54 insertions(+), 66 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java b/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java index 151a6b0131103..b0b7e6c88984a 100644 --- a/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java +++ b/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java @@ -61,14 +61,9 @@ public class ForegroundServiceNotificationListener { @Override public void onEntryRemoved( NotificationData.Entry entry, - String key, - StatusBarNotification old, NotificationVisibility visibility, - boolean lifetimeExtended, boolean removedByUser) { - if (entry != null && !lifetimeExtended) { - removeNotification(entry.notification); - } + removeNotification(entry.notification); } }); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationMediaManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationMediaManager.java index 1bf101c00711f..e59bc2a82c4cf 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationMediaManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationMediaManager.java @@ -37,7 +37,6 @@ import android.media.session.PlaybackState; import android.os.Handler; import android.os.Trace; import android.os.UserHandle; -import android.service.notification.StatusBarNotification; import android.util.Log; import android.view.View; import android.widget.ImageView; @@ -157,15 +156,10 @@ public class NotificationMediaManager implements Dumpable { notificationEntryManager.addNotificationEntryListener(new NotificationEntryListener() { @Override public void onEntryRemoved( - @Nullable Entry entry, - String key, - StatusBarNotification old, + Entry entry, NotificationVisibility visibility, - boolean lifetimeExtended, boolean removedByUser) { - if (!lifetimeExtended) { - onNotificationRemoved(key); - } + onNotificationRemoved(entry.key); } }); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java index 886d99eeff176..1ab9c5c2b4a2a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoteInputManager.java @@ -254,13 +254,10 @@ public class NotificationRemoteInputManager implements Dumpable { @Override public void onEntryRemoved( @Nullable NotificationData.Entry entry, - String key, - StatusBarNotification old, NotificationVisibility visibility, - boolean lifetimeExtended, boolean removedByUser) { if (removedByUser && entry != null) { - onPerformRemoveNotification(entry, key); + onPerformRemoveNotification(entry, entry.key); } } }); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationAlertingManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationAlertingManager.java index 7b42dd901d728..2bb0d5ce9161f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationAlertingManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationAlertingManager.java @@ -20,7 +20,6 @@ import static com.android.systemui.statusbar.NotificationRemoteInputManager.FORC import static com.android.systemui.statusbar.notification.row.NotificationInflater.FLAG_CONTENT_VIEW_AMBIENT; import static com.android.systemui.statusbar.notification.row.NotificationInflater.FLAG_CONTENT_VIEW_HEADS_UP; -import android.annotation.Nullable; import android.app.Notification; import android.service.notification.StatusBarNotification; import android.util.Log; @@ -83,13 +82,10 @@ public class NotificationAlertingManager { @Override public void onEntryRemoved( - @Nullable NotificationData.Entry entry, - String key, - StatusBarNotification old, + NotificationData.Entry entry, NotificationVisibility visibility, - boolean lifetimeExtended, boolean removedByUser) { - stopAlerting(key); + stopAlerting(entry.key); } }); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryListener.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryListener.java index 1d06ce031f2be..2f60f115ed530 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryListener.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryListener.java @@ -71,20 +71,14 @@ public interface NotificationEntryListener { * because the developer retracted it). * @param entry notification data entry that was removed. Null if no entry existed for the * removed key at the time of removal. - * @param key key of notification that was removed - * @param old StatusBarNotification of the notification before it was removed * @param visibility logging data related to the visibility of the notification at the time of * removal, if it was removed by a user action. Null if it was not removed by * a user action. - * @param lifetimeExtended true if something is artificially extending how long the notification * @param removedByUser true if the notification was removed by a user action */ default void onEntryRemoved( - @Nullable NotificationData.Entry entry, - String key, - StatusBarNotification old, + NotificationData.Entry entry, @Nullable NotificationVisibility visibility, - boolean lifetimeExtended, boolean removedByUser) { } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java index 2d316508c8cf2..b679eb3225ca2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java @@ -153,6 +153,12 @@ public class NotificationEntryManager implements return mNotificationRowBinder; } + // TODO: Remove this once we can always use a mocked row binder in our tests + @VisibleForTesting + void setRowBinder(NotificationRowBinder notificationRowBinder) { + mNotificationRowBinder = notificationRowBinder; + } + public void setUpWithPresenter(NotificationPresenter presenter, NotificationListContainer listContainer, HeadsUpManager headsUpManager) { @@ -309,7 +315,6 @@ public class NotificationEntryManager implements abortExistingInflation(key); - StatusBarNotification old = null; boolean lifetimeExtended = false; if (entry != null) { @@ -342,12 +347,12 @@ public class NotificationEntryManager implements // Let's remove the children if this was a summary handleGroupSummaryRemoved(key); - old = removeNotificationViews(key, ranking); - } - } + removeNotificationViews(key, ranking); - for (NotificationEntryListener listener : mNotificationEntryListeners) { - listener.onEntryRemoved(entry, key, old, visibility, lifetimeExtended, removedByUser); + for (NotificationEntryListener listener : mNotificationEntryListeners) { + listener.onEntryRemoved(entry, visibility, removedByUser); + } + } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java index 610d3003f0e93..43048a2c087e1 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java @@ -15,7 +15,6 @@ */ package com.android.systemui.statusbar.notification.logging; -import android.annotation.Nullable; import android.content.Context; import android.os.Handler; import android.os.RemoteException; @@ -168,14 +167,11 @@ public class NotificationLogger implements StateListener { entryManager.addNotificationEntryListener(new NotificationEntryListener() { @Override public void onEntryRemoved( - @Nullable NotificationData.Entry entry, - String key, - StatusBarNotification old, + NotificationData.Entry entry, NotificationVisibility visibility, - boolean lifetimeExtended, boolean removedByUser) { - if (removedByUser && visibility != null && entry != null) { - logNotificationClear(key, entry.notification, visibility); + if (removedByUser && visibility != null) { + logNotificationClear(entry.key, entry.notification, visibility); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelper.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelper.java index 3839ed5d5d88f..af3257ae423ea 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelper.java @@ -220,15 +220,12 @@ public class NotificationGroupAlertTransferHelper implements OnHeadsUpChangedLis @Override public void onEntryRemoved( @Nullable Entry entry, - String key, - StatusBarNotification old, NotificationVisibility visibility, - boolean lifetimeExtended, boolean removedByUser) { // Removes any alerts pending on this entry. Note that this will not stop any inflation // tasks started by a transfer, so this should only be used as clean-up for when // inflation is stopped and the pending alert no longer needs to happen. - mPendingAlerts.remove(key); + mPendingAlerts.remove(entry.key); } }; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationPresenter.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationPresenter.java index b9372e8935adc..fb3157a128d67 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationPresenter.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationPresenter.java @@ -196,14 +196,10 @@ public class StatusBarNotificationPresenter implements NotificationPresenter, @Override public void onEntryRemoved( @Nullable Entry entry, - String key, - StatusBarNotification old, NotificationVisibility visibility, - boolean lifetimeExtended, boolean removedByUser) { - if (!lifetimeExtended) { - StatusBarNotificationPresenter.this.onNotificationRemoved(key, old); - } + StatusBarNotificationPresenter.this.onNotificationRemoved( + entry.key, entry.notification); if (removedByUser) { maybeEndAmbientPulse(); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceControllerTest.java index 199a3a62d72bc..f8912bcca3020 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/ForegroundServiceControllerTest.java @@ -392,7 +392,7 @@ public class ForegroundServiceControllerTest extends SysuiTestCase { private void entryRemoved(StatusBarNotification notification) { mEntryListener.onEntryRemoved(new NotificationData.Entry(notification), - null, null, null, false, false); + null, false); } private void entryAdded(StatusBarNotification notification, int importance) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java index 57c9d29b24f46..6197341acda54 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java @@ -121,6 +121,7 @@ public class NotificationEntryManagerTest extends SysuiTestCase { @Mock private MetricsLogger mMetricsLogger; @Mock private SmartReplyController mSmartReplyController; @Mock private RowInflaterTask mAsyncInflationTask; + @Mock private NotificationRowBinder mMockedRowBinder; private NotificationData.Entry mEntry; private StatusBarNotification mSbn; @@ -310,8 +311,8 @@ public class NotificationEntryManagerTest extends SysuiTestCase { verify(mListContainer).cleanUpViewStateForEntry(mEntry); verify(mPresenter).updateNotificationViews(); - verify(mEntryListener).onEntryRemoved(mEntry, mSbn.getKey(), mSbn, - null, false /* lifetimeExtended */, false /* removedByUser */); + verify(mEntryListener).onEntryRemoved( + mEntry, null, false /* removedByUser */); verify(mRow).setRemoved(); assertNull(mEntryManager.getNotificationData().get(mSbn.getKey())); @@ -335,8 +336,31 @@ public class NotificationEntryManagerTest extends SysuiTestCase { assertNotNull(mEntryManager.getNotificationData().get(mSbn.getKey())); verify(extender).setShouldManageLifetime(mEntry, true /* shouldManage */); - verify(mEntryListener).onEntryRemoved(mEntry, mSbn.getKey(), null, - null, true /* lifetimeExtended */, false /* removedByUser */); + verify(mEntryListener, never()).onEntryRemoved( + mEntry, null, false /* removedByUser */); + } + + @Test + public void testRemoveNotification_onEntryRemoveNotFiredIfEntryDoesntExist() { + com.android.systemui.util.Assert.isNotMainThread(); + + mEntryManager.removeNotification("not_a_real_key", mRankingMap); + + verify(mEntryListener, never()).onEntryRemoved( + mEntry, null, false /* removedByUser */); + } + + @Test + public void testRemoveNotification_whilePending() throws InterruptedException { + com.android.systemui.util.Assert.isNotMainThread(); + + mEntryManager.setRowBinder(mMockedRowBinder); + + mEntryManager.addNotification(mSbn, mRankingMap); + mEntryManager.removeNotification(mSbn.getKey(), mRankingMap); + + verify(mEntryListener, never()).onEntryRemoved( + mEntry, null, false /* removedByUser */); } @Test diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationLoggerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationLoggerTest.java index 983ca837b6396..afdeb62a8f28d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationLoggerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationLoggerTest.java @@ -159,11 +159,6 @@ public class NotificationLoggerTest extends SysuiTestCase { verify(mBarService, times(1)).onNotificationVisibilityChanged(any(), any()); } - @Test - public void testHandleNullEntryOnEntryRemoved() { - mNotificationEntryListener.onEntryRemoved(null, "foobar", null, null, false, false); - } - private class TestableNotificationLogger extends NotificationLogger { TestableNotificationLogger(NotificationListener notificationListener, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelperTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelperTest.java index 56af40054316d..79695fd6d38d2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelperTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelperTest.java @@ -237,8 +237,7 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { mGroupManager.onEntryAdded(summaryEntry); mGroupManager.onEntryAdded(childEntry); - mNotificationEntryListener.onEntryRemoved(childEntry, childEntry.key, null, null, - false, false); + mNotificationEntryListener.onEntryRemoved(childEntry, null, false); assertFalse(mGroupAlertTransferHelper.isAlertTransferPending(childEntry)); } From 6976087e24c61413c278f7f7f443c9297fd365dc Mon Sep 17 00:00:00 2001 From: Ned Burns Date: Thu, 3 Jan 2019 15:36:38 -0500 Subject: [PATCH 2/3] Inline a few methods in NotificationEntryManager These methods were only called in one location and either (a) weren't safe to call in any other situation or (b) weren't conceptually distinct from their caller. Test: atest Change-Id: I7bbb2e9b51b678144f13897db27ad324e78be587 --- .../NotificationEntryManager.java | 70 ++++++------------- 1 file changed, 21 insertions(+), 49 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java index b679eb3225ca2..ec388db060e24 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java @@ -253,21 +253,6 @@ public class NotificationEntryManager implements } } - private void addEntry(NotificationData.Entry shadeEntry) { - if (shadeEntry == null) { - return; - } - // Add the expanded view and icon. - mNotificationData.add(shadeEntry); - tagForeground(shadeEntry.notification); - updateNotifications(); - for (NotificationEntryListener listener : mNotificationEntryListeners) { - listener.onNotificationAdded(shadeEntry); - } - - maybeScheduleUpdateNotificationViews(shadeEntry); - } - private void maybeScheduleUpdateNotificationViews(NotificationData.Entry entry) { long audibleAlertTimeout = RECENTLY_ALERTED_THRESHOLD_MS - (System.currentTimeMillis() - entry.lastAudiblyAlertedMs); @@ -289,7 +274,13 @@ public class NotificationEntryManager implements for (NotificationEntryListener listener : mNotificationEntryListeners) { listener.onEntryInflated(entry, inflatedFlags); } - addEntry(entry); + mNotificationData.add(entry); + tagForeground(entry.notification); + updateNotifications(); + for (NotificationEntryListener listener : mNotificationEntryListeners) { + listener.onNotificationAdded(entry); + } + maybeScheduleUpdateNotificationViews(entry); } else { for (NotificationEntryListener listener : mNotificationEntryListeners) { listener.onEntryReinflated(entry); @@ -347,7 +338,9 @@ public class NotificationEntryManager implements // Let's remove the children if this was a summary handleGroupSummaryRemoved(key); - removeNotificationViews(key, ranking); + mNotificationData.remove(key, ranking); + updateNotifications(); + Dependency.get(LeakDetector.class).trackGarbage(entry); for (NotificationEntryListener listener : mNotificationEntryListeners) { listener.onEntryRemoved(entry, visibility, removedByUser); @@ -356,18 +349,6 @@ public class NotificationEntryManager implements } } - private StatusBarNotification removeNotificationViews(String key, - NotificationListenerService.RankingMap ranking) { - NotificationData.Entry entry = mNotificationData.remove(key, ranking); - if (entry == null) { - Log.w(TAG, "removeNotification for unknown key: " + key); - return null; - } - updateNotifications(); - Dependency.get(LeakDetector.class).trackGarbage(entry); - return entry.notification; - } - /** * Ensures that the group children are cancelled immediately when the group summary is cancelled * instead of waiting for the notification manager to send all cancels. Otherwise this could @@ -423,25 +404,6 @@ public class NotificationEntryManager implements } } - private NotificationData.Entry createNotificationEntry( - StatusBarNotification sbn, NotificationListenerService.Ranking ranking) - throws InflationException { - if (DEBUG) { - Log.d(TAG, "createNotificationEntry(notification=" + sbn + " " + ranking); - } - - NotificationData.Entry entry = new NotificationData.Entry(sbn, ranking); - if (BubbleController.shouldAutoBubble(getContext(), entry)) { - entry.setIsBubble(true); - } - - Dependency.get(LeakDetector.class).trackInstance(entry); - // Construct the expanded view. - getRowBinder().inflateViews(entry, () -> performRemoveNotification(sbn), - mNotificationData.get(entry.key) != null); - return entry; - } - private void addNotificationInternal(StatusBarNotification notification, NotificationListenerService.RankingMap rankingMap) throws InflationException { String key = notification.getKey(); @@ -452,7 +414,17 @@ public class NotificationEntryManager implements mNotificationData.updateRanking(rankingMap); NotificationListenerService.Ranking ranking = new NotificationListenerService.Ranking(); rankingMap.getRanking(key, ranking); - NotificationData.Entry entry = createNotificationEntry(notification, ranking); + + NotificationData.Entry entry = new NotificationData.Entry(notification, ranking); + if (BubbleController.shouldAutoBubble(getContext(), entry)) { + entry.setIsBubble(true); + } + + Dependency.get(LeakDetector.class).trackInstance(entry); + // Construct the expanded view. + getRowBinder().inflateViews(entry, () -> performRemoveNotification(notification), + mNotificationData.get(entry.key) != null); + abortExistingInflation(key); mPendingNotifications.put(key, entry); From 01e3821aa335459e626bf3326f4e0eb31cab4ded Mon Sep 17 00:00:00 2001 From: Ned Burns Date: Thu, 3 Jan 2019 16:32:52 -0500 Subject: [PATCH 3/3] Invert BubbleController <-> NEM dependency Test: atest Change-Id: I35b9dfbead7c623a6d0321943570c6115cf7eb5e --- .../systemui/bubbles/BubbleController.java | 78 +++++++++---------- .../NotificationEntryManager.java | 28 +------ .../bubbles/BubbleControllerTest.java | 34 ++++++++ .../NotificationViewHierarchyManagerTest.java | 4 +- 4 files changed, 76 insertions(+), 68 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java index 881aa18285ff8..644723321103d 100644 --- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java +++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java @@ -33,8 +33,11 @@ import android.view.WindowManager; import android.widget.FrameLayout; import com.android.internal.annotations.VisibleForTesting; +import com.android.systemui.Dependency; import com.android.systemui.R; import com.android.systemui.statusbar.notification.NotificationData; +import com.android.systemui.statusbar.notification.NotificationEntryListener; +import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.phone.StatusBarWindowController; import java.util.ArrayList; @@ -57,46 +60,30 @@ public class BubbleController { private static final String TAG = "BubbleController"; // Enables some subset of notifs to automatically become bubbles - public static final boolean DEBUG_ENABLE_AUTO_BUBBLE = false; + private static final boolean DEBUG_ENABLE_AUTO_BUBBLE = false; // When a bubble is dismissed, recreate it as a notification - public static final boolean DEBUG_DEMOTE_TO_NOTIF = false; + private static final boolean DEBUG_DEMOTE_TO_NOTIF = false; // Secure settings private static final String ENABLE_AUTO_BUBBLE_MESSAGES = "experiment_autobubble_messaging"; private static final String ENABLE_AUTO_BUBBLE_ONGOING = "experiment_autobubble_ongoing"; private static final String ENABLE_AUTO_BUBBLE_ALL = "experiment_autobubble_all"; - private Context mContext; - private BubbleDismissListener mDismissListener; + private final Context mContext; + private final NotificationEntryManager mNotificationEntryManager; private BubbleStateChangeListener mStateChangeListener; private BubbleExpandListener mExpandListener; - private Map mBubbles = new HashMap<>(); + private final Map mBubbles = new HashMap<>(); private BubbleStackView mStackView; - private Point mDisplaySize; + private final Point mDisplaySize; // Bubbles get added to the status bar view - @VisibleForTesting - protected StatusBarWindowController mStatusBarWindowController; + private final StatusBarWindowController mStatusBarWindowController; // Used for determining view rect for touch interaction private Rect mTempRect = new Rect(); - /** - * Listener to find out about bubble / bubble stack dismissal events. - */ - public interface BubbleDismissListener { - /** - * Called when the entire stack of bubbles is dismissed by the user. - */ - void onStackDismissed(); - - /** - * Called when a specific bubble is dismissed by the user. - */ - void onBubbleDismissed(String key); - } - /** * Listener to be notified when some states of the bubbles change. */ @@ -123,17 +110,13 @@ public class BubbleController { @Inject public BubbleController(Context context, StatusBarWindowController statusBarWindowController) { mContext = context; + mNotificationEntryManager = Dependency.get(NotificationEntryManager.class); WindowManager wm = (WindowManager) context.getSystemService(Context.WINDOW_SERVICE); mDisplaySize = new Point(); wm.getDefaultDisplay().getSize(mDisplaySize); mStatusBarWindowController = statusBarWindowController; - } - /** - * Set a listener to be notified of bubble dismissal events. - */ - public void setDismissListener(BubbleDismissListener listener) { - mDismissListener = listener; + mNotificationEntryManager.addNotificationEntryListener(mEntryListener); } /** @@ -180,7 +163,7 @@ public class BubbleController { /** * Tell the stack of bubbles to be dismissed, this will remove all of the bubbles in the stack. */ - public void dismissStack() { + void dismissStack() { if (mStackView == null) { return; } @@ -190,9 +173,7 @@ public class BubbleController { for (String key: mBubbles.keySet()) { removeBubble(key); } - if (mDismissListener != null) { - mDismissListener.onStackDismissed(); - } + mNotificationEntryManager.updateNotifications(); updateBubblesShowing(); } @@ -238,18 +219,35 @@ public class BubbleController { /** * Removes the bubble associated with the {@param uri}. */ - public void removeBubble(String key) { + void removeBubble(String key) { BubbleView bv = mBubbles.get(key); if (mStackView != null && bv != null) { mStackView.removeBubble(bv); bv.getEntry().setBubbleDismissed(true); } - if (mDismissListener != null) { - mDismissListener.onBubbleDismissed(key); + + NotificationData.Entry entry = mNotificationEntryManager.getNotificationData().get(key); + if (entry != null) { + entry.setBubbleDismissed(true); + if (!DEBUG_DEMOTE_TO_NOTIF) { + mNotificationEntryManager.performRemoveNotification(entry.notification); + } } + mNotificationEntryManager.updateNotifications(); + updateBubblesShowing(); } + @SuppressWarnings("FieldCanBeLocal") + private final NotificationEntryListener mEntryListener = new NotificationEntryListener() { + @Override + public void onPendingEntryAdded(NotificationData.Entry entry) { + if (shouldAutoBubble(mContext, entry)) { + entry.setIsBubble(true); + } + } + }; + private void updateBubblesShowing() { boolean hasBubblesShowing = false; for (BubbleView bv : mBubbles.values()) { @@ -309,7 +307,7 @@ public class BubbleController { } @VisibleForTesting - public BubbleStackView getStackView() { + BubbleStackView getStackView() { return mStackView; } @@ -317,7 +315,7 @@ public class BubbleController { /** * Gets an appropriate starting point to position the bubble stack. */ - public static Point getStartPoint(int size, Point displaySize) { + private static Point getStartPoint(int size, Point displaySize) { final int x = displaySize.x - size + EDGE_OVERLAP; final int y = displaySize.y / 4; return new Point(x, y); @@ -326,7 +324,7 @@ public class BubbleController { /** * Gets an appropriate position for the bubble when the stack is expanded. */ - public static Point getExpandPoint(BubbleStackView view, int size, Point displaySize) { + static Point getExpandPoint(BubbleStackView view, int size, Point displaySize) { // Same place for now.. return new Point(EDGE_OVERLAP, size); } @@ -334,7 +332,7 @@ public class BubbleController { /** * Whether the notification should bubble or not. */ - public static boolean shouldAutoBubble(Context context, NotificationData.Entry entry) { + private static boolean shouldAutoBubble(Context context, NotificationData.Entry entry) { if (entry.isBubbleDismissed()) { return false; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java index ec388db060e24..5d6f60eadf1fd 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java @@ -15,8 +15,6 @@ */ package com.android.systemui.statusbar.notification; -import static com.android.systemui.bubbles.BubbleController.DEBUG_DEMOTE_TO_NOTIF; - import android.annotation.Nullable; import android.app.Notification; import android.content.Context; @@ -33,7 +31,6 @@ import com.android.internal.statusbar.NotificationVisibility; import com.android.systemui.Dependency; import com.android.systemui.Dumpable; import com.android.systemui.ForegroundServiceController; -import com.android.systemui.bubbles.BubbleController; import com.android.systemui.statusbar.NotificationLifetimeExtender; import com.android.systemui.statusbar.NotificationPresenter; import com.android.systemui.statusbar.NotificationRemoteInputManager; @@ -64,8 +61,7 @@ public class NotificationEntryManager implements Dumpable, NotificationInflater.InflationCallback, NotificationUpdateHandler, - VisualStabilityManager.Callback, - BubbleController.BubbleDismissListener { + VisualStabilityManager.Callback { private static final String TAG = "NotificationEntryMgr"; protected static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); @@ -80,7 +76,6 @@ public class NotificationEntryManager implements Dependency.get(DeviceProvisionedController.class); private final ForegroundServiceController mForegroundServiceController = Dependency.get(ForegroundServiceController.class); - private final BubbleController mBubbleController = Dependency.get(BubbleController.class); // Lazily retrieved dependencies private NotificationRemoteInputManager mRemoteInputManager; @@ -126,7 +121,6 @@ public class NotificationEntryManager implements public NotificationEntryManager(Context context) { mContext = context; - mBubbleController.setDismissListener(this /* bubbleEventListener */); mNotificationData = new NotificationData(); mDeferredNotificationViewUpdateHandler = new Handler(); } @@ -209,23 +203,6 @@ public class NotificationEntryManager implements n.getKey(), null, nv, false /* forceRemove */, true /* removedByUser */); } - @Override - public void onStackDismissed() { - updateNotifications(); - } - - @Override - public void onBubbleDismissed(String key) { - NotificationData.Entry entry = mNotificationData.get(key); - if (entry != null) { - entry.setBubbleDismissed(true); - if (!DEBUG_DEMOTE_TO_NOTIF) { - performRemoveNotification(entry.notification); - } - } - updateNotifications(); - } - private void abortExistingInflation(String key) { if (mPendingNotifications.containsKey(key)) { NotificationData.Entry entry = mPendingNotifications.get(key); @@ -416,9 +393,6 @@ public class NotificationEntryManager implements rankingMap.getRanking(key, ranking); NotificationData.Entry entry = new NotificationData.Entry(notification, ranking); - if (BubbleController.shouldAutoBubble(getContext(), entry)) { - entry.setIsBubble(true); - } Dependency.get(LeakDetector.class).trackInstance(entry); // Construct the expanded view. 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 8f2b2d065f72d..31df4a3fc7666 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/bubbles/BubbleControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/bubbles/BubbleControllerTest.java @@ -18,6 +18,10 @@ package com.android.systemui.bubbles; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.app.IActivityManager; import android.content.Context; @@ -29,6 +33,9 @@ import android.widget.FrameLayout; import com.android.systemui.SysuiTestCase; import com.android.systemui.statusbar.NotificationTestHelper; +import com.android.systemui.statusbar.notification.NotificationData; +import com.android.systemui.statusbar.notification.NotificationEntryListener; +import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; import com.android.systemui.statusbar.phone.DozeParameters; import com.android.systemui.statusbar.phone.StatusBarWindowController; @@ -36,6 +43,8 @@ import com.android.systemui.statusbar.phone.StatusBarWindowController; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -44,6 +53,8 @@ import org.mockito.MockitoAnnotations; @TestableLooper.RunWithLooper(setAsMainLooper = true) public class BubbleControllerTest extends SysuiTestCase { + @Mock + private NotificationEntryManager mNotificationEntryManager; @Mock private WindowManager mWindowManager; @Mock @@ -52,17 +63,23 @@ public class BubbleControllerTest extends SysuiTestCase { private DozeParameters mDozeParameters; @Mock private FrameLayout mStatusBarView; + @Captor + private ArgumentCaptor mEntryListenerCaptor; private TestableBubbleController mBubbleController; private StatusBarWindowController mStatusBarWindowController; + private NotificationEntryListener mEntryListener; private NotificationTestHelper mNotificationTestHelper; private ExpandableNotificationRow mRow; private ExpandableNotificationRow mRow2; + private final NotificationData mNotificationData = new NotificationData(); + @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); + mDependency.injectTestDependency(NotificationEntryManager.class, mNotificationEntryManager); // Bubbles get added to status bar window view mStatusBarWindowController = new StatusBarWindowController(mContext, mWindowManager, @@ -74,7 +91,15 @@ public class BubbleControllerTest extends SysuiTestCase { mRow = mNotificationTestHelper.createBubble(); mRow2 = mNotificationTestHelper.createBubble(); + // Return non-null notification data from the NEM + when(mNotificationEntryManager.getNotificationData()).thenReturn(mNotificationData); + mBubbleController = new TestableBubbleController(mContext, mStatusBarWindowController); + + // Get a reference to the BubbleController's entry listener + verify(mNotificationEntryManager, atLeastOnce()) + .addNotificationEntryListener(mEntryListenerCaptor.capture()); + mEntryListener = mEntryListenerCaptor.getValue(); } @Test @@ -102,6 +127,8 @@ public class BubbleControllerTest extends SysuiTestCase { mBubbleController.removeBubble(mRow.getEntry().key); assertFalse(mStatusBarWindowController.getBubblesShowing()); + assertTrue(mRow.getEntry().isBubbleDismissed()); + verify(mNotificationEntryManager).updateNotifications(); } @Test @@ -112,6 +139,7 @@ public class BubbleControllerTest extends SysuiTestCase { mBubbleController.dismissStack(); assertFalse(mStatusBarWindowController.getBubblesShowing()); + verify(mNotificationEntryManager, times(3)).updateNotifications(); } @Test @@ -140,6 +168,12 @@ public class BubbleControllerTest extends SysuiTestCase { assertFalse(mBubbleController.isStackExpanded()); } + @Test + public void testMarkNewNotificationAsBubble() { + mEntryListener.onPendingEntryAdded(mRow.getEntry()); + assertTrue(mRow.getEntry().isBubble()); + } + static class TestableBubbleController extends BubbleController { TestableBubbleController(Context context, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java index 8cf4b05b63710..e716421a1e693 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java @@ -77,7 +77,7 @@ public class NotificationViewHierarchyManagerTest extends SysuiTestCase { @Mock private ShadeController mShadeController; private NotificationViewHierarchyManager mViewHierarchyManager; - private NotificationTestHelper mHelper = new NotificationTestHelper(mContext); + private NotificationTestHelper mHelper; @Before public void setUp() { @@ -90,6 +90,8 @@ public class NotificationViewHierarchyManagerTest extends SysuiTestCase { mDependency.injectTestDependency(VisualStabilityManager.class, mVisualStabilityManager); mDependency.injectTestDependency(ShadeController.class, mShadeController); + mHelper = new NotificationTestHelper(mContext); + when(mEntryManager.getNotificationData()).thenReturn(mNotificationData); mViewHierarchyManager = new NotificationViewHierarchyManager(mContext,