From f4af65bbd6ccd9cf6951e26da12c2789ab14c42a Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Tue, 20 Dec 2016 17:05:12 -0500 Subject: [PATCH] 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 --- .../server/notification/GroupHelper.java | 29 +++++------ .../server/notification/GroupHelperTest.java | 52 +++++++++++++++++-- 2 files changed, 61 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/notification/GroupHelper.java b/services/core/java/com/android/server/notification/GroupHelper.java index 8ea49093a7554..57c558cddb548 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 notificationsToUnAutogroup = new ArrayList<>(); boolean removeSummary = false; synchronized (mUngroupedNotifications) { - Map> ungroupdNotificationsByUser + Map> ungroupedNotificationsByUser = mUngroupedNotifications.get(sbn.getUserId()); - if (ungroupdNotificationsByUser == null || ungroupdNotificationsByUser.size() == 0) { + if (ungroupedNotificationsByUser == null || ungroupedNotificationsByUser.size() == 0) { return; } LinkedHashSet 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 6c3f44778ae64..f48d785b660d5 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 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); } - mGroupHelper.onNotificationRemoved(posted.remove(0)); 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); + + 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 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, 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()); } }