Ensure cancelation order for groups in clearAll

Group summaries will be canceled before their children, to
prevent situations where a child cancelation causes a group summary
update which interferes with the cancelation of the summary from
the clear all action.

Additionally this fixes a bug where child notifications that
are priority conversations where not being canceled during clear all.

Test: atest NotificationManagerServiceTest, atest NotificationManagerTest
Fixes: 148387460
Change-Id: I21d34444e5097ec1efc6e06654ced9f894399bf4

2 files changed, 80 insertions, 32 deletions

diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index 1fd7a73f1174..79882dab48a1 100755
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -8019,7 +8019,7 @@ public class NotificationManagerService extends SystemService {
             int callingUid, int callingPid, String pkg, boolean nullPkgIndicatesUserSwitch,
             String channelId, FlagChecker flagChecker, boolean includeCurrentProfiles, int userId,
             boolean sendDelete, int reason, String listenerName, boolean wasPosted) {
-        ArrayList<NotificationRecord> canceledNotifications = null;
+        Set<String> childNotifications = null;
         for (int i = notificationList.size() - 1; i >= 0; --i) {
             NotificationRecord r = notificationList.get(i);
             if (includeCurrentProfiles) {
@@ -8042,20 +8042,30 @@ public class NotificationManagerService extends SystemService {
             if (channelId != null && !channelId.equals(r.getChannel().getId())) {
                 continue;
             }
-            if (canceledNotifications == null) {
-                canceledNotifications = new ArrayList<>();
+            if (r.getSbn().isGroup() && r.getNotification().isGroupChild()) {
+                if (childNotifications == null) {
+                    childNotifications = new HashSet<>();
+                }
+                childNotifications.add(r.getKey());
+                continue;
             }
             notificationList.remove(i);
             mNotificationsByKey.remove(r.getKey());
             r.recordDismissalSentiment(NotificationStats.DISMISS_SENTIMENT_NEUTRAL);
-            canceledNotifications.add(r);
             cancelNotificationLocked(r, sendDelete, reason, wasPosted, listenerName);
         }
-        if (canceledNotifications != null) {
-            final int M = canceledNotifications.size();
-            for (int i = 0; i < M; i++) {
-                cancelGroupChildrenLocked(canceledNotifications.get(i), callingUid, callingPid,
-                        listenerName, false /* sendDelete */, flagChecker, reason);
+        if (childNotifications != null) {
+            final int M = notificationList.size();
+            for (int i = M - 1; i >= 0; i--) {
+                NotificationRecord r = notificationList.get(i);
+                if (childNotifications.contains(r.getKey())) {
+                    // dismiss conditions were checked in the first loop and so don't need to be
+                    // checked again
+                    notificationList.remove(i);
+                    mNotificationsByKey.remove(r.getKey());
+                    r.recordDismissalSentiment(NotificationStats.DISMISS_SENTIMENT_NEUTRAL);
+                    cancelNotificationLocked(r, sendDelete, reason, wasPosted, listenerName);
+                }
             }
             updateLightsLocked();
         }
diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
index 1a4b119a6c99..ed6a20b97d37 100755
--- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -74,6 +74,7 @@ import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.reset;
@@ -185,6 +186,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
+import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.mockito.stubbing.Answer;
@@ -563,6 +565,37 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
                 .getUiAutomation().dropShellPermissionIdentity();
     }
 
+    private void simulatePackageSuspendBroadcast(boolean suspend, String pkg,
+            int uid) {
+        // mimics receive broadcast that package is (un)suspended
+        // but does not actually (un)suspend the package
+        final Bundle extras = new Bundle();
+        extras.putStringArray(Intent.EXTRA_CHANGED_PACKAGE_LIST,
+                new String[]{pkg});
+        extras.putIntArray(Intent.EXTRA_CHANGED_UID_LIST, new int[]{uid});
+
+        final String action = suspend ? Intent.ACTION_PACKAGES_SUSPENDED
+                : Intent.ACTION_PACKAGES_UNSUSPENDED;
+        final Intent intent = new Intent(action);
+        intent.putExtras(extras);
+
+        mPackageIntentReceiver.onReceive(getContext(), intent);
+    }
+
+    private void simulatePackageDistractionBroadcast(int flag, String[] pkgs, int[] uids) {
+        // mimics receive broadcast that package is (un)distracting
+        // but does not actually register that info with packagemanager
+        final Bundle extras = new Bundle();
+        extras.putStringArray(Intent.EXTRA_CHANGED_PACKAGE_LIST, pkgs);
+        extras.putInt(Intent.EXTRA_DISTRACTION_RESTRICTIONS, flag);
+        extras.putIntArray(Intent.EXTRA_CHANGED_UID_LIST, uids);
+
+        final Intent intent = new Intent(Intent.ACTION_DISTRACTING_PACKAGES_CHANGED);
+        intent.putExtras(extras);
+
+        mPackageIntentReceiver.onReceive(getContext(), intent);
+    }
+
     private ArrayMap<Boolean, ArrayList<ComponentName>> generateResetComponentValues() {
         ArrayMap<Boolean, ArrayList<ComponentName>> changed = new ArrayMap<>();
         changed.put(true, new ArrayList<>());
@@ -7066,34 +7099,39 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
         assertTrue(mService.isVisibleToListener(sbn, info));
     }
 
-    private void simulatePackageSuspendBroadcast(boolean suspend, String pkg,
-            int uid) {
-        // mimics receive broadcast that package is (un)suspended
-        // but does not actually (un)suspend the package
-        final Bundle extras = new Bundle();
-        extras.putStringArray(Intent.EXTRA_CHANGED_PACKAGE_LIST,
-                new String[]{pkg});
-        extras.putIntArray(Intent.EXTRA_CHANGED_UID_LIST, new int[]{uid});
+    @Test
+    public void testUserInitiatedCancelAll_groupCancellationOrder_groupPostedFirst() {
+        final NotificationRecord parent = spy(generateNotificationRecord(
+                mTestNotificationChannel, 1, "group", true));
+        final NotificationRecord child = spy(generateNotificationRecord(
+                mTestNotificationChannel, 2, "group", false));
+        mService.addNotification(parent);
+        mService.addNotification(child);
 
-        final String action = suspend ? Intent.ACTION_PACKAGES_SUSPENDED
-                : Intent.ACTION_PACKAGES_UNSUSPENDED;
-        final Intent intent = new Intent(action);
-        intent.putExtras(extras);
+        InOrder inOrder = inOrder(parent, child);
 
-        mPackageIntentReceiver.onReceive(getContext(), intent);
+        mService.mNotificationDelegate.onClearAll(mUid, Binder.getCallingPid(),
+                parent.getUserId());
+        waitForIdle();
+        inOrder.verify(parent).recordDismissalSentiment(anyInt());
+        inOrder.verify(child).recordDismissalSentiment(anyInt());
     }
 
-    private void simulatePackageDistractionBroadcast(int flag, String[] pkgs, int[] uids) {
-        // mimics receive broadcast that package is (un)distracting
-        // but does not actually register that info with packagemanager
-        final Bundle extras = new Bundle();
-        extras.putStringArray(Intent.EXTRA_CHANGED_PACKAGE_LIST, pkgs);
-        extras.putInt(Intent.EXTRA_DISTRACTION_RESTRICTIONS, flag);
-        extras.putIntArray(Intent.EXTRA_CHANGED_UID_LIST, uids);
+    @Test
+    public void testUserInitiatedCancelAll_groupCancellationOrder_groupPostedSecond() {
+        final NotificationRecord parent = spy(generateNotificationRecord(
+                mTestNotificationChannel, 1, "group", true));
+        final NotificationRecord child = spy(generateNotificationRecord(
+                mTestNotificationChannel, 2, "group", false));
+        mService.addNotification(child);
+        mService.addNotification(parent);
 
-        final Intent intent = new Intent(Intent.ACTION_DISTRACTING_PACKAGES_CHANGED);
-        intent.putExtras(extras);
+        InOrder inOrder = inOrder(parent, child);
 
-        mPackageIntentReceiver.onReceive(getContext(), intent);
+        mService.mNotificationDelegate.onClearAll(mUid, Binder.getCallingPid(),
+                parent.getUserId());
+        waitForIdle();
+        inOrder.verify(parent).recordDismissalSentiment(anyInt());
+        inOrder.verify(child).recordDismissalSentiment(anyInt());
     }
 }