Merge "Update removal logic for notifications"

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java

@@ -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)) {
+        if (cannotBeLifetimeExtended(entry)) {
-            // User-dismissed notifications cannot be lifetime-extended
             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) {
+    private boolean cannotBeLifetimeExtended(NotificationEntry entry) {
-        return entry.getDismissState() != NOT_DISMISSED;
+        final boolean locallyDismissedByUser = entry.getDismissState() != NOT_DISMISSED;
+        final boolean systemServerReportedUserCancel =
+                entry.mCancellationReason == REASON_CLICK
+                        || entry.mCancellationReason == REASON_CANCEL;
+        return locallyDismissedByUser || systemServerReportedUserCancel;
     }
 
     /**

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/OnUserInteractionCallbackImplLegacy.java

@@ -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

packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java

@@ -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());
     }
 
     /** */

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java

@@ -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) {

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationBlockingHelperManager.java

@@ -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.isChildInGroup() || mGroupMembershipManager.isOnlyChildInGroup(entry))
-                        && row.getNumUniqueChannels() <= 1)) {
+                    && row.getNumUniqueChannels() <= 1)) {
             // Dismiss any current blocking helper before continuing forward (only one can be shown
             // at a given time).
             dismissCurrentBlockingHelper();

packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java

@@ -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,
+            boolean wasOccluded) {
-            NotificationEntry parentToCancelFinal) {
         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) {
+        // TODO (b/162832756): delete these notification removals when migrating to the new
-            if (parentToCancelFinal != null) {
+        //  pipeline; this is taken care of in {@link NotifCollection#tryRemoveNotification}
-                // TODO: (b/145659174) remove - this cancels the parent if the notification clicked
+        //  which cancels lifetime extenders if the notification was dismissed by the user (ie:
-                // on will auto-cancel and is the only child in the group. This won't be
+        //  clicked or manually dismissed)
-                // necessary in the new pipeline due to group pruning in ShadeListBuilder.
+        if (!canBubble && !mFeatureFlags.isNewNotifPipelineRenderingEnabled()) {
-                removeNotification(parentToCancelFinal);
-            }
             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;
     }
 

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotifCollectionTest.java

@@ -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(notif1.sbn, REASON_APP_CANCEL);
-        mNoMan.retractNotif(notif2.sbn, REASON_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
+        // WHEN a notification is removed by the app
-        mNoMan.retractNotif(notif2.sbn, REASON_CLICK);
+        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(mExtender1).shouldExtendLifetime(entry2, REASON_APP_CANCEL);
-        verify(mExtender2).shouldExtendLifetime(entry2, REASON_CLICK);
+        verify(mExtender2).shouldExtendLifetime(entry2, REASON_APP_CANCEL);
-        verify(mExtender3).shouldExtendLifetime(entry2, REASON_CLICK);
+        verify(mExtender3).shouldExtendLifetime(entry2, REASON_APP_CANCEL);
 
         // THEN the entry is not removed
         assertTrue(mCollection.getAllNotifs().contains(entry2));

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationBlockingHelperManagerTest.java

@@ -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);