Update removal logic for notifications

New pipeline:
Cancel the lifetime extension for notifications that are manually
dismissed by the user whether this is via manually dismissal or if the
user is clicking on an autoCancellable notification

Old pipeline:
Move the logic for dismissing the summary of a notification if the
user is dismissing the last child of a notification group to the
OnUserInteraction callback.

Test: atest NotifCollectionTest
Test: manual
Fixes: 166784583
Change-Id: I55398075ad9733b73d8ff900cd465fee9a4f01d1
This commit is contained in:
Beverly
2020-08-28 17:28:07 -04:00
parent fb94cc526b
commit 5d3cdb4e4c
8 changed files with 91 additions and 60 deletions

View File

@@ -18,6 +18,7 @@ package com.android.systemui.statusbar.notification.collection;
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.service.notification.NotificationListenerService.REASON_CHANNEL_BANNED;
import static android.service.notification.NotificationListenerService.REASON_CLICK;
@@ -459,8 +460,7 @@ public class NotifCollection implements Dumpable {
+ ": has not been marked for removal"));
}
if (isDismissedByUser(entry)) {
// User-dismissed notifications cannot be lifetime-extended
if (cannotBeLifetimeExtended(entry)) {
cancelLifetimeExtension(entry);
} else {
updateLifetimeExtension(entry);
@@ -583,7 +583,7 @@ public class NotifCollection implements Dumpable {
}
private void cancelLocalDismissal(NotificationEntry entry) {
if (isDismissedByUser(entry)) {
if (entry.getDismissState() != NOT_DISMISSED) {
entry.setDismissState(NOT_DISMISSED);
if (entry.getSbn().getNotification().isGroupSummary()) {
for (NotificationEntry otherEntry : mNotificationSet.values()) {
@@ -669,12 +669,16 @@ public class NotifCollection implements Dumpable {
* immediately removed from the collection, but can sometimes stick around due to lifetime
* extenders.
*/
private static boolean isCanceled(NotificationEntry entry) {
private boolean isCanceled(NotificationEntry entry) {
return entry.mCancellationReason != REASON_NOT_CANCELED;
}
private static boolean isDismissedByUser(NotificationEntry entry) {
return entry.getDismissState() != NOT_DISMISSED;
private boolean cannotBeLifetimeExtended(NotificationEntry entry) {
final boolean locallyDismissedByUser = entry.getDismissState() != NOT_DISMISSED;
final boolean systemServerReportedUserCancel =
entry.mCancellationReason == REASON_CLICK
|| entry.mCancellationReason == REASON_CANCEL;
return locallyDismissedByUser || systemServerReportedUserCancel;
}
/**

View File

@@ -26,6 +26,7 @@ import com.android.systemui.plugins.statusbar.StatusBarStateController;
import com.android.systemui.statusbar.notification.NotificationEntryManager;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.collection.notifcollection.DismissedByUserStats;
import com.android.systemui.statusbar.notification.collection.render.GroupMembershipManager;
import com.android.systemui.statusbar.notification.logging.NotificationLogger;
import com.android.systemui.statusbar.notification.row.OnUserInteractionCallback;
import com.android.systemui.statusbar.policy.HeadsUpManager;
@@ -38,17 +39,20 @@ public class OnUserInteractionCallbackImplLegacy implements OnUserInteractionCal
private final HeadsUpManager mHeadsUpManager;
private final StatusBarStateController mStatusBarStateController;
private final VisualStabilityManager mVisualStabilityManager;
private final GroupMembershipManager mGroupMembershipManager;
public OnUserInteractionCallbackImplLegacy(
NotificationEntryManager notificationEntryManager,
HeadsUpManager headsUpManager,
StatusBarStateController statusBarStateController,
VisualStabilityManager visualStabilityManager
VisualStabilityManager visualStabilityManager,
GroupMembershipManager groupMembershipManager
) {
mNotificationEntryManager = notificationEntryManager;
mHeadsUpManager = headsUpManager;
mStatusBarStateController = statusBarStateController;
mVisualStabilityManager = visualStabilityManager;
mGroupMembershipManager = groupMembershipManager;
}
/**
@@ -69,6 +73,13 @@ public class OnUserInteractionCallbackImplLegacy implements OnUserInteractionCal
dismissalSurface = NotificationStats.DISMISSAL_AOD;
}
if (mGroupMembershipManager.isOnlyChildInGroup(entry)) {
NotificationEntry groupSummary = mGroupMembershipManager.getLogicalGroupSummary(entry);
if (groupSummary.isClearable()) {
onDismiss(groupSummary, cancellationReason);
}
}
mNotificationEntryManager.performRemoveNotification(
entry.getSbn(),
new DismissedByUserStats(
@@ -82,6 +93,7 @@ public class OnUserInteractionCallbackImplLegacy implements OnUserInteractionCal
NotificationLogger.getNotificationLocation(entry))),
cancellationReason
);
}
@Override

View File

@@ -205,9 +205,11 @@ public interface NotificationsModule {
Context context,
NotificationGutsManager notificationGutsManager,
NotificationEntryManager notificationEntryManager,
MetricsLogger metricsLogger) {
MetricsLogger metricsLogger,
GroupMembershipManager groupMembershipManager) {
return new NotificationBlockingHelperManager(
context, notificationGutsManager, notificationEntryManager, metricsLogger);
context, notificationGutsManager, notificationEntryManager, metricsLogger,
groupMembershipManager);
}
/** Provides an instance of {@link GroupMembershipManager} */
@@ -273,7 +275,8 @@ public interface NotificationsModule {
Lazy<NotifCollection> notifCollection,
Lazy<VisualStabilityCoordinator> visualStabilityCoordinator,
NotificationEntryManager entryManager,
VisualStabilityManager visualStabilityManager) {
VisualStabilityManager visualStabilityManager,
Lazy<GroupMembershipManager> groupMembershipManagerLazy) {
return featureFlags.isNewNotifPipelineRenderingEnabled()
? new OnUserInteractionCallbackImpl(
pipeline.get(),
@@ -285,7 +288,8 @@ public interface NotificationsModule {
entryManager,
headsUpManager,
statusBarStateController,
visualStabilityManager);
visualStabilityManager,
groupMembershipManagerLazy.get());
}
/** */

View File

@@ -817,13 +817,6 @@ public class ExpandableNotificationRow extends ActivatableNotificationView
return mNotificationParent != null;
}
/**
* @return whether this notification is the only child in the group summary
*/
public boolean isOnlyChildInGroup() {
return mGroupMembershipManager.isOnlyChildInGroup(mEntry);
}
public ExpandableNotificationRow getNotificationParent() {
return mNotificationParent;
}
@@ -1425,14 +1418,6 @@ public class ExpandableNotificationRow extends ActivatableNotificationView
}
public void performDismiss(boolean fromAccessibility) {
if (isOnlyChildInGroup()) {
NotificationEntry groupSummary = mGroupMembershipManager.getLogicalGroupSummary(mEntry);
if (groupSummary.isClearable()) {
// If this is the only child in the group, dismiss the group, but don't try to show
// the blocking helper affordance!
groupSummary.getRow().performDismiss(fromAccessibility);
}
}
dismiss(fromAccessibility);
if (mEntry.isClearable()) {
if (mOnUserInteractionCallback != null) {

View File

@@ -28,6 +28,8 @@ import com.android.internal.logging.MetricsLogger;
import com.android.internal.logging.nano.MetricsProto.MetricsEvent;
import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin;
import com.android.systemui.statusbar.notification.NotificationEntryManager;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.collection.render.GroupMembershipManager;
import com.android.systemui.statusbar.notification.dagger.NotificationsModule;
import com.android.systemui.statusbar.notification.logging.NotificationCounters;
@@ -48,6 +50,7 @@ public class NotificationBlockingHelperManager {
private final NotificationGutsManager mNotificationGutsManager;
private final NotificationEntryManager mNotificationEntryManager;
private final MetricsLogger mMetricsLogger;
private final GroupMembershipManager mGroupMembershipManager;
/** Row that the blocking helper will be shown in (via {@link NotificationGuts}. */
private ExpandableNotificationRow mBlockingHelperRow;
private Set<String> mNonBlockablePkgs;
@@ -65,7 +68,8 @@ public class NotificationBlockingHelperManager {
Context context,
NotificationGutsManager notificationGutsManager,
NotificationEntryManager notificationEntryManager,
MetricsLogger metricsLogger) {
MetricsLogger metricsLogger,
GroupMembershipManager groupMembershipManager) {
mContext = context;
mNotificationGutsManager = notificationGutsManager;
mNotificationEntryManager = notificationEntryManager;
@@ -73,6 +77,7 @@ public class NotificationBlockingHelperManager {
mNonBlockablePkgs = new HashSet<>();
Collections.addAll(mNonBlockablePkgs, mContext.getResources().getStringArray(
com.android.internal.R.array.config_nonBlockableNotificationPackages));
mGroupMembershipManager = groupMembershipManager;
}
/**
@@ -92,11 +97,12 @@ public class NotificationBlockingHelperManager {
// - The row is blockable (i.e. not non-blockable)
// - The dismissed row is a valid group (>1 or 0 children from the same channel)
// or the only child in the group
if ((row.getEntry().getUserSentiment() == USER_SENTIMENT_NEGATIVE || DEBUG)
final NotificationEntry entry = row.getEntry();
if ((entry.getUserSentiment() == USER_SENTIMENT_NEGATIVE || DEBUG)
&& mIsShadeExpanded
&& !row.getIsNonblockable()
&& ((!row.isChildInGroup() || row.isOnlyChildInGroup())
&& row.getNumUniqueChannels() <= 1)) {
&& ((!row.isChildInGroup() || mGroupMembershipManager.isOnlyChildInGroup(entry))
&& row.getNumUniqueChannels() <= 1)) {
// Dismiss any current blocking helper before continuing forward (only one can be shown
// at a given time).
dismissCurrentBlockingHelper();

View File

@@ -281,17 +281,10 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit
// TODO: Some of this code may be able to move to NotificationEntryManager.
removeHUN(row);
NotificationEntry parentToCancel = null;
if (shouldAutoCancel(entry.getSbn()) && mGroupMembershipManager.isOnlyChildInGroup(entry)) {
NotificationEntry summarySbn = mGroupMembershipManager.getLogicalGroupSummary(entry);
if (shouldAutoCancel(summarySbn.getSbn())) {
parentToCancel = summarySbn;
}
}
final NotificationEntry parentToCancelFinal = parentToCancel;
final Runnable runnable = () -> handleNotificationClickAfterPanelCollapsed(
entry, row, controller, intent,
isActivityIntent, wasOccluded, parentToCancelFinal);
isActivityIntent, wasOccluded);
if (showOverLockscreen) {
mShadeController.addPostCollapseAction(runnable);
@@ -312,8 +305,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit
RemoteInputController controller,
PendingIntent intent,
boolean isActivityIntent,
boolean wasOccluded,
NotificationEntry parentToCancelFinal) {
boolean wasOccluded) {
String notificationKey = entry.getKey();
mLogger.logHandleClickAfterPanelCollapsed(notificationKey);
@@ -373,22 +365,23 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit
NotificationLogger.getNotificationLocation(entry);
final NotificationVisibility nv = NotificationVisibility.obtain(notificationKey,
rank, count, true, location);
// NMS will officially remove notification if the notification has FLAG_AUTO_CANCEL:
mClickNotifier.onNotificationClick(notificationKey, nv);
if (!canBubble) {
if (parentToCancelFinal != null) {
// TODO: (b/145659174) remove - this cancels the parent if the notification clicked
// on will auto-cancel and is the only child in the group. This won't be
// necessary in the new pipeline due to group pruning in ShadeListBuilder.
removeNotification(parentToCancelFinal);
}
// TODO (b/162832756): delete these notification removals when migrating to the new
// pipeline; this is taken care of in {@link NotifCollection#tryRemoveNotification}
// which cancels lifetime extenders if the notification was dismissed by the user (ie:
// clicked or manually dismissed)
if (!canBubble && !mFeatureFlags.isNewNotifPipelineRenderingEnabled()) {
if (shouldAutoCancel(entry.getSbn())
|| mRemoteInputManager.isNotificationKeptForRemoteInputHistory(
notificationKey)) {
// Automatically remove all notifications that we may have kept around longer
// manually call notification removal in order to cancel any lifetime extenders
removeNotification(row.getEntry());
}
}
mIsCollapsingToShowActivityOverLockscreen = false;
}

View File

@@ -442,7 +442,7 @@ public class NotifCollectionTest extends SysuiTestCase {
}
@Test
public void testDismissingLifetimeExtendedSummaryDoesNotDismissChildren() {
public void testRetractingLifetimeExtendedSummaryDoesNotDismissChildren() {
// GIVEN A notif group with one summary and two children
mCollection.addNotificationLifetimeExtender(mExtender1);
CollectionEvent notif1 = postNotif(
@@ -460,15 +460,16 @@ public class NotifCollectionTest extends SysuiTestCase {
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
// GIVEN that the summary and one child are retracted by the app, but both are
// lifetime-extended
mExtender1.shouldExtendLifetime = true;
mNoMan.retractNotif(notif1.sbn, REASON_CANCEL);
mNoMan.retractNotif(notif2.sbn, REASON_CANCEL);
mNoMan.retractNotif(notif1.sbn, REASON_APP_CANCEL);
mNoMan.retractNotif(notif2.sbn, REASON_APP_CANCEL);
assertEquals(
new ArraySet<>(List.of(entry1, entry2, entry3)),
new ArraySet<>(mCollection.getAllNotifs()));
// WHEN the summary is dismissed by the user
// WHEN the summary is retracted by the app
mCollection.dismissNotification(entry1, defaultStats(entry1));
// THEN the summary is removed, but both children stick around
@@ -479,6 +480,28 @@ public class NotifCollectionTest extends SysuiTestCase {
assertEquals(NOT_DISMISSED, entry3.getDismissState());
}
@Test
public void testNMSReportsUserDismissalAlwaysRemovesNotif() throws RemoteException {
// GIVEN notifications are lifetime extended
mExtender1.shouldExtendLifetime = true;
CollectionEvent notif = postNotif(buildNotif(TEST_PACKAGE, 1, "myTag"));
CollectionEvent notif2 = postNotif(buildNotif(TEST_PACKAGE, 2, "myTag"));
NotificationEntry entry = mCollectionListener.getEntry(notif.key);
NotificationEntry entry2 = mCollectionListener.getEntry(notif2.key);
assertEquals(
new ArraySet<>(List.of(entry, entry2)),
new ArraySet<>(mCollection.getAllNotifs()));
// WHEN the notifications are reported to be dismissed by the user by NMS
mNoMan.retractNotif(notif.sbn, REASON_CANCEL);
mNoMan.retractNotif(notif2.sbn, REASON_CLICK);
// THEN the notifications are removed b/c they were dismissed by the user
assertEquals(
new ArraySet<>(List.of()),
new ArraySet<>(mCollection.getAllNotifs()));
}
@Test
public void testDismissNotificationCallsDismissInterceptors() throws RemoteException {
// GIVEN a collection with notifications with multiple dismiss interceptors
@@ -833,13 +856,13 @@ public class NotifCollectionTest extends SysuiTestCase {
NotifEvent notif2 = mNoMan.postNotif(buildNotif(TEST_PACKAGE2, 88));
NotificationEntry entry2 = mCollectionListener.getEntry(notif2.key);
// WHEN a notification is removed
mNoMan.retractNotif(notif2.sbn, REASON_CLICK);
// WHEN a notification is removed by the app
mNoMan.retractNotif(notif2.sbn, REASON_APP_CANCEL);
// THEN each extender is asked whether to extend, even if earlier ones return true
verify(mExtender1).shouldExtendLifetime(entry2, REASON_CLICK);
verify(mExtender2).shouldExtendLifetime(entry2, REASON_CLICK);
verify(mExtender3).shouldExtendLifetime(entry2, REASON_CLICK);
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.getAllNotifs().contains(entry2));

View File

@@ -49,6 +49,7 @@ import com.android.systemui.SysuiTestCase;
import com.android.systemui.bubbles.BubbleController;
import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin;
import com.android.systemui.statusbar.notification.NotificationEntryManager;
import com.android.systemui.statusbar.notification.collection.render.GroupMembershipManager;
import org.junit.Before;
import org.junit.Test;
@@ -71,6 +72,7 @@ public class NotificationBlockingHelperManagerTest extends SysuiTestCase {
@Mock private NotificationEntryManager mEntryManager;
@Mock private NotificationMenuRow mMenuRow;
@Mock private NotificationMenuRowPlugin.MenuItem mMenuItem;
@Mock private GroupMembershipManager mGroupMembershipManager;
@Before
public void setUp() {
@@ -89,7 +91,8 @@ public class NotificationBlockingHelperManagerTest extends SysuiTestCase {
mHelper = new NotificationTestHelper(mContext, mDependency, TestableLooper.get(this));
mBlockingHelperManager = new NotificationBlockingHelperManager(
mContext, mGutsManager, mEntryManager, mock(MetricsLogger.class));
mContext, mGutsManager, mEntryManager, mock(MetricsLogger.class),
mGroupMembershipManager);
// By default, have the shade visible/expanded.
mBlockingHelperManager.setNotificationShadeExpanded(1f);
}
@@ -185,6 +188,7 @@ public class NotificationBlockingHelperManagerTest extends SysuiTestCase {
.build();
assertFalse(childRow.getIsNonblockable());
when(mGroupMembershipManager.isOnlyChildInGroup(childRow.getEntry())).thenReturn(true);
assertTrue(mBlockingHelperManager.perhapsShowBlockingHelper(childRow, mMenuRow));
verify(mGutsManager).openGuts(childRow, 0, 0, mMenuItem);