Don't automatically remove autogroup summaries

Unless there are no notifications left that need to be autogrouped.

Change-Id: I76f90cd083273d1babf1209e3c509d213deffa9c
Fixes: 28612382
Test: runtest systemui-notification

2 files changed, 60 insertions, 19 deletions

diff --git a/services/core/java/com/android/server/notification/GroupHelper.java b/services/core/java/com/android/server/notification/GroupHelper.java
index 8ea49093a755..57c558cddb54 100644
--- a/services/core/java/com/android/server/notification/GroupHelper.java
+++ b/services/core/java/com/android/server/notification/GroupHelper.java
@@ -95,21 +95,19 @@ public class GroupHelper {
     }
 
     /**
-     * Un-autogroups notifications that are now grouped by the app. Additionally cancels
-     * autogrouping if the status change of this notification resulted in the loose notification
-     * count being under the limit.
+     * Un-autogroups notifications that are now grouped by the app.
      */
     private void maybeUngroup(StatusBarNotification sbn, boolean notificationGone, int userId) {
         List<String> notificationsToUnAutogroup = new ArrayList<>();
         boolean removeSummary = false;
         synchronized (mUngroupedNotifications) {
-            Map<String, LinkedHashSet<String>> ungroupdNotificationsByUser
+            Map<String, LinkedHashSet<String>> ungroupedNotificationsByUser
                     = mUngroupedNotifications.get(sbn.getUserId());
-            if (ungroupdNotificationsByUser == null || ungroupdNotificationsByUser.size() == 0) {
+            if (ungroupedNotificationsByUser == null || ungroupedNotificationsByUser.size() == 0) {
                 return;
             }
             LinkedHashSet<String> notificationsForPackage
-                    = ungroupdNotificationsByUser.get(sbn.getPackageName());
+                    = ungroupedNotificationsByUser.get(sbn.getPackageName());
             if (notificationsForPackage == null || notificationsForPackage.size() == 0) {
                 return;
             }
@@ -118,20 +116,17 @@ public class GroupHelper {
                     // Add the current notification to the ungrouping list if it still exists.
                     notificationsToUnAutogroup.add(sbn.getKey());
                 }
-                // If the status change of this notification has brought the number of loose
-                // notifications back below the limit, remove the summary and un-autogroup.
-                if (notificationsForPackage.size() == AUTOGROUP_AT_COUNT - 1) {
-                    removeSummary = true;
-                    for (String key : notificationsForPackage) {
-                        notificationsToUnAutogroup.add(key);
-                    }
-                }
+            }
+            // If the status change of this notification has brought the number of loose
+            // notifications to zero, remove the summary and un-autogroup.
+            if (notificationsForPackage.size() == 0) {
+                removeSummary = true;
             }
         }
+        if (removeSummary) {
+            adjustAutogroupingSummary(userId, sbn.getPackageName(), null, false);
+        }
         if (notificationsToUnAutogroup.size() > 0) {
-            if (removeSummary) {
-                adjustAutogroupingSummary(userId, sbn.getPackageName(), null, false);
-            }
             adjustNotificationBundling(notificationsToUnAutogroup, false);
         }
     }
diff --git a/services/tests/notification/src/com/android/server/notification/GroupHelperTest.java b/services/tests/notification/src/com/android/server/notification/GroupHelperTest.java
index 6c3f44778ae6..f48d785b660d 100644
--- a/services/tests/notification/src/com/android/server/notification/GroupHelperTest.java
+++ b/services/tests/notification/src/com/android/server/notification/GroupHelperTest.java
@@ -26,6 +26,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 
 import android.app.AlarmManager;
@@ -152,7 +153,7 @@ public class GroupHelperTest {
     }
 
     @Test
-    public void testDropBelowLimitRemoveGroup() throws Exception {
+    public void testDropToZeroRemoveGroup() throws Exception {
         final String pkg = "package";
         List<StatusBarNotification> posted = new ArrayList<>();
         for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) {
@@ -160,10 +161,55 @@ public class GroupHelperTest {
             posted.add(sbn);
             mGroupHelper.onNotificationPosted(sbn);
         }
+        verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString());
+        verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT)).addAutoGroup(anyString());
+        verify(mCallback, never()).removeAutoGroup(anyString());
+        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());
+        Mockito.reset(mCallback);
+
+        for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 1; i++) {
+            mGroupHelper.onNotificationRemoved(posted.remove(0));
+        }
+        verify(mCallback, never()).removeAutoGroup(anyString());
+        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());
+        Mockito.reset(mCallback);
+
         mGroupHelper.onNotificationRemoved(posted.remove(0));
+        verify(mCallback, never()).removeAutoGroup(anyString());
+        verify(mCallback, times(1)).removeAutoGroupSummary(anyInt(), anyString());
+    }
+
+    @Test
+    public void testAppStartsGrouping() throws Exception {
+        final String pkg = "package";
+        List<StatusBarNotification> posted = new ArrayList<>();
+        for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) {
+            final StatusBarNotification sbn = getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM);
+            posted.add(sbn);
+            mGroupHelper.onNotificationPosted(sbn);
+        }
         verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString());
         verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT)).addAutoGroup(anyString());
-        verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT - 1)).removeAutoGroup(anyString());
+        verify(mCallback, never()).removeAutoGroup(anyString());
+        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());
+        Mockito.reset(mCallback);
+
+        int i = 0;
+        for (i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 2; i++) {
+            final StatusBarNotification sbn =
+                    getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM, "app group");
+            mGroupHelper.onNotificationPosted(sbn);
+        }
+        verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT - 2)).removeAutoGroup(anyString());
+        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());
+        Mockito.reset(mCallback);
+
+        for (; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) {
+            final StatusBarNotification sbn =
+                    getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM, "app group");
+            mGroupHelper.onNotificationPosted(sbn);
+        }
+        verify(mCallback, times(2)).removeAutoGroup(anyString());
         verify(mCallback, times(1)).removeAutoGroupSummary(anyInt(), anyString());
     }
 }