Merge "Optimistically dismiss children of dismissed groups"
diff --git a/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java b/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java
index eab9706..924d16d 100644
--- a/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java
+++ b/packages/SystemUI/src/com/android/systemui/ForegroundServiceNotificationListener.java
@@ -89,7 +89,7 @@
}
@Override
- public void onEntryRemoved(NotificationEntry entry, int reason, boolean removedByUser) {
+ public void onEntryRemoved(NotificationEntry entry, int reason) {
removeNotification(entry.getSbn());
}
});
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListDumper.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListDumper.java
index eaa9d78..7fe229c 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListDumper.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListDumper.java
@@ -16,6 +16,9 @@
package com.android.systemui.statusbar.notification.collection;
+import static com.android.systemui.statusbar.notification.collection.NotifCollection.REASON_NOT_CANCELED;
+import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.NOT_DISMISSED;
+
import java.util.Arrays;
import java.util.List;
@@ -132,8 +135,20 @@
.append(" ");
}
+ if (notifEntry.mCancellationReason != REASON_NOT_CANCELED) {
+ rksb.append("cancellationReason=")
+ .append(notifEntry.mCancellationReason)
+ .append(" ");
+ }
+
if (notifEntry.hasInflationError()) {
- rksb.append("hasInflationError ");
+ rksb.append("(!)hasInflationError ");
+ }
+
+ if (notifEntry.getDismissState() != NOT_DISMISSED) {
+ rksb.append("dismissState=")
+ .append(notifEntry.getDismissState())
+ .append(" ");
}
String rkString = rksb.toString();
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java
index 1b61703..3b2fe94 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java
@@ -35,9 +35,14 @@
import static android.service.notification.NotificationListenerService.REASON_UNAUTOBUNDLED;
import static android.service.notification.NotificationListenerService.REASON_USER_STOPPED;
+import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.DISMISSED;
+import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.NOT_DISMISSED;
+import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.PARENT_DISMISSED;
+
+import static java.util.Objects.requireNonNull;
+
import android.annotation.IntDef;
import android.annotation.MainThread;
-import android.annotation.NonNull;
import android.annotation.Nullable;
import android.os.RemoteException;
import android.service.notification.NotificationListenerService.Ranking;
@@ -45,6 +50,8 @@
import android.service.notification.StatusBarNotification;
import android.util.ArrayMap;
+import androidx.annotation.NonNull;
+
import com.android.internal.statusbar.IStatusBarService;
import com.android.systemui.DumpController;
import com.android.systemui.Dumpable;
@@ -172,15 +179,65 @@
/**
* Dismiss a notification on behalf of the user.
*/
- void dismissNotification(
- NotificationEntry entry,
- @CancellationReason int reason,
- @NonNull DismissedByUserStats stats) {
+ void dismissNotification(NotificationEntry entry, @NonNull DismissedByUserStats stats) {
Assert.isMainThread();
- Objects.requireNonNull(stats);
+ requireNonNull(stats);
checkForReentrantCall();
- removeNotification(entry.getKey(), null, reason, stats);
+ if (entry != mNotificationSet.get(entry.getKey())) {
+ throw new IllegalStateException("Invalid entry: " + entry.getKey());
+ }
+
+ if (entry.getDismissState() == DISMISSED) {
+ return;
+ }
+
+ // Optimistically mark the notification as dismissed -- we'll wait for the signal from
+ // system server before removing it from our notification set.
+ entry.setDismissState(DISMISSED);
+ mLogger.logNotifDismissed(entry.getKey());
+
+ List<NotificationEntry> canceledEntries = new ArrayList<>();
+
+ if (isCanceled(entry)) {
+ canceledEntries.add(entry);
+ } else {
+ // Ask system server to remove it for us
+ try {
+ mStatusBarService.onNotificationClear(
+ entry.getSbn().getPackageName(),
+ entry.getSbn().getTag(),
+ entry.getSbn().getId(),
+ entry.getSbn().getUser().getIdentifier(),
+ entry.getSbn().getKey(),
+ stats.dismissalSurface,
+ stats.dismissalSentiment,
+ stats.notificationVisibility);
+ } catch (RemoteException e) {
+ // system process is dead if we're here.
+ }
+
+ // Also mark any children as dismissed as system server will auto-dismiss them as well
+ if (entry.getSbn().getNotification().isGroupSummary()) {
+ for (NotificationEntry otherEntry : mNotificationSet.values()) {
+ if (otherEntry.getSbn().getGroupKey().equals(entry.getSbn().getGroupKey())
+ && otherEntry.getDismissState() != DISMISSED) {
+ otherEntry.setDismissState(PARENT_DISMISSED);
+ if (isCanceled(otherEntry)) {
+ canceledEntries.add(otherEntry);
+ }
+ }
+ }
+ }
+ }
+
+ // Immediately remove any dismissed notifs that have already been canceled by system server
+ // (probably due to being lifetime-extended up until this point).
+ for (NotificationEntry canceledEntry : canceledEntries) {
+ tryRemoveNotification(canceledEntry);
+ }
+
+ rebuildList();
}
private void onNotificationPosted(StatusBarNotification sbn, RankingMap rankingMap) {
@@ -208,7 +265,15 @@
Assert.isMainThread();
mLogger.logNotifRemoved(sbn.getKey(), reason);
- removeNotification(sbn.getKey(), rankingMap, reason, null);
+
+ final NotificationEntry entry = mNotificationSet.get(sbn.getKey());
+ if (entry == null) {
+ throw new IllegalStateException("No notification to remove with key " + sbn.getKey());
+ }
+ entry.mCancellationReason = reason;
+ applyRanking(rankingMap);
+ tryRemoveNotification(entry);
+ rebuildList();
}
private void onNotificationRankingUpdate(RankingMap rankingMap) {
@@ -242,9 +307,12 @@
// Update to an existing entry
mLogger.logNotifUpdated(sbn.getKey());
- // Notification is updated so it is essentially re-added and thus alive again. Don't
+ cancelLocalDismissal(entry);
+
+ // Notification is updated so it is essentially re-added and thus alive again. Don't
// need to keep its lifetime extended.
cancelLifetimeExtension(entry);
+ entry.mCancellationReason = REASON_NOT_CANCELED;
entry.setSbn(sbn);
if (rankingMap != null) {
@@ -255,59 +323,42 @@
}
}
- private void removeNotification(
- String key,
- @Nullable RankingMap rankingMap,
- @CancellationReason int reason,
- @Nullable DismissedByUserStats dismissedByUserStats) {
-
- NotificationEntry entry = mNotificationSet.get(key);
- if (entry == null) {
- throw new IllegalStateException("No notification to remove with key " + key);
+ /**
+ * Tries to remove a notification from the notification set. This removal may be blocked by
+ * lifetime extenders. Does not trigger a rebuild of the list; caller must do that manually.
+ *
+ * @return True if the notification was removed, false otherwise.
+ */
+ private boolean tryRemoveNotification(NotificationEntry entry) {
+ if (mNotificationSet.get(entry.getKey()) != entry) {
+ throw new IllegalStateException("No notification to remove with key " + entry.getKey());
}
- entry.mLifetimeExtenders.clear();
- mAmDispatchingToOtherCode = true;
- for (NotifLifetimeExtender extender : mLifetimeExtenders) {
- if (extender.shouldExtendLifetime(entry, reason)) {
- entry.mLifetimeExtenders.add(extender);
- }
+ if (!isCanceled(entry)) {
+ throw new IllegalStateException("Cannot remove notification " + entry.getKey()
+ + ": has not been marked for removal");
}
- mAmDispatchingToOtherCode = false;
+
+ if (isDismissedByUser(entry)) {
+ // User-dismissed notifications cannot be lifetime-extended
+ cancelLifetimeExtension(entry);
+ } else {
+ updateLifetimeExtension(entry);
+ }
if (!isLifetimeExtended(entry)) {
mNotificationSet.remove(entry.getKey());
-
- if (dismissedByUserStats != null) {
- try {
- mStatusBarService.onNotificationClear(
- entry.getSbn().getPackageName(),
- entry.getSbn().getTag(),
- entry.getSbn().getId(),
- entry.getSbn().getUser().getIdentifier(),
- entry.getSbn().getKey(),
- dismissedByUserStats.dismissalSurface,
- dismissedByUserStats.dismissalSentiment,
- dismissedByUserStats.notificationVisibility);
- } catch (RemoteException e) {
- // system process is dead if we're here.
- }
- }
-
- if (rankingMap != null) {
- applyRanking(rankingMap);
- }
-
- dispatchOnEntryRemoved(entry, reason, dismissedByUserStats != null /* removedByUser */);
+ dispatchOnEntryRemoved(entry, entry.mCancellationReason);
dispatchOnEntryCleanUp(entry);
+ return true;
+ } else {
+ return false;
}
-
- rebuildList();
}
private void applyRanking(@NonNull RankingMap rankingMap) {
for (NotificationEntry entry : mNotificationSet.values()) {
- if (!isLifetimeExtended(entry)) {
+ if (!isCanceled(entry)) {
// TODO: (b/148791039) We should crash if we are ever handed a ranking with
// incomplete entries. Right now, there's a race condition in NotificationListener
@@ -355,9 +406,9 @@
}
if (!isLifetimeExtended(entry)) {
- // TODO: This doesn't need to be undefined -- we can set either EXTENDER_EXPIRED or
- // save the original reason
- removeNotification(entry.getKey(), null, REASON_UNKNOWN, null);
+ if (tryRemoveNotification(entry)) {
+ rebuildList();
+ }
}
}
@@ -374,6 +425,31 @@
return entry.mLifetimeExtenders.size() > 0;
}
+ private void updateLifetimeExtension(NotificationEntry entry) {
+ entry.mLifetimeExtenders.clear();
+ mAmDispatchingToOtherCode = true;
+ for (NotifLifetimeExtender extender : mLifetimeExtenders) {
+ if (extender.shouldExtendLifetime(entry, entry.mCancellationReason)) {
+ entry.mLifetimeExtenders.add(extender);
+ }
+ }
+ mAmDispatchingToOtherCode = false;
+ }
+
+ private void cancelLocalDismissal(NotificationEntry entry) {
+ if (isDismissedByUser(entry)) {
+ entry.setDismissState(NOT_DISMISSED);
+ if (entry.getSbn().getNotification().isGroupSummary()) {
+ for (NotificationEntry otherEntry : mNotificationSet.values()) {
+ if (otherEntry.getSbn().getGroupKey().equals(entry.getSbn().getGroupKey())
+ && otherEntry.getDismissState() == PARENT_DISMISSED) {
+ otherEntry.setDismissState(NOT_DISMISSED);
+ }
+ }
+ }
+ }
+ }
+
private void checkForReentrantCall() {
if (mAmDispatchingToOtherCode) {
throw new IllegalStateException("Reentrant call detected");
@@ -389,6 +465,19 @@
return ranking;
}
+ /**
+ * True if the notification has been canceled by system server. Usually, such notifications are
+ * immediately removed from the collection, but can sometimes stick around due to lifetime
+ * extenders.
+ */
+ private static boolean isCanceled(NotificationEntry entry) {
+ return entry.mCancellationReason != REASON_NOT_CANCELED;
+ }
+
+ private static boolean isDismissedByUser(NotificationEntry entry) {
+ return entry.getDismissState() != NOT_DISMISSED;
+ }
+
private void dispatchOnEntryInit(NotificationEntry entry) {
mAmDispatchingToOtherCode = true;
for (NotifCollectionListener listener : mNotifCollectionListeners) {
@@ -421,13 +510,10 @@
mAmDispatchingToOtherCode = false;
}
- private void dispatchOnEntryRemoved(
- NotificationEntry entry,
- @CancellationReason int reason,
- boolean removedByUser) {
+ private void dispatchOnEntryRemoved(NotificationEntry entry, @CancellationReason int reason) {
mAmDispatchingToOtherCode = true;
for (NotifCollectionListener listener : mNotifCollectionListeners) {
- listener.onEntryRemoved(entry, reason, removedByUser);
+ listener.onEntryRemoved(entry, reason);
}
mAmDispatchingToOtherCode = false;
}
@@ -439,6 +525,20 @@
}
mAmDispatchingToOtherCode = false;
}
+ @Override
+ public void dump(@NonNull FileDescriptor fd, PrintWriter pw, @NonNull String[] args) {
+ final List<NotificationEntry> entries = new ArrayList<>(getActiveNotifs());
+
+ pw.println("\t" + TAG + " unsorted/unfiltered notifications:");
+ if (entries.size() == 0) {
+ pw.println("\t\t None");
+ }
+ pw.println(
+ ListDumper.dumpList(
+ entries,
+ true,
+ "\t\t"));
+ }
private final BatchableNotificationHandler mNotifHandler = new BatchableNotificationHandler() {
@Override
@@ -494,20 +594,6 @@
@Retention(RetentionPolicy.SOURCE)
public @interface CancellationReason {}
+ public static final int REASON_NOT_CANCELED = -1;
public static final int REASON_UNKNOWN = 0;
-
- @Override
- public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
- final List<NotificationEntry> entries = new ArrayList<>(getActiveNotifs());
-
- pw.println("\t" + TAG + " unsorted/unfiltered notifications:");
- if (entries.size() == 0) {
- pw.println("\t\t None");
- }
- pw.println(
- ListDumper.dumpList(
- entries,
- true,
- "\t\t"));
- }
}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java
index e7b772f..7a6d4f1 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifInflaterImpl.java
@@ -98,13 +98,12 @@
@Override
public void run() {
int dismissalSurface = NotificationStats.DISMISSAL_SHADE;
- /**
+ /*
* TODO: determine dismissal surface (ie: shade / headsup / aod)
* see {@link NotificationLogger#logNotificationClear}
*/
mNotifCollection.dismissNotification(
entry,
- 0,
new DismissedByUserStats(
dismissalSurface,
DISMISS_SENTIMENT_NEUTRAL,
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 1f77ec2..a95668b 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
@@ -29,9 +29,11 @@
import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_PEEK;
import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_STATUS_BAR;
+import static com.android.systemui.statusbar.notification.collection.NotifCollection.REASON_NOT_CANCELED;
import static com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_ALERTING;
-import android.annotation.NonNull;
+import static java.util.Objects.requireNonNull;
+
import android.app.Notification;
import android.app.Notification.MessagingStyle.Message;
import android.app.NotificationChannel;
@@ -50,6 +52,7 @@
import android.view.View;
import android.widget.ImageView;
+import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import com.android.internal.annotations.VisibleForTesting;
@@ -59,6 +62,7 @@
import com.android.systemui.statusbar.InflationTask;
import com.android.systemui.statusbar.StatusBarIconView;
import com.android.systemui.statusbar.notification.InflationException;
+import com.android.systemui.statusbar.notification.collection.NotifCollection.CancellationReason;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifPromoter;
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifLifetimeExtender;
@@ -105,9 +109,18 @@
/** If this was a group child that was promoted to the top level, then who did the promoting. */
@Nullable NotifPromoter mNotifPromoter;
- /** If this notification had an issue with inflating. Only used with the NewNotifPipeline **/
+ /**
+ * If this notification was cancelled by system server, then the reason that was supplied.
+ * Uncancelled notifications always have REASON_NOT_CANCELED. Note that lifetime-extended
+ * notifications will have this set even though they are still in the active notification set.
+ */
+ @CancellationReason int mCancellationReason = REASON_NOT_CANCELED;
+
+ /** @see #hasInflationError() */
private boolean mHasInflationError;
+ /** @see #getDismissState() */
+ @NonNull private DismissState mDismissState = DismissState.NOT_DISMISSED;
/*
* Old members
@@ -169,9 +182,9 @@
public NotificationEntry(
@NonNull StatusBarNotification sbn,
@NonNull Ranking ranking) {
- super(Objects.requireNonNull(Objects.requireNonNull(sbn).getKey()));
+ super(requireNonNull(requireNonNull(sbn).getKey()));
- Objects.requireNonNull(ranking);
+ requireNonNull(ranking);
mKey = sbn.getKey();
setSbn(sbn);
@@ -201,8 +214,8 @@
* TODO: Make this package-private
*/
public void setSbn(@NonNull StatusBarNotification sbn) {
- Objects.requireNonNull(sbn);
- Objects.requireNonNull(sbn.getKey());
+ requireNonNull(sbn);
+ requireNonNull(sbn.getKey());
if (!sbn.getKey().equals(mKey)) {
throw new IllegalArgumentException("New key " + sbn.getKey()
@@ -227,8 +240,8 @@
* TODO: Make this package-private
*/
public void setRanking(@NonNull Ranking ranking) {
- Objects.requireNonNull(ranking);
- Objects.requireNonNull(ranking.getKey());
+ requireNonNull(ranking);
+ requireNonNull(ranking.getKey());
if (!ranking.getKey().equals(mKey)) {
throw new IllegalArgumentException("New key " + ranking.getKey()
@@ -239,6 +252,34 @@
}
/*
+ * Bookkeeping getters and setters
+ */
+
+ /**
+ * Whether this notification had an error when attempting to inflate. This is only used in
+ * the NewNotifPipeline
+ */
+ public boolean hasInflationError() {
+ return mHasInflationError;
+ }
+
+ void setHasInflationError(boolean hasError) {
+ mHasInflationError = hasError;
+ }
+
+ /**
+ * Set if the user has dismissed this notif but we haven't yet heard back from system server to
+ * confirm the dismissal.
+ */
+ @NonNull public DismissState getDismissState() {
+ return mDismissState;
+ }
+
+ void setDismissState(@NonNull DismissState dismissState) {
+ mDismissState = requireNonNull(dismissState);
+ }
+
+ /*
* Convenience getters for SBN and Ranking members
*/
@@ -275,7 +316,6 @@
return mRanking.canBubble();
}
-
public @NonNull List<Notification.Action> getSmartActions() {
return mRanking.getSmartActions();
}
@@ -578,18 +618,6 @@
remoteInputTextWhenReset = null;
}
- void setHasInflationError(boolean hasError) {
- mHasInflationError = hasError;
- }
-
- /**
- * Whether this notification had an error when attempting to inflate. This is only used in
- * the NewNotifPipeline
- */
- public boolean hasInflationError() {
- return mHasInflationError;
- }
-
public void setHasSentReply() {
hasSentReply = true;
}
@@ -974,6 +1002,16 @@
}
}
+ /** @see #getDismissState() */
+ public enum DismissState {
+ /** User has not dismissed this notif or its parent */
+ NOT_DISMISSED,
+ /** User has dismissed this notif specifically */
+ DISMISSED,
+ /** User has dismissed this notif's parent (which implicitly dismisses this one as well) */
+ PARENT_DISMISSED,
+ }
+
private static final long LAUNCH_COOLDOWN = 2000;
private static final long REMOTE_INPUT_COOLDOWN = 500;
private static final long INITIALIZATION_DELAY = 400;
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HideLocallyDismissedNotifsCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HideLocallyDismissedNotifsCoordinator.java
new file mode 100644
index 0000000..0059e7b
--- /dev/null
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HideLocallyDismissedNotifsCoordinator.java
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2020 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.notification.collection.coordinator;
+
+import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.NOT_DISMISSED;
+
+import com.android.systemui.statusbar.notification.collection.NotifPipeline;
+import com.android.systemui.statusbar.notification.collection.NotificationEntry;
+import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter;
+
+/**
+ * Filters out notifications that have been dismissed locally (by the user) but that system server
+ * hasn't yet confirmed the removal of.
+ */
+public class HideLocallyDismissedNotifsCoordinator implements Coordinator {
+ @Override
+ public void attach(NotifPipeline pipeline) {
+ pipeline.addPreGroupFilter(mFilter);
+ }
+
+ private final NotifFilter mFilter = new NotifFilter("HideLocallyDismissedNotifsFilter") {
+ @Override
+ public boolean shouldFilterOut(NotificationEntry entry, long now) {
+ return entry.getDismissState() != NOT_DISMISSED;
+ }
+ };
+}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.java
index 8d0dd5b..0a1e09f 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.java
@@ -56,6 +56,7 @@
PreparationCoordinator preparationCoordinator) {
dumpController.registerDumpable(TAG, this);
+ mCoordinators.add(new HideLocallyDismissedNotifsCoordinator());
mCoordinators.add(keyguardCoordinator);
mCoordinators.add(rankingCoordinator);
mCoordinators.add(foregroundCoordinator);
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java
index 20c9cbc..41314b8 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java
@@ -72,7 +72,7 @@
}
@Override
- public void onEntryRemoved(NotificationEntry entry, int reason, boolean removedByUser) {
+ public void onEntryRemoved(NotificationEntry entry, int reason) {
abortInflation(entry, "entryRemoved reason=" + reason);
}
};
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionListener.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionListener.java
index ff6da12..b2c53da 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionListener.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionListener.java
@@ -54,10 +54,7 @@
* immediately after a user dismisses a notification: we wait until we receive confirmation from
* system server before considering the notification removed.
*/
- default void onEntryRemoved(
- NotificationEntry entry,
- @CancellationReason int reason,
- boolean removedByUser) {
+ default void onEntryRemoved(NotificationEntry entry, @CancellationReason int reason) {
}
/**
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionLogger.kt
index 0d0a46a..14e1503 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionLogger.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/notifcollection/NotifCollectionLogger.kt
@@ -61,6 +61,14 @@
})
}
+ fun logNotifDismissed(key: String) {
+ buffer.log(TAG, INFO, {
+ str1 = key
+ }, {
+ "DISMISSED $str1"
+ })
+ }
+
fun logRankingMissing(key: String, rankingMap: RankingMap) {
buffer.log(TAG, WARNING, { str1 = key }, { "Ranking update is missing ranking for $str1" })
buffer.log(TAG, DEBUG, {}, { "Ranking map contents:" })
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java
index b652980..7c94ed20 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java
@@ -17,9 +17,16 @@
package com.android.systemui.statusbar.notification.collection;
import static android.service.notification.NotificationListenerService.REASON_APP_CANCEL;
+import static android.service.notification.NotificationListenerService.REASON_CANCEL;
import static android.service.notification.NotificationListenerService.REASON_CLICK;
+import static android.service.notification.NotificationStats.DISMISSAL_SHADE;
+import static android.service.notification.NotificationStats.DISMISS_SENTIMENT_NEUTRAL;
+import static com.android.systemui.statusbar.notification.collection.NotifCollection.REASON_NOT_CANCELED;
import static com.android.systemui.statusbar.notification.collection.NotifCollection.REASON_UNKNOWN;
+import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.DISMISSED;
+import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.NOT_DISMISSED;
+import static com.android.systemui.statusbar.notification.collection.NotificationEntry.DismissState.PARENT_DISMISSED;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -40,7 +47,6 @@
import android.annotation.Nullable;
import android.os.RemoteException;
import android.service.notification.NotificationListenerService.Ranking;
-import android.service.notification.NotificationStats;
import android.testing.AndroidTestingRunner;
import android.testing.TestableLooper;
import android.util.ArrayMap;
@@ -77,6 +83,7 @@
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
@@ -236,7 +243,7 @@
mNoMan.retractNotif(notif.sbn, REASON_APP_CANCEL);
// THEN the listener is notified
- verify(mCollectionListener).onEntryRemoved(entry, REASON_APP_CANCEL, false);
+ verify(mCollectionListener).onEntryRemoved(entry, REASON_APP_CANCEL);
verify(mCollectionListener).onEntryCleanUp(entry);
assertEquals(notif.sbn, entry.getSbn());
assertEquals(notif.ranking, entry.getRanking());
@@ -322,24 +329,15 @@
}
@Test
- public void testDismissNotification() throws RemoteException {
- // GIVEN a collection with a couple notifications and a lifetime extender
- mCollection.addNotificationLifetimeExtender(mExtender1);
-
+ public void testDismissNotificationSentToSystemServer() throws RemoteException {
+ // GIVEN a collection with a couple notifications
NotifEvent notif1 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 47, "myTag"));
NotifEvent notif2 = mNoMan.postNotif(buildNotif(TEST_PACKAGE2, 88, "barTag"));
NotificationEntry entry2 = mCollectionListener.getEntry(notif2.key);
// WHEN a notification is manually dismissed
- DismissedByUserStats stats = new DismissedByUserStats(
- NotificationStats.DISMISSAL_SHADE,
- NotificationStats.DISMISS_SENTIMENT_NEUTRAL,
- NotificationVisibility.obtain(entry2.getKey(), 7, 2, true));
-
- mCollection.dismissNotification(entry2, REASON_CLICK, stats);
-
- // THEN we check for lifetime extension
- verify(mExtender1).shouldExtendLifetime(entry2, REASON_CLICK);
+ DismissedByUserStats stats = defaultStats(entry2);
+ mCollection.dismissNotification(entry2, defaultStats(entry2));
// THEN we send the dismissal to system server
verify(mStatusBarService).onNotificationClear(
@@ -351,9 +349,96 @@
stats.dismissalSurface,
stats.dismissalSentiment,
stats.notificationVisibility);
+ }
- // THEN we fire a remove event
- verify(mCollectionListener).onEntryRemoved(entry2, REASON_CLICK, true);
+ @Test
+ public void testDismissedNotificationsAreMarkedAsDismissedLocally() {
+ // GIVEN a collection with a notification
+ NotifEvent notif1 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 47, "myTag"));
+ NotificationEntry entry1 = mCollectionListener.getEntry(notif1.key);
+
+ // WHEN a notification is manually dismissed
+ mCollection.dismissNotification(entry1, defaultStats(entry1));
+
+ // THEN the entry is marked as dismissed locally
+ assertEquals(DISMISSED, entry1.getDismissState());
+ }
+
+ @Test
+ public void testDismissedNotificationsCannotBeLifetimeExtended() {
+ // GIVEN a collection with a notification and a lifetime extender
+ mCollection.addNotificationLifetimeExtender(mExtender1);
+ NotifEvent notif1 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 47, "myTag"));
+ NotificationEntry entry1 = mCollectionListener.getEntry(notif1.key);
+
+ // WHEN a notification is manually dismissed
+ mCollection.dismissNotification(entry1, defaultStats(entry1));
+
+ // THEN lifetime extenders are never queried
+ verify(mExtender1, never()).shouldExtendLifetime(eq(entry1), anyInt());
+ }
+
+ @Test
+ public void testDismissedNotificationsDoNotTriggerRemovalEvents() {
+ // GIVEN a collection with a notification
+ NotifEvent notif1 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 47, "myTag"));
+ NotificationEntry entry1 = mCollectionListener.getEntry(notif1.key);
+
+ // WHEN a notification is manually dismissed
+ mCollection.dismissNotification(entry1, defaultStats(entry1));
+
+ // THEN onEntryRemoved is not called
+ verify(mCollectionListener, never()).onEntryRemoved(eq(entry1), anyInt());
+ }
+
+ @Test
+ public void testDismissedNotificationsStillAppearInNotificationSet() {
+ // GIVEN a collection with a notification
+ NotifEvent notif1 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 47, "myTag"));
+ NotificationEntry entry1 = mCollectionListener.getEntry(notif1.key);
+
+ // WHEN a notification is manually dismissed
+ mCollection.dismissNotification(entry1, defaultStats(entry1));
+
+ // THEN the dismissed entry still appears in the notification set
+ assertEquals(
+ new ArraySet<>(Collections.singletonList(entry1)),
+ new ArraySet<>(mCollection.getActiveNotifs()));
+ }
+
+ @Test
+ public void testDismissingLifetimeExtendedSummaryDoesNotDismissChildren() {
+ // GIVEN A notif group with one summary and two children
+ mCollection.addNotificationLifetimeExtender(mExtender1);
+ NotifEvent notif1 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 1, "myTag")
+ .setGroup(mContext, GROUP_1)
+ .setGroupSummary(mContext, true));
+ NotifEvent notif2 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 2, "myTag")
+ .setGroup(mContext, GROUP_1));
+ NotifEvent notif3 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 3, "myTag")
+ .setGroup(mContext, GROUP_1));
+
+ NotificationEntry entry1 = mCollectionListener.getEntry(notif1.key);
+ NotificationEntry entry2 = mCollectionListener.getEntry(notif2.key);
+ NotificationEntry entry3 = mCollectionListener.getEntry(notif3.key);
+
+ // GIVEN that the summary and one child are retracted, but both are lifetime-extended
+ mExtender1.shouldExtendLifetime = true;
+ mNoMan.retractNotif(notif1.sbn, REASON_CANCEL);
+ mNoMan.retractNotif(notif2.sbn, REASON_CANCEL);
+ assertEquals(
+ new ArraySet<>(List.of(entry1, entry2, entry3)),
+ new ArraySet<>(mCollection.getActiveNotifs()));
+
+ // WHEN the summary is dismissed by the user
+ mCollection.dismissNotification(entry1, defaultStats(entry1));
+
+ // THEN the summary is removed, but both children stick around
+ assertEquals(
+ new ArraySet<>(List.of(entry2, entry3)),
+ new ArraySet<>(mCollection.getActiveNotifs()));
+ assertEquals(NOT_DISMISSED, entry2.getDismissState());
+ assertEquals(NOT_DISMISSED, entry3.getDismissState());
}
@Test(expected = IllegalStateException.class)
@@ -366,15 +451,115 @@
mNoMan.retractNotif(notif2.sbn, REASON_UNKNOWN);
// WHEN we try to dismiss a notification that isn't present
- mCollection.dismissNotification(
- entry2,
- REASON_CLICK,
- new DismissedByUserStats(0, 0, NotificationVisibility.obtain("foo", 47, 3, true)));
+ mCollection.dismissNotification(entry2, defaultStats(entry2));
// THEN an exception is thrown
}
@Test
+ public void testGroupChildrenAreDismissedLocallyWhenSummaryIsDismissed() {
+ // GIVEN a collection with two grouped notifs in it
+ NotifEvent notif0 = mNoMan.postNotif(
+ buildNotif(TEST_PACKAGE, 0)
+ .setGroup(mContext, GROUP_1)
+ .setGroupSummary(mContext, true));
+ NotifEvent notif1 = mNoMan.postNotif(
+ buildNotif(TEST_PACKAGE, 1)
+ .setGroup(mContext, GROUP_1));
+ NotificationEntry entry0 = mCollectionListener.getEntry(notif0.key);
+ NotificationEntry entry1 = mCollectionListener.getEntry(notif1.key);
+
+ // WHEN the summary is dismissed
+ mCollection.dismissNotification(entry0, defaultStats(entry0));
+
+ // THEN all members of the group are marked as dismissed locally
+ assertEquals(DISMISSED, entry0.getDismissState());
+ assertEquals(PARENT_DISMISSED, entry1.getDismissState());
+ }
+
+ @Test
+ public void testUpdatingDismissedSummaryBringsChildrenBack() {
+ // GIVEN a collection with two grouped notifs in it
+ NotifEvent notif0 = mNoMan.postNotif(
+ buildNotif(TEST_PACKAGE, 0)
+ .setGroup(mContext, GROUP_1)
+ .setGroupSummary(mContext, true));
+ NotifEvent notif1 = mNoMan.postNotif(
+ buildNotif(TEST_PACKAGE, 1)
+ .setGroup(mContext, GROUP_1));
+ NotificationEntry entry0 = mCollectionListener.getEntry(notif0.key);
+ NotificationEntry entry1 = mCollectionListener.getEntry(notif1.key);
+
+ // WHEN the summary is dismissed but then reposted without a group
+ mCollection.dismissNotification(entry0, defaultStats(entry0));
+ NotifEvent notif0a = mNoMan.postNotif(
+ buildNotif(TEST_PACKAGE, 0));
+
+ // THEN it and all of its previous children are no longer dismissed locally
+ assertEquals(NOT_DISMISSED, entry0.getDismissState());
+ assertEquals(NOT_DISMISSED, entry1.getDismissState());
+ }
+
+ @Test
+ public void testDismissedChildrenAreNotResetByParentUpdate() {
+ // GIVEN a collection with three grouped notifs in it
+ NotifEvent notif0 = mNoMan.postNotif(
+ buildNotif(TEST_PACKAGE, 0)
+ .setGroup(mContext, GROUP_1)
+ .setGroupSummary(mContext, true));
+ NotifEvent notif1 = mNoMan.postNotif(
+ buildNotif(TEST_PACKAGE, 1)
+ .setGroup(mContext, GROUP_1));
+ NotifEvent notif2 = mNoMan.postNotif(
+ buildNotif(TEST_PACKAGE, 2)
+ .setGroup(mContext, GROUP_1));
+ NotificationEntry entry0 = mCollectionListener.getEntry(notif0.key);
+ NotificationEntry entry1 = mCollectionListener.getEntry(notif1.key);
+ NotificationEntry entry2 = mCollectionListener.getEntry(notif2.key);
+
+ // WHEN a child is dismissed, then the parent is dismissed, then the parent is updated
+ mCollection.dismissNotification(entry1, defaultStats(entry1));
+ mCollection.dismissNotification(entry0, defaultStats(entry0));
+ NotifEvent notif0a = mNoMan.postNotif(
+ buildNotif(TEST_PACKAGE, 0));
+
+ // THEN the manually-dismissed child is still marked as dismissed
+ assertEquals(NOT_DISMISSED, entry0.getDismissState());
+ assertEquals(DISMISSED, entry1.getDismissState());
+ assertEquals(NOT_DISMISSED, entry2.getDismissState());
+ }
+
+ @Test
+ public void testUpdatingGroupKeyOfDismissedSummaryBringsChildrenBack() {
+ // GIVEN a collection with two grouped notifs in it
+ NotifEvent notif0 = mNoMan.postNotif(
+ buildNotif(TEST_PACKAGE, 0)
+ .setOverrideGroupKey(GROUP_1)
+ .setGroupSummary(mContext, true));
+ NotifEvent notif1 = mNoMan.postNotif(
+ buildNotif(TEST_PACKAGE, 1)
+ .setOverrideGroupKey(GROUP_1));
+ NotificationEntry entry0 = mCollectionListener.getEntry(notif0.key);
+ NotificationEntry entry1 = mCollectionListener.getEntry(notif1.key);
+
+ // WHEN the summary is dismissed but then reposted AND in the same update one of the
+ // children's ranking loses its override group
+ mCollection.dismissNotification(entry0, defaultStats(entry0));
+ mNoMan.setRanking(entry1.getKey(), new RankingBuilder()
+ .setKey(entry1.getKey())
+ .build());
+ mNoMan.postNotif(
+ buildNotif(TEST_PACKAGE, 0)
+ .setOverrideGroupKey(GROUP_1)
+ .setGroupSummary(mContext, true));
+
+ // THEN it and all of its previous children are no longer dismissed locally, including the
+ // child that is no longer part of the group
+ assertEquals(NOT_DISMISSED, entry0.getDismissState());
+ assertEquals(NOT_DISMISSED, entry1.getDismissState());
+ }
+
+ @Test
public void testLifetimeExtendersAreQueriedWhenNotifRemoved() {
// GIVEN a couple notifications and a few lifetime extenders
mExtender1.shouldExtendLifetime = true;
@@ -389,12 +574,12 @@
NotificationEntry entry2 = mCollectionListener.getEntry(notif2.key);
// WHEN a notification is removed
- mNoMan.retractNotif(notif2.sbn, REASON_UNKNOWN);
+ mNoMan.retractNotif(notif2.sbn, REASON_CLICK);
// THEN each extender is asked whether to extend, even if earlier ones return true
- verify(mExtender1).shouldExtendLifetime(entry2, REASON_UNKNOWN);
- verify(mExtender2).shouldExtendLifetime(entry2, REASON_UNKNOWN);
- verify(mExtender3).shouldExtendLifetime(entry2, REASON_UNKNOWN);
+ verify(mExtender1).shouldExtendLifetime(entry2, REASON_CLICK);
+ verify(mExtender2).shouldExtendLifetime(entry2, REASON_CLICK);
+ verify(mExtender3).shouldExtendLifetime(entry2, REASON_CLICK);
// THEN the entry is not removed
assertTrue(mCollection.getActiveNotifs().contains(entry2));
@@ -428,9 +613,9 @@
mExtender2.callback.onEndLifetimeExtension(mExtender2, entry2);
// THEN each extender is re-queried
- verify(mExtender1).shouldExtendLifetime(entry2, REASON_UNKNOWN);
- verify(mExtender2).shouldExtendLifetime(entry2, REASON_UNKNOWN);
- verify(mExtender3).shouldExtendLifetime(entry2, REASON_UNKNOWN);
+ verify(mExtender1).shouldExtendLifetime(entry2, REASON_APP_CANCEL);
+ verify(mExtender2).shouldExtendLifetime(entry2, REASON_APP_CANCEL);
+ verify(mExtender3).shouldExtendLifetime(entry2, REASON_APP_CANCEL);
// THEN the entry is not removed
assertTrue(mCollection.getActiveNotifs().contains(entry2));
@@ -466,9 +651,9 @@
assertTrue(mCollection.getActiveNotifs().contains(entry2));
// THEN we don't re-query the extenders
- verify(mExtender1, never()).shouldExtendLifetime(eq(entry2), anyInt());
- verify(mExtender2, never()).shouldExtendLifetime(eq(entry2), anyInt());
- verify(mExtender3, never()).shouldExtendLifetime(eq(entry2), anyInt());
+ verify(mExtender1, never()).shouldExtendLifetime(entry2, REASON_APP_CANCEL);
+ verify(mExtender2, never()).shouldExtendLifetime(entry2, REASON_APP_CANCEL);
+ verify(mExtender3, never()).shouldExtendLifetime(entry2, REASON_APP_CANCEL);
// THEN the entry properly records all extenders that returned true
assertEquals(Arrays.asList(mExtender1), entry2.mLifetimeExtenders);
@@ -501,7 +686,7 @@
// THEN the entry removed
assertFalse(mCollection.getActiveNotifs().contains(entry2));
- verify(mCollectionListener).onEntryRemoved(entry2, REASON_UNKNOWN, false);
+ verify(mCollectionListener).onEntryRemoved(entry2, REASON_UNKNOWN);
}
@Test
@@ -591,6 +776,36 @@
assertEquals(notif2a.ranking, entry2.getRanking());
}
+ @Test
+ public void testCancellationReasonIsSetWhenNotifIsCancelled() {
+ // GIVEN a notification
+ NotifEvent notif0 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 3));
+ NotificationEntry entry0 = mCollectionListener.getEntry(notif0.key);
+
+ // WHEN the notification is retracted
+ mNoMan.retractNotif(notif0.sbn, REASON_APP_CANCEL);
+
+ // THEN the retraction reason is stored on the notif
+ assertEquals(REASON_APP_CANCEL, entry0.mCancellationReason);
+ }
+
+ @Test
+ public void testCancellationReasonIsClearedWhenNotifIsUpdated() {
+ // GIVEN a notification and a lifetime extender that will preserve it
+ NotifEvent notif0 = mNoMan.postNotif(buildNotif(TEST_PACKAGE, 3));
+ NotificationEntry entry0 = mCollectionListener.getEntry(notif0.key);
+ mCollection.addNotificationLifetimeExtender(mExtender1);
+ mExtender1.shouldExtendLifetime = true;
+
+ // WHEN the notification is retracted and subsequently reposted
+ mNoMan.retractNotif(notif0.sbn, REASON_APP_CANCEL);
+ assertEquals(REASON_APP_CANCEL, entry0.mCancellationReason);
+ mNoMan.postNotif(buildNotif(TEST_PACKAGE, 3));
+
+ // THEN the notification has its cancellation reason cleared
+ assertEquals(REASON_NOT_CANCELED, entry0.mCancellationReason);
+ }
+
private static NotificationEntryBuilder buildNotif(String pkg, int id, String tag) {
return new NotificationEntryBuilder()
.setPkg(pkg)
@@ -604,6 +819,13 @@
.setId(id);
}
+ private static DismissedByUserStats defaultStats(NotificationEntry entry) {
+ return new DismissedByUserStats(
+ DISMISSAL_SHADE,
+ DISMISS_SENTIMENT_NEUTRAL,
+ NotificationVisibility.obtain(entry.getKey(), 7, 2, true));
+ }
+
private static class RecordingCollectionListener implements NotifCollectionListener {
private final Map<String, NotificationEntry> mLastSeenEntries = new ArrayMap<>();
@@ -621,7 +843,7 @@
}
@Override
- public void onEntryRemoved(NotificationEntry entry, int reason, boolean removedByUser) {
+ public void onEntryRemoved(NotificationEntry entry, int reason) {
}
@Override
@@ -674,4 +896,6 @@
private static final String TEST_PACKAGE = "com.android.test.collection";
private static final String TEST_PACKAGE2 = "com.android.test.collection2";
+
+ private static final String GROUP_1 = "group_1";
}