Merge "Group new free notis if autogroup summary exists" into oc-mr1-dev

This commit is contained in:
TreeHugger Robot
2017-08-11 16:22:59 +00:00
committed by Android (Google) Code Review
4 changed files with 80 additions and 27 deletions

View File

@@ -38,14 +38,14 @@ public class GroupHelper {
private final Callback mCallback; private final Callback mCallback;
// Map of user : <Map of package : notification keys>. Only contains notifications that are not // Map of user : <Map of package : notification keys>. 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<Integer, Map<String, LinkedHashSet<String>>> mUngroupedNotifications = new HashMap<>(); Map<Integer, Map<String, LinkedHashSet<String>>> mUngroupedNotifications = new HashMap<>();
public GroupHelper(Callback callback) {; public GroupHelper(Callback callback) {;
mCallback = callback; mCallback = callback;
} }
public void onNotificationPosted(StatusBarNotification sbn) { public void onNotificationPosted(StatusBarNotification sbn, boolean autogroupSummaryExists) {
if (DEBUG) Log.i(TAG, "POSTED " + sbn.getKey()); if (DEBUG) Log.i(TAG, "POSTED " + sbn.getKey());
try { try {
List<String> notificationsToGroup = new ArrayList<>(); List<String> notificationsToGroup = new ArrayList<>();
@@ -68,7 +68,8 @@ public class GroupHelper {
notificationsForPackage.add(sbn.getKey()); notificationsForPackage.add(sbn.getKey());
ungroupedNotificationsByUser.put(sbn.getPackageName(), notificationsForPackage); ungroupedNotificationsByUser.put(sbn.getPackageName(), notificationsForPackage);
if (notificationsForPackage.size() >= AUTOGROUP_AT_COUNT) { if (notificationsForPackage.size() >= AUTOGROUP_AT_COUNT
|| autogroupSummaryExists) {
notificationsToGroup.addAll(notificationsForPackage); notificationsToGroup.addAll(notificationsForPackage);
} }
} }
@@ -120,6 +121,7 @@ public class GroupHelper {
// If the status change of this notification has brought the number of loose // If the status change of this notification has brought the number of loose
// notifications to zero, remove the summary and un-autogroup. // notifications to zero, remove the summary and un-autogroup.
if (notificationsForPackage.size() == 0) { if (notificationsForPackage.size() == 0) {
ungroupedNotificationsByUser.remove(sbn.getPackageName());
removeSummary = true; removeSummary = true;
} }
} }

View File

@@ -187,7 +187,6 @@ import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
import java.io.File; import java.io.File;
import java.io.FileDescriptor; import java.io.FileDescriptor;
import java.io.FileInputStream;
import java.io.FileNotFoundException; import java.io.FileNotFoundException;
import java.io.FileOutputStream; import java.io.FileOutputStream;
import java.io.IOException; import java.io.IOException;
@@ -2051,9 +2050,7 @@ public class NotificationManagerService extends SystemService {
private StatusBarNotification sanitizeSbn(String pkg, int userId, private StatusBarNotification sanitizeSbn(String pkg, int userId,
StatusBarNotification sbn) { StatusBarNotification sbn) {
if (sbn.getPackageName().equals(pkg) && sbn.getUserId() == userId if (sbn.getPackageName().equals(pkg) && sbn.getUserId() == userId) {
&& (sbn.getNotification().flags
& Notification.FLAG_AUTOGROUP_SUMMARY) == 0) {
// We could pass back a cloneLight() but clients might get confused and // 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 // try to send this thing back to notify() again, which would not work
// very well. // very well.
@@ -3061,6 +3058,12 @@ public class NotificationManagerService extends SystemService {
} }
} }
@GuardedBy("mNotificationLock")
private boolean hasAutoGroupSummaryLocked(StatusBarNotification sbn) {
ArrayMap<String, String> 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. // Posts a 'fake' summary for a package that has exceeded the solo-notification limit.
private void createAutoGroupSummary(int userId, String pkg, String triggeringKey) { private void createAutoGroupSummary(int userId, String pkg, String triggeringKey) {
NotificationRecord summaryRecord = null; NotificationRecord summaryRecord = null;
@@ -3836,7 +3839,8 @@ public class NotificationManagerService extends SystemService {
mHandler.post(new Runnable() { mHandler.post(new Runnable() {
@Override @Override
public void run() { public void run() {
mGroupHelper.onNotificationPosted(n); mGroupHelper.onNotificationPosted(
n, hasAutoGroupSummaryLocked(n));
} }
}); });
} }

View File

@@ -15,6 +15,9 @@
*/ */
package com.android.server.notification; 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.anyInt;
import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq; import static org.mockito.Matchers.eq;
@@ -29,18 +32,16 @@ import org.mockito.Mock;
import org.mockito.Mockito; import org.mockito.Mockito;
import org.mockito.MockitoAnnotations; import org.mockito.MockitoAnnotations;
import android.app.AlarmManager;
import android.app.Notification; import android.app.Notification;
import android.app.NotificationChannel;
import android.app.NotificationManager;
import android.app.PendingIntent;
import android.os.UserHandle; import android.os.UserHandle;
import android.service.notification.StatusBarNotification; import android.service.notification.StatusBarNotification;
import android.support.test.runner.AndroidJUnit4; import android.support.test.runner.AndroidJUnit4;
import android.test.suitebuilder.annotation.SmallTest; import android.test.suitebuilder.annotation.SmallTest;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List; import java.util.List;
import java.util.Map;
@SmallTest @SmallTest
@RunWith(AndroidJUnit4.class) @RunWith(AndroidJUnit4.class)
@@ -77,7 +78,8 @@ public class GroupHelperTest extends NotificationTestCase {
public void testNoGroup_postingUnderLimit() throws Exception { public void testNoGroup_postingUnderLimit() throws Exception {
final String pkg = "package"; final String pkg = "package";
for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 1; i++) { 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( verify(mCallback, never()).addAutoGroupSummary(
eq(UserHandle.USER_SYSTEM), eq(pkg), anyString()); eq(UserHandle.USER_SYSTEM), eq(pkg), anyString());
@@ -91,10 +93,11 @@ public class GroupHelperTest extends NotificationTestCase {
final String pkg = "package"; final String pkg = "package";
final String pkg2 = "package2"; final String pkg2 = "package2";
for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 1; i++) { 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( 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( verify(mCallback, never()).addAutoGroupSummary(
eq(UserHandle.USER_SYSTEM), eq(pkg), anyString()); eq(UserHandle.USER_SYSTEM), eq(pkg), anyString());
verify(mCallback, never()).addAutoGroup(anyString()); verify(mCallback, never()).addAutoGroup(anyString());
@@ -106,10 +109,12 @@ public class GroupHelperTest extends NotificationTestCase {
public void testNoGroup_multiUser() throws Exception { public void testNoGroup_multiUser() throws Exception {
final String pkg = "package"; final String pkg = "package";
for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 1; i++) { 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( 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()).addAutoGroupSummary(anyInt(), eq(pkg), anyString());
verify(mCallback, never()).addAutoGroup(anyString()); verify(mCallback, never()).addAutoGroup(anyString());
verify(mCallback, never()).removeAutoGroup(anyString()); verify(mCallback, never()).removeAutoGroup(anyString());
@@ -120,10 +125,12 @@ public class GroupHelperTest extends NotificationTestCase {
public void testNoGroup_someAreGrouped() throws Exception { public void testNoGroup_someAreGrouped() throws Exception {
final String pkg = "package"; final String pkg = "package";
for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 1; i++) { 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( 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( verify(mCallback, never()).addAutoGroupSummary(
eq(UserHandle.USER_SYSTEM), eq(pkg), anyString()); eq(UserHandle.USER_SYSTEM), eq(pkg), anyString());
verify(mCallback, never()).addAutoGroup(anyString()); verify(mCallback, never()).addAutoGroup(anyString());
@@ -136,7 +143,8 @@ public class GroupHelperTest extends NotificationTestCase {
public void testPostingOverLimit() throws Exception { public void testPostingOverLimit() throws Exception {
final String pkg = "package"; final String pkg = "package";
for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) { 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(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString());
verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT)).addAutoGroup(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++) { for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) {
final StatusBarNotification sbn = getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM); final StatusBarNotification sbn = getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM);
posted.add(sbn); posted.add(sbn);
mGroupHelper.onNotificationPosted(sbn); mGroupHelper.onNotificationPosted(sbn, false);
} }
verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString()); 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)).addAutoGroup(anyString());
@@ -178,7 +186,7 @@ public class GroupHelperTest extends NotificationTestCase {
for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) { for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) {
final StatusBarNotification sbn = getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM); final StatusBarNotification sbn = getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM);
posted.add(sbn); posted.add(sbn);
mGroupHelper.onNotificationPosted(sbn); mGroupHelper.onNotificationPosted(sbn, false);
} }
verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString()); 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)).addAutoGroup(anyString());
@@ -190,7 +198,7 @@ public class GroupHelperTest extends NotificationTestCase {
for (i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 2; i++) { for (i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 2; i++) {
final StatusBarNotification sbn = final StatusBarNotification sbn =
getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM, "app group"); 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, times(GroupHelper.AUTOGROUP_AT_COUNT - 2)).removeAutoGroup(anyString());
verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString()); verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());
@@ -199,9 +207,48 @@ public class GroupHelperTest extends NotificationTestCase {
for (; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) { for (; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) {
final StatusBarNotification sbn = final StatusBarNotification sbn =
getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM, "app group"); 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(2)).removeAutoGroup(anyString());
verify(mCallback, times(1)).removeAutoGroupSummary(anyInt(), anyString()); verify(mCallback, times(1)).removeAutoGroupSummary(anyInt(), anyString());
} }
@Test
public void testNewNotificationsAddedToAutogroup_ifOriginalNotificationsCanceled()
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, 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<String, LinkedHashSet<String>> 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());
}
} }

View File

@@ -1339,7 +1339,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
runnable.run(); runnable.run();
waitForIdle(); waitForIdle();
verify(mGroupHelper, times(1)).onNotificationPosted(any()); verify(mGroupHelper, times(1)).onNotificationPosted(any(), anyBoolean());
} }
@Test @Test
@@ -1355,7 +1355,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
runnable.run(); runnable.run();
waitForIdle(); waitForIdle();
verify(mGroupHelper, times(1)).onNotificationPosted(any()); verify(mGroupHelper, times(1)).onNotificationPosted(any(), anyBoolean());
} }
@Test @Test
@@ -1371,7 +1371,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
runnable.run(); runnable.run();
waitForIdle(); waitForIdle();
verify(mGroupHelper, never()).onNotificationPosted(any()); verify(mGroupHelper, never()).onNotificationPosted(any(), anyBoolean());
} }
@Test @Test