diff --git a/services/core/java/com/android/server/notification/GroupHelper.java b/services/core/java/com/android/server/notification/GroupHelper.java index 57c558cddb548..ce805aadec777 100644 --- a/services/core/java/com/android/server/notification/GroupHelper.java +++ b/services/core/java/com/android/server/notification/GroupHelper.java @@ -38,14 +38,14 @@ public class GroupHelper { private final Callback mCallback; // Map of user : . Only contains notifications that are not - // groupd by the app (aka no group or sort key). + // grouped by the app (aka no group or sort key). Map>> mUngroupedNotifications = new HashMap<>(); public GroupHelper(Callback callback) {; mCallback = callback; } - public void onNotificationPosted(StatusBarNotification sbn) { + public void onNotificationPosted(StatusBarNotification sbn, boolean autogroupSummaryExists) { if (DEBUG) Log.i(TAG, "POSTED " + sbn.getKey()); try { List notificationsToGroup = new ArrayList<>(); @@ -68,7 +68,8 @@ public class GroupHelper { notificationsForPackage.add(sbn.getKey()); ungroupedNotificationsByUser.put(sbn.getPackageName(), notificationsForPackage); - if (notificationsForPackage.size() >= AUTOGROUP_AT_COUNT) { + if (notificationsForPackage.size() >= AUTOGROUP_AT_COUNT + || autogroupSummaryExists) { notificationsToGroup.addAll(notificationsForPackage); } } @@ -120,6 +121,7 @@ public class GroupHelper { // 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) { + ungroupedNotificationsByUser.remove(sbn.getPackageName()); removeSummary = true; } } diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 704978b9616f8..cc9f183fb69ee 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -187,7 +187,6 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileDescriptor; -import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; @@ -2051,9 +2050,7 @@ public class NotificationManagerService extends SystemService { private StatusBarNotification sanitizeSbn(String pkg, int userId, StatusBarNotification sbn) { - if (sbn.getPackageName().equals(pkg) && sbn.getUserId() == userId - && (sbn.getNotification().flags - & Notification.FLAG_AUTOGROUP_SUMMARY) == 0) { + if (sbn.getPackageName().equals(pkg) && sbn.getUserId() == userId) { // We could pass back a cloneLight() but clients might get confused and // try to send this thing back to notify() again, which would not work // very well. @@ -3061,6 +3058,12 @@ public class NotificationManagerService extends SystemService { } } + @GuardedBy("mNotificationLock") + private boolean hasAutoGroupSummaryLocked(StatusBarNotification sbn) { + ArrayMap summaries = mAutobundledSummaries.get(sbn.getUserId()); + return summaries != null && summaries.containsKey(sbn.getPackageName()); + } + // Posts a 'fake' summary for a package that has exceeded the solo-notification limit. private void createAutoGroupSummary(int userId, String pkg, String triggeringKey) { NotificationRecord summaryRecord = null; @@ -3836,7 +3839,8 @@ public class NotificationManagerService extends SystemService { mHandler.post(new Runnable() { @Override public void run() { - mGroupHelper.onNotificationPosted(n); + mGroupHelper.onNotificationPosted( + n, hasAutoGroupSummaryLocked(n)); } }); } 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 8dd177921022d..f75c648f3c3e5 100644 --- a/services/tests/notification/src/com/android/server/notification/GroupHelperTest.java +++ b/services/tests/notification/src/com/android/server/notification/GroupHelperTest.java @@ -15,6 +15,9 @@ */ package com.android.server.notification; +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertNotNull; + import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; @@ -29,18 +32,16 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; -import android.app.AlarmManager; import android.app.Notification; -import android.app.NotificationChannel; -import android.app.NotificationManager; -import android.app.PendingIntent; import android.os.UserHandle; import android.service.notification.StatusBarNotification; import android.support.test.runner.AndroidJUnit4; import android.test.suitebuilder.annotation.SmallTest; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; @SmallTest @RunWith(AndroidJUnit4.class) @@ -77,7 +78,8 @@ public class GroupHelperTest extends NotificationTestCase { public void testNoGroup_postingUnderLimit() throws Exception { final String pkg = "package"; for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 1; i++) { - mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM)); + mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM), + false); } verify(mCallback, never()).addAutoGroupSummary( eq(UserHandle.USER_SYSTEM), eq(pkg), anyString()); @@ -91,10 +93,11 @@ public class GroupHelperTest extends NotificationTestCase { final String pkg = "package"; final String pkg2 = "package2"; for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 1; i++) { - mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM)); + mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM), + false); } mGroupHelper.onNotificationPosted( - getSbn(pkg2, GroupHelper.AUTOGROUP_AT_COUNT, "four", UserHandle.SYSTEM)); + getSbn(pkg2, GroupHelper.AUTOGROUP_AT_COUNT, "four", UserHandle.SYSTEM), false); verify(mCallback, never()).addAutoGroupSummary( eq(UserHandle.USER_SYSTEM), eq(pkg), anyString()); verify(mCallback, never()).addAutoGroup(anyString()); @@ -106,10 +109,12 @@ public class GroupHelperTest extends NotificationTestCase { public void testNoGroup_multiUser() throws Exception { final String pkg = "package"; for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 1; i++) { - mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM)); + mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM), + false); } mGroupHelper.onNotificationPosted( - getSbn(pkg, GroupHelper.AUTOGROUP_AT_COUNT, "four", UserHandle.ALL)); + getSbn(pkg, GroupHelper.AUTOGROUP_AT_COUNT, "four", UserHandle.ALL), + false); verify(mCallback, never()).addAutoGroupSummary(anyInt(), eq(pkg), anyString()); verify(mCallback, never()).addAutoGroup(anyString()); verify(mCallback, never()).removeAutoGroup(anyString()); @@ -120,10 +125,12 @@ public class GroupHelperTest extends NotificationTestCase { public void testNoGroup_someAreGrouped() throws Exception { final String pkg = "package"; for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 1; i++) { - mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM)); + mGroupHelper.onNotificationPosted( + getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM), false); } mGroupHelper.onNotificationPosted( - getSbn(pkg, GroupHelper.AUTOGROUP_AT_COUNT, "four", UserHandle.SYSTEM, "a")); + getSbn(pkg, GroupHelper.AUTOGROUP_AT_COUNT, "four", UserHandle.SYSTEM, "a"), + false); verify(mCallback, never()).addAutoGroupSummary( eq(UserHandle.USER_SYSTEM), eq(pkg), anyString()); verify(mCallback, never()).addAutoGroup(anyString()); @@ -136,7 +143,8 @@ public class GroupHelperTest extends NotificationTestCase { public void testPostingOverLimit() throws Exception { final String pkg = "package"; for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) { - mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM)); + mGroupHelper.onNotificationPosted( + getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM), false); } verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString()); verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT)).addAutoGroup(anyString()); @@ -151,7 +159,7 @@ public class GroupHelperTest extends NotificationTestCase { 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); + mGroupHelper.onNotificationPosted(sbn, false); } verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString()); verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT)).addAutoGroup(anyString()); @@ -178,7 +186,7 @@ public class GroupHelperTest extends NotificationTestCase { 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); + mGroupHelper.onNotificationPosted(sbn, false); } verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString()); verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT)).addAutoGroup(anyString()); @@ -190,7 +198,7 @@ public class GroupHelperTest extends NotificationTestCase { 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); + mGroupHelper.onNotificationPosted(sbn, false); } verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT - 2)).removeAutoGroup(anyString()); verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString()); @@ -199,9 +207,48 @@ public class GroupHelperTest extends NotificationTestCase { for (; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) { final StatusBarNotification sbn = getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM, "app group"); - mGroupHelper.onNotificationPosted(sbn); + mGroupHelper.onNotificationPosted(sbn, false); } verify(mCallback, times(2)).removeAutoGroup(anyString()); verify(mCallback, times(1)).removeAutoGroupSummary(anyInt(), anyString()); } + + @Test + public void testNewNotificationsAddedToAutogroup_ifOriginalNotificationsCanceled() + 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, false); + } + 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 = posted.size() - 2; i >= 0; i--) { + mGroupHelper.onNotificationRemoved(posted.remove(i)); + } + verify(mCallback, never()).removeAutoGroup(anyString()); + verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString()); + Mockito.reset(mCallback); + + // only one child remains + Map> ungroupedForUser = + mGroupHelper.mUngroupedNotifications.get(UserHandle.USER_SYSTEM); + assertNotNull(ungroupedForUser); + assertEquals(1, ungroupedForUser.get(pkg).size()); + + // Add new notification; it should be autogrouped even though the total count is + // < AUTOGROUP_AT_COUNT + final StatusBarNotification sbn = getSbn(pkg, 5, String.valueOf(5), UserHandle.SYSTEM); + posted.add(sbn); + mGroupHelper.onNotificationPosted(sbn, true); + verify(mCallback, times(posted.size())).addAutoGroup(anyString()); + verify(mCallback, never()).removeAutoGroup(anyString()); + verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString()); + } } diff --git a/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java index 9acf5194286fc..04b42f1ee3121 100644 --- a/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/notification/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -1339,7 +1339,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase { runnable.run(); waitForIdle(); - verify(mGroupHelper, times(1)).onNotificationPosted(any()); + verify(mGroupHelper, times(1)).onNotificationPosted(any(), anyBoolean()); } @Test @@ -1355,7 +1355,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase { runnable.run(); waitForIdle(); - verify(mGroupHelper, times(1)).onNotificationPosted(any()); + verify(mGroupHelper, times(1)).onNotificationPosted(any(), anyBoolean()); } @Test @@ -1371,7 +1371,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase { runnable.run(); waitForIdle(); - verify(mGroupHelper, never()).onNotificationPosted(any()); + verify(mGroupHelper, never()).onNotificationPosted(any(), anyBoolean()); } @Test