Merge "When a new group summary is added, clean up any ungrouped children" into main
diff --git a/services/core/java/com/android/server/notification/GroupHelper.java b/services/core/java/com/android/server/notification/GroupHelper.java
index 6e5308e..3f4df1d 100644
--- a/services/core/java/com/android/server/notification/GroupHelper.java
+++ b/services/core/java/com/android/server/notification/GroupHelper.java
@@ -1002,8 +1002,7 @@
private FullyQualifiedGroupKey getSectionGroupKeyWithFallback(final NotificationRecord record) {
final NotificationSectioner sectioner = getSection(record);
if (sectioner != null) {
- return new FullyQualifiedGroupKey(record.getUserId(), record.getSbn().getPackageName(),
- sectioner);
+ return FullyQualifiedGroupKey.forRecord(record, sectioner);
} else {
return getPreviousValidSectionKey(record);
}
@@ -1105,6 +1104,49 @@
}
}
+ /**
+ * Called when a group summary is posted. If there are any ungrouped notifications that are
+ * in that group, remove them as they are no longer candidates for autogrouping.
+ *
+ * @param summaryRecord the NotificationRecord for the newly posted group summary
+ * @param notificationList the full notification list from NotificationManagerService
+ */
+ @FlaggedApi(android.service.notification.Flags.FLAG_NOTIFICATION_FORCE_GROUPING)
+ protected void onGroupSummaryAdded(final NotificationRecord summaryRecord,
+ final List<NotificationRecord> notificationList) {
+ String groupKey = summaryRecord.getSbn().getGroup();
+ synchronized (mAggregatedNotifications) {
+ final NotificationSectioner sectioner = getSection(summaryRecord);
+ if (sectioner == null) {
+ Slog.w(TAG, "onGroupSummaryAdded " + summaryRecord + ": no valid section found");
+ return;
+ }
+
+ FullyQualifiedGroupKey aggregateGroupKey = FullyQualifiedGroupKey.forRecord(
+ summaryRecord, sectioner);
+ ArrayMap<String, NotificationAttributes> ungrouped =
+ mUngroupedAbuseNotifications.getOrDefault(aggregateGroupKey,
+ new ArrayMap<>());
+ if (ungrouped.isEmpty()) {
+ // don't bother looking through the notification list if there are no pending
+ // ungrouped notifications in this section (likely to be the most common case)
+ return;
+ }
+
+ // Look through full notification list for any notifications belonging to this group;
+ // remove from ungrouped map if needed, as the presence of the summary means they will
+ // now be grouped
+ for (NotificationRecord r : notificationList) {
+ if (!r.getNotification().isGroupSummary()
+ && groupKey.equals(r.getSbn().getGroup())
+ && ungrouped.containsKey(r.getKey())) {
+ ungrouped.remove(r.getKey());
+ }
+ }
+ mUngroupedAbuseNotifications.put(aggregateGroupKey, ungrouped);
+ }
+ }
+
private record NotificationMoveOp(NotificationRecord record, FullyQualifiedGroupKey oldGroup,
FullyQualifiedGroupKey newGroup) { }
@@ -1496,8 +1538,8 @@
private boolean isNotificationAggregatedInSection(NotificationRecord record,
NotificationSectioner sectioner) {
- final FullyQualifiedGroupKey fullAggregateGroupKey = new FullyQualifiedGroupKey(
- record.getUserId(), record.getSbn().getPackageName(), sectioner);
+ final FullyQualifiedGroupKey fullAggregateGroupKey = FullyQualifiedGroupKey.forRecord(
+ record, sectioner);
return record.getGroupKey().equals(fullAggregateGroupKey.toString());
}
@@ -1895,6 +1937,12 @@
this(userId, pkg, AGGREGATE_GROUP_KEY + (sectioner != null ? sectioner.mName : ""));
}
+ static FullyQualifiedGroupKey forRecord(NotificationRecord record,
+ @Nullable NotificationSectioner sectioner) {
+ return new FullyQualifiedGroupKey(record.getUserId(), record.getSbn().getPackageName(),
+ sectioner);
+ }
+
@Override
public String toString() {
return userId + "|" + pkg + "|" + "g:" + groupName;
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index 0f1d28d..bfe0d32 100644
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -10196,6 +10196,12 @@
}
if (isSummary) {
mSummaryByGroupKey.put(group, r);
+
+ if (notificationForceGrouping()) {
+ // If any formerly-ungrouped notifications will be grouped by this summary, update
+ // accordingly.
+ mGroupHelper.onGroupSummaryAdded(r, mNotificationList);
+ }
}
FlagChecker childrenFlagChecker = (flags) -> {
diff --git a/services/tests/uiservicestests/src/com/android/server/notification/GroupHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/GroupHelperTest.java
index 4eac1d1..a9759c8 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/GroupHelperTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/GroupHelperTest.java
@@ -102,7 +102,9 @@
import platform.test.runner.parameterized.Parameters;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
@SmallTest
@SuppressLint("GuardedBy") // It's ok for this test to access guarded methods from the class.
@@ -4443,4 +4445,97 @@
verify(mCallback).sendAppProvidedSummaryDeleteIntent(eq(pkg),
eq(deleteIntentofFirstSummary));
}
+
+ @Test
+ @EnableFlags(FLAG_NOTIFICATION_FORCE_GROUPING)
+ public void testGroupSummaryAdded_hadUngroupedNotif_doesNotAutogroup() {
+ // Scenario:
+ // * child notification posted before summary; added to ungrouped notifications
+ // * summary posted, so now the child has a group summary and is no longer "ungrouped
+ // * another ungrouped notification is posted
+ // Confirm that the first notification (that now has a summary) is not autogrouped.
+
+ // Bookkeeping items
+ List<NotificationRecord> notifList = new ArrayList<>();
+ Map<String, NotificationRecord> summaryByGroupKey = new HashMap<>();
+
+ // Setup: post AUTOGROUP_AT_COUNT - 2 notifications so that the next notification would not
+ // trigger autogrouping, but the one after that would
+ for (int i = 0; i < AUTOGROUP_AT_COUNT - 2; i++) {
+ NotificationRecord child = getNotificationRecord(mPkg, i, "", mUser, "group" + i, false,
+ IMPORTANCE_DEFAULT);
+ notifList.add(child);
+ mGroupHelper.onNotificationPostedWithDelay(child, notifList, summaryByGroupKey);
+ }
+
+ // Group child: posted enough before its associated summary to be put in the "ungrouped"
+ // set of notifications
+ NotificationRecord groupChild = getNotificationRecord(mPkg, AUTOGROUP_AT_COUNT - 2, "",
+ mUser, "specialGroup", false, IMPORTANCE_DEFAULT);
+ notifList.add(groupChild);
+ mGroupHelper.onNotificationPostedWithDelay(groupChild, notifList, summaryByGroupKey);
+
+ // Group summary: posted after child 1
+ NotificationRecord groupSummary = getNotificationRecord(mPkg, AUTOGROUP_AT_COUNT - 1, "",
+ mUser, "specialGroup", true, IMPORTANCE_DEFAULT);
+ notifList.add(groupSummary);
+ summaryByGroupKey.put(groupSummary.getSbn().getGroupKey(), groupSummary);
+ mGroupHelper.onGroupSummaryAdded(groupSummary, notifList);
+ mGroupHelper.onNotificationPostedWithDelay(groupSummary, notifList, summaryByGroupKey);
+
+ // One more notification posted to the group; because its summary already exists, it should
+ // never be counted as an "ungrouped" notification
+ NotificationRecord groupChild2 = getNotificationRecord(mPkg, AUTOGROUP_AT_COUNT, "",
+ mUser, "specialGroup", false, IMPORTANCE_DEFAULT);
+ notifList.add(groupChild2);
+ mGroupHelper.onNotificationPostedWithDelay(groupChild2, notifList, summaryByGroupKey);
+
+ // Now one more ungrouped notification; this would have put the number of "ungrouped"
+ // notifications above the limit if the first groupChild notification were left ungrouped
+ NotificationRecord extra = getNotificationRecord(mPkg, AUTOGROUP_AT_COUNT + 1, "", mUser,
+ "yetAnotherGroup", false, IMPORTANCE_DEFAULT);
+ notifList.add(extra);
+ mGroupHelper.onNotificationPostedWithDelay(extra, notifList, summaryByGroupKey);
+
+ // no autogrouping should have occurred
+ verifyZeroInteractions(mCallback);
+ }
+
+ @Test
+ @EnableFlags(FLAG_NOTIFICATION_FORCE_GROUPING)
+ public void testGroupSummaryAdded_onlyUnrelatedGroupedNotifs() {
+ // If all of the existing ungrouped notifications have nothing to do with the summary
+ // they should still get grouped as needed.
+ List<NotificationRecord> notifList = new ArrayList<>();
+ Map<String, NotificationRecord> summaryByGroupKey = new HashMap<>();
+
+ // Post 1 fewer than the autogroupable notifications, each associated with a different
+ // group without a summary.
+ for (int i = 0; i < AUTOGROUP_AT_COUNT - 1; i++) {
+ NotificationRecord child = getNotificationRecord(mPkg, i, "", mUser, "group" + i, false,
+ IMPORTANCE_DEFAULT);
+ notifList.add(child);
+ mGroupHelper.onNotificationPostedWithDelay(child, notifList, summaryByGroupKey);
+ }
+
+ // At this point we do not yet expect autogrouping.
+ // Add a group summary that is a summary associated with none of the above notifications.
+ // Because this gets considered a "summary without children", all of these notifications
+ // should now be autogrouped.
+ NotificationRecord summary = getNotificationRecord(mPkg, AUTOGROUP_AT_COUNT, "", mUser,
+ "summaryGroup", true, IMPORTANCE_DEFAULT);
+ notifList.add(summary);
+ summaryByGroupKey.put(summary.getSbn().getKey(), summary);
+ mGroupHelper.onGroupSummaryAdded(summary, notifList);
+ mGroupHelper.onNotificationPostedWithDelay(summary, notifList, summaryByGroupKey);
+
+ // all of the above posted notifications should be autogrouped
+ String expectedGroupKey = getExpectedAutogroupKey(
+ getNotificationRecord(mPkg, 0, String.valueOf(0), mUser));
+ verify(mCallback, times(1)).addAutoGroupSummary(
+ anyInt(), eq(mPkg), anyString(), eq(expectedGroupKey),
+ anyInt(), eq(getNotificationAttributes(BASE_FLAGS)));
+ verify(mCallback, times(AUTOGROUP_AT_COUNT)).addAutoGroup(anyString(),
+ eq(expectedGroupKey), anyBoolean());
+ }
}