When a new group summary is added, clean up any ungrouped children
If a child notification of a group is added sufficiently before its summary, GroupHelper keeps it in the list of ungrouped child notifications without summaries. If it is still ungrouped when the summary comes in, then it will be grouped in the UI and should not then be re-autogrouped later.
In practice, this means that if a group summary is posted while a child is still ungrouped, it will be grouped as the app intended; if the child has already been autogrouped at that point with other app notifications, it will remain in the aggregated autogroup.
Bug: 394020651
Test: manual with test scenario, GroupHelperTest
Flag: android.service.notification.notification_force_grouping
Change-Id: I88dfcc1439e81421bde2be97116e93b29e5d2e32
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 60371d7..b51e556 100644
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -10191,6 +10191,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());
+ }
}