Ensure the notification is removed when bubble is removed & fix cancel all
Bubbles has some particular notification behaviour needs:
1) Whenever there is a bubble, the notif is kept around but may be hidden
from the shade
2) When the bubble is dismissed, if the notif is no longer in the
shade => really remove the notification
3) When the bubble is dismissed, if there is still a notification in the
shade => notif stays around and is normal
4) Clear all should only hide the bubble notification, not remove the
bubble
5) Apps canceling a notification that has a bubble will cancel the bubble
This CL does this by:
* Including the removal reason removeNotification path
* Adding a NotificationRemoveInterceptor that gets the option of overriding
the removal
* BubbleController has this new interceptor and uses the removal reason
to determine what should happen to the bubble & if the notification
should be allowed to be removed
* When the bubble is dismissed, if the notif is no longer in the shade,
then actually remove that notification
Test: atest BubbleControllerTest NotificationEntryManagerTest
Bug: 130347307
Bug: 130687293
Change-Id: I4459864a2ee5522076117f84ae37022bdfe4ee5d
diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
index d071363..9ecc6f4 100644
--- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
+++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
@@ -16,6 +16,10 @@
package com.android.systemui.bubbles;
+import static android.service.notification.NotificationListenerService.REASON_APP_CANCEL;
+import static android.service.notification.NotificationListenerService.REASON_APP_CANCEL_ALL;
+import static android.service.notification.NotificationListenerService.REASON_CANCEL;
+import static android.service.notification.NotificationListenerService.REASON_CANCEL_ALL;
import static android.view.Display.DEFAULT_DISPLAY;
import static android.view.Display.INVALID_DISPLAY;
import static android.view.View.INVISIBLE;
@@ -53,13 +57,13 @@
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.statusbar.IStatusBarService;
-import com.android.internal.statusbar.NotificationVisibility;
import com.android.systemui.Dependency;
import com.android.systemui.R;
import com.android.systemui.plugins.statusbar.StatusBarStateController;
import com.android.systemui.shared.system.ActivityManagerWrapper;
import com.android.systemui.shared.system.TaskStackChangeListener;
import com.android.systemui.shared.system.WindowManagerWrapper;
+import com.android.systemui.statusbar.NotificationRemoveInterceptor;
import com.android.systemui.statusbar.notification.NotificationEntryListener;
import com.android.systemui.statusbar.notification.NotificationEntryManager;
import com.android.systemui.statusbar.notification.NotificationInterruptionStateProvider;
@@ -210,6 +214,7 @@
mNotificationEntryManager = Dependency.get(NotificationEntryManager.class);
mNotificationEntryManager.addNotificationEntryListener(mEntryListener);
+ mNotificationEntryManager.setNotificationRemoveInterceptor(mRemoveInterceptor);
mStatusBarWindowController = statusBarWindowController;
mStatusBarStateListener = new StatusBarStateListener();
@@ -389,6 +394,46 @@
}
@SuppressWarnings("FieldCanBeLocal")
+ private final NotificationRemoveInterceptor mRemoveInterceptor =
+ new NotificationRemoveInterceptor() {
+ @Override
+ public boolean onNotificationRemoveRequested(String key, int reason) {
+ if (!mBubbleData.hasBubbleWithKey(key)) {
+ return false;
+ }
+ NotificationEntry entry = mBubbleData.getBubbleWithKey(key).entry;
+
+ final boolean isClearAll = reason == REASON_CANCEL_ALL;
+ final boolean isUserDimiss = reason == REASON_CANCEL;
+ final boolean isAppCancel = reason == REASON_APP_CANCEL
+ || reason == REASON_APP_CANCEL_ALL;
+
+ // Need to check for !appCancel here because the notification may have
+ // previously been dismissed & entry.isRowDismissed would still be true
+ boolean userRemovedNotif = (entry.isRowDismissed() && !isAppCancel)
+ || isClearAll || isUserDimiss;
+
+ // The bubble notification sticks around in the data as long as the bubble is
+ // not dismissed and the app hasn't cancelled the notification.
+ boolean bubbleExtended = entry.isBubble() && !entry.isBubbleDismissed()
+ && userRemovedNotif;
+ if (bubbleExtended) {
+ entry.setShowInShadeWhenBubble(false);
+ if (mStackView != null) {
+ mStackView.updateDotVisibility(entry.key);
+ }
+ mNotificationEntryManager.updateNotifications();
+ return true;
+ } else if (!userRemovedNotif && !entry.isBubbleDismissed()) {
+ // This wasn't a user removal so we should remove the bubble as well
+ mBubbleData.notificationEntryRemoved(entry, DISMISS_NOTIF_CANCEL);
+ return false;
+ }
+ return false;
+ }
+ };
+
+ @SuppressWarnings("FieldCanBeLocal")
private final NotificationEntryListener mEntryListener = new NotificationEntryListener() {
@Override
public void onPendingEntryAdded(NotificationEntry entry) {
@@ -396,7 +441,6 @@
return;
}
if (mNotificationInterruptionStateProvider.shouldBubbleUp(entry)) {
- // TODO: handle group summaries?
updateShowInShadeForSuppressNotification(entry);
}
}
@@ -426,23 +470,6 @@
updateBubble(entry);
}
}
-
- @Override
- public void onEntryRemoved(NotificationEntry entry,
- @Nullable NotificationVisibility visibility,
- boolean removedByUser) {
- if (!areBubblesEnabled(mContext)) {
- return;
- }
- entry.setShowInShadeWhenBubble(false);
- if (mStackView != null) {
- mStackView.updateDotVisibility(entry.key);
- }
- if (!removedByUser) {
- // This was a cancel so we should remove the bubble
- removeBubble(entry.key, DISMISS_NOTIF_CANCEL);
- }
- }
};
@SuppressWarnings("FieldCanBeLocal")
@@ -455,13 +482,15 @@
}
@Override
- public void onBubbleRemoved(Bubble bubble, int reason) {
+ public void onBubbleRemoved(Bubble bubble, @DismissReason int reason) {
if (mStackView != null) {
mStackView.removeBubble(bubble);
}
- if (!bubble.entry.showInShadeWhenBubble()) {
- // The notification is gone & bubble is gone, time to actually remove it
- mNotificationEntryManager.performRemoveNotification(bubble.entry.notification);
+ if (!mBubbleData.hasBubbleWithKey(bubble.getKey())
+ && !bubble.entry.showInShadeWhenBubble()) {
+ // The bubble is gone & the notification is gone, time to actually remove it
+ mNotificationEntryManager.performRemoveNotification(bubble.entry.notification,
+ 0 /* reason */);
} else {
// The notification is still in the shade but we've removed the bubble so
// lets make sure NoMan knows it's not a bubble anymore
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationListener.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationListener.java
index aeaceb0..d59a5e8 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationListener.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationListener.java
@@ -104,7 +104,7 @@
// Remove existing notification to avoid stale data.
if (isUpdate) {
- mEntryManager.removeNotification(key, rankingMap);
+ mEntryManager.removeNotification(key, rankingMap, 0 /* reason */);
} else {
mEntryManager.getNotificationData()
.updateRanking(rankingMap);
@@ -121,18 +121,23 @@
}
@Override
- public void onNotificationRemoved(StatusBarNotification sbn,
- final RankingMap rankingMap) {
- if (DEBUG) Log.d(TAG, "onNotificationRemoved: " + sbn);
+ public void onNotificationRemoved(StatusBarNotification sbn, RankingMap rankingMap,
+ int reason) {
+ if (DEBUG) Log.d(TAG, "onNotificationRemoved: " + sbn + " reason: " + reason);
if (sbn != null && !onPluginNotificationRemoved(sbn, rankingMap)) {
final String key = sbn.getKey();
Dependency.get(Dependency.MAIN_HANDLER).post(() -> {
- mEntryManager.removeNotification(key, rankingMap);
+ mEntryManager.removeNotification(key, rankingMap, reason);
});
}
}
@Override
+ public void onNotificationRemoved(StatusBarNotification sbn, RankingMap rankingMap) {
+ onNotificationRemoved(sbn, rankingMap, 0 /* reason */);
+ }
+
+ @Override
public void onNotificationRankingUpdate(final RankingMap rankingMap) {
if (DEBUG) Log.d(TAG, "onRankingUpdate");
if (rankingMap != null) {
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoveInterceptor.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoveInterceptor.java
new file mode 100644
index 0000000..930116e
--- /dev/null
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationRemoveInterceptor.java
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.statusbar;
+
+import android.service.notification.NotificationListenerService;
+
+/**
+ * Interface for anything that may need to prevent notifications from being removed. This is
+ * similar to a {@link NotificationLifetimeExtender} in the sense that it extends the life of
+ * a notification by preventing the removal, however, unlike the extender, the remove interceptor
+ * gets first pick at intercepting any type of removal -- the life time extender is unable to
+ * extend the life of a user dismissed or force removed notification.
+ */
+public interface NotificationRemoveInterceptor {
+
+ /**
+ * Called when a notification has been removed.
+ *
+ * @param key the entry key of the notification being removed.
+ * @param removeReason why the notification is being removed, e.g.
+ * {@link NotificationListenerService#REASON_CANCEL} or 0 if unknown.
+ *
+ * @return true if the removal should be ignored, false otherwise.
+ */
+ boolean onNotificationRemoveRequested(String key, int removeReason);
+}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationUpdateHandler.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationUpdateHandler.java
index 0044194..1ac8198 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationUpdateHandler.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationUpdateHandler.java
@@ -37,8 +37,10 @@
*
* @param key Key identifying the notification to remove
* @param ranking RankingMap to update with
+ * @param reason why the notification is being removed, e.g.
+ * {@link NotificationListenerService#REASON_CANCEL}.
*/
- void removeNotification(String key, NotificationListenerService.RankingMap ranking);
+ void removeNotification(String key, NotificationListenerService.RankingMap ranking, int reason);
/**
* Update a given notification and the current notification ranking map.
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 7d224fb..d926f88 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java
@@ -15,6 +15,9 @@
*/
package com.android.systemui.statusbar.notification;
+import static android.service.notification.NotificationListenerService.REASON_CANCEL;
+import static android.service.notification.NotificationListenerService.REASON_ERROR;
+
import android.annotation.Nullable;
import android.app.Notification;
import android.content.Context;
@@ -30,6 +33,7 @@
import com.android.systemui.statusbar.NotificationLifetimeExtender;
import com.android.systemui.statusbar.NotificationPresenter;
import com.android.systemui.statusbar.NotificationRemoteInputManager;
+import com.android.systemui.statusbar.NotificationRemoveInterceptor;
import com.android.systemui.statusbar.NotificationUiAdjustment;
import com.android.systemui.statusbar.NotificationUpdateHandler;
import com.android.systemui.statusbar.notification.collection.NotificationData;
@@ -82,6 +86,7 @@
final ArrayList<NotificationLifetimeExtender> mNotificationLifetimeExtenders
= new ArrayList<>();
private final List<NotificationEntryListener> mNotificationEntryListeners = new ArrayList<>();
+ private NotificationRemoveInterceptor mRemoveInterceptor;
@Override
public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
@@ -115,6 +120,11 @@
mNotificationEntryListeners.add(listener);
}
+ /** Sets the {@link NotificationRemoveInterceptor}. */
+ public void setNotificationRemoveInterceptor(NotificationRemoveInterceptor interceptor) {
+ mRemoveInterceptor = interceptor;
+ }
+
/**
* Our dependencies can have cyclic references, so some need to be lazy
*/
@@ -146,7 +156,7 @@
/** Adds a {@link NotificationLifetimeExtender}. */
public void addNotificationLifetimeExtender(NotificationLifetimeExtender extender) {
mNotificationLifetimeExtenders.add(extender);
- extender.setCallback(key -> removeNotification(key, mLatestRankingMap));
+ extender.setCallback(key -> removeNotification(key, mLatestRankingMap, 0));
}
public NotificationData getNotificationData() {
@@ -158,10 +168,18 @@
updateNotifications();
}
- public void performRemoveNotification(StatusBarNotification n) {
+ /**
+ * Requests a notification to be removed.
+ *
+ * @param n the notification to remove.
+ * @param reason why it is being removed e.g. {@link NotificationListenerService#REASON_CANCEL},
+ * or 0 if unknown.
+ */
+ public void performRemoveNotification(StatusBarNotification n, int reason) {
final NotificationVisibility nv = obtainVisibility(n.getKey());
removeNotificationInternal(
- n.getKey(), null, nv, false /* forceRemove */, true /* removedByUser */);
+ n.getKey(), null, nv, false /* forceRemove */, true /* removedByUser */,
+ reason);
}
private NotificationVisibility obtainVisibility(String key) {
@@ -193,7 +211,8 @@
@Override
public void handleInflationException(StatusBarNotification n, Exception e) {
removeNotificationInternal(
- n.getKey(), null, null, true /* forceRemove */, false /* removedByUser */);
+ n.getKey(), null, null, true /* forceRemove */, false /* removedByUser */,
+ REASON_ERROR);
for (NotificationEntryListener listener : mNotificationEntryListeners) {
listener.onInflationError(n, e);
}
@@ -228,9 +247,10 @@
}
@Override
- public void removeNotification(String key, NotificationListenerService.RankingMap ranking) {
+ public void removeNotification(String key, NotificationListenerService.RankingMap ranking,
+ int reason) {
removeNotificationInternal(key, ranking, obtainVisibility(key), false /* forceRemove */,
- false /* removedByUser */);
+ false /* removedByUser */, reason);
}
private void removeNotificationInternal(
@@ -238,7 +258,15 @@
@Nullable NotificationListenerService.RankingMap ranking,
@Nullable NotificationVisibility visibility,
boolean forceRemove,
- boolean removedByUser) {
+ boolean removedByUser,
+ int reason) {
+
+ if (mRemoveInterceptor != null
+ && mRemoveInterceptor.onNotificationRemoveRequested(key, reason)) {
+ // Remove intercepted; skip
+ return;
+ }
+
final NotificationEntry entry = mNotificationData.get(key);
abortExistingInflation(key);
@@ -342,7 +370,8 @@
Dependency.get(LeakDetector.class).trackInstance(entry);
// Construct the expanded view.
- requireBinder().inflateViews(entry, () -> performRemoveNotification(notification));
+ requireBinder().inflateViews(entry, () -> performRemoveNotification(notification,
+ REASON_CANCEL));
abortExistingInflation(key);
@@ -383,7 +412,8 @@
listener.onPreEntryUpdated(entry);
}
- requireBinder().inflateViews(entry, () -> performRemoveNotification(notification));
+ requireBinder().inflateViews(entry, () -> performRemoveNotification(notification,
+ REASON_CANCEL));
updateNotifications();
if (DEBUG) {
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java
index d89354b..a3e18ef 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java
@@ -247,7 +247,7 @@
*/
public boolean showInShadeWhenBubble() {
// We always show it in the shade if non-clearable
- return !isClearable() || mShowInShadeWhenBubble;
+ return !isRowDismissed() && (!isClearable() || mShowInShadeWhenBubble);
}
/**
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 c4ecb82..94f7e65 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
@@ -270,6 +270,8 @@
}
}
+ // TODO: This method has side effects, it is NOT just logging that a notification
+ // was cleared, it also actually removes the notification
private void logNotificationClear(String key, StatusBarNotification notification,
NotificationVisibility nv) {
final String pkg = notification.getPackageName();
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java
index ebda585..c6d6ca6 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java
@@ -46,6 +46,7 @@
import android.os.Bundle;
import android.os.ServiceManager;
import android.provider.Settings;
+import android.service.notification.NotificationListenerService;
import android.service.notification.StatusBarNotification;
import android.util.AttributeSet;
import android.util.DisplayMetrics;
@@ -5586,7 +5587,8 @@
setDismissAllInProgress(false);
for (ExpandableNotificationRow rowToRemove : viewsToRemove) {
if (canChildBeDismissed(rowToRemove)) {
- mEntryManager.removeNotification(rowToRemove.getEntry().key, null);
+ mEntryManager.removeNotification(rowToRemove.getEntry().key, null /* ranking */,
+ NotificationListenerService.REASON_CANCEL_ALL);
} else {
rowToRemove.resetTranslation();
}
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 e4af15c..e00d439 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java
@@ -16,6 +16,8 @@
package com.android.systemui.statusbar.phone;
+import static android.service.notification.NotificationListenerService.REASON_CLICK;
+
import static com.android.systemui.statusbar.phone.StatusBar.getActivityOptions;
import android.app.ActivityManager;
@@ -483,7 +485,7 @@
// We have to post it to the UI thread for synchronization
mMainThreadHandler.post(() -> {
Runnable removeRunnable =
- () -> mEntryManager.performRemoveNotification(notification);
+ () -> mEntryManager.performRemoveNotification(notification, REASON_CLICK);
if (mPresenter.isCollapsing()) {
// To avoid lags we're only performing the remove
// after the shade was collapsed