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 90492b5d606dc..fdfd72489e93d 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 @@ -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; } /** diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/OnUserInteractionCallbackImplLegacy.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/OnUserInteractionCallbackImplLegacy.java index cce8cdc64d306..610cd33383e91 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/OnUserInteractionCallbackImplLegacy.java +++ b/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 diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java index e2aae64ce220e..ea86d25389fae 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java +++ b/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, Lazy visualStabilityCoordinator, NotificationEntryManager entryManager, - VisualStabilityManager visualStabilityManager) { + VisualStabilityManager visualStabilityManager, + Lazy groupMembershipManagerLazy) { return featureFlags.isNewNotifPipelineRenderingEnabled() ? new OnUserInteractionCallbackImpl( pipeline.get(), @@ -285,7 +288,8 @@ public interface NotificationsModule { entryManager, headsUpManager, statusBarStateController, - visualStabilityManager); + visualStabilityManager, + groupMembershipManagerLazy.get()); } /** */ diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java index 89f720535402b..adda049951ac0 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java +++ b/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) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationBlockingHelperManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationBlockingHelperManager.java index 921232568755c..ab78d197da0bb 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationBlockingHelperManager.java +++ b/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 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(); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java index f80656706f37e..737cdeba797a6 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java +++ b/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, - 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; } 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 359faba48f084..ce0f1220fc881 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 @@ -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)); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationBlockingHelperManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationBlockingHelperManagerTest.java index 5aeb43fbd9590..edb8776bcb021 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationBlockingHelperManagerTest.java +++ b/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);