Merge "Update removal logic for notifications"

This commit is contained in:
TreeHugger Robot
2020-09-09 00:06:29 +00:00
committed by Android (Google) Code Review
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);