diff options
| author | 2024-11-19 15:51:19 +0100 | |
|---|---|---|
| committer | 2024-11-20 14:22:52 +0000 | |
| commit | 2dca76f0d6ffbc7f88e15029a5961b954588f67e (patch) | |
| tree | 3263fea9a903841f8a99dc229567c6320479bcb6 | |
| parent | 6d65789125c698d99b9a0a956c4c3332de5d1e3b (diff) | |
Trigger notification force grouping check on group child removal
Fixes the scenario where an app cancels children notifications only, leaving a singleton group or summary-only group.
Triggers autogrouping after some delay, so that the app has a chance to cancel/post notifications and create a valid group.
Only the group summary is used to check for invalid grouping, as the other scenarios are covered by the existing implementation.
Flag: android.service.notification.notification_force_grouping
Test: atest NotificationManagerServiceTest
Test: atest GroupHelperTest
Bug: 380004477
Bug: 379467923
Change-Id: Id3f8c7cf27962699c31c75d37e971bdf86017768
4 files changed, 375 insertions, 30 deletions
diff --git a/services/core/java/com/android/server/notification/GroupHelper.java b/services/core/java/com/android/server/notification/GroupHelper.java index 7fd962003ce9..f9145514a4e0 100644 --- a/services/core/java/com/android/server/notification/GroupHelper.java +++ b/services/core/java/com/android/server/notification/GroupHelper.java @@ -755,36 +755,7 @@ public class GroupHelper { Log.i(TAG, "isGroupChildWithoutSummary OR isGroupSummaryWithoutChild" + record); } - - ArrayMap<String, NotificationAttributes> ungrouped = - mUngroupedAbuseNotifications.getOrDefault(fullAggregateGroupKey, - new ArrayMap<>()); - ungrouped.put(record.getKey(), new NotificationAttributes( - record.getFlags(), - record.getNotification().getSmallIcon(), - record.getNotification().color, - record.getNotification().visibility, - record.getNotification().getGroupAlertBehavior(), - record.getChannel().getId())); - mUngroupedAbuseNotifications.put(fullAggregateGroupKey, ungrouped); - // Create/update summary and group if >= mAutoGroupAtCount notifications - // or if aggregate group exists - boolean hasSummary = !mAggregatedNotifications.getOrDefault(fullAggregateGroupKey, - new ArrayMap<>()).isEmpty(); - if (ungrouped.size() >= mAutoGroupAtCount || hasSummary) { - if (DEBUG) { - if (ungrouped.size() >= mAutoGroupAtCount) { - Log.i(TAG, - "Found >=" + mAutoGroupAtCount - + " ungrouped notifications => force grouping"); - } else { - Log.i(TAG, "Found aggregate summary => force grouping"); - } - } - aggregateUngroupedNotifications(fullAggregateGroupKey, sbn.getKey(), - ungrouped, hasSummary, sectioner.mSummaryId); - } - + addToUngroupedAndMaybeAggregate(record, fullAggregateGroupKey, sectioner); return; } @@ -815,6 +786,38 @@ public class GroupHelper { } } + @GuardedBy("mAggregatedNotifications") + private void addToUngroupedAndMaybeAggregate(NotificationRecord record, + FullyQualifiedGroupKey fullAggregateGroupKey, NotificationSectioner sectioner) { + ArrayMap<String, NotificationAttributes> ungrouped = + mUngroupedAbuseNotifications.getOrDefault(fullAggregateGroupKey, + new ArrayMap<>()); + ungrouped.put(record.getKey(), new NotificationAttributes( + record.getFlags(), + record.getNotification().getSmallIcon(), + record.getNotification().color, + record.getNotification().visibility, + record.getNotification().getGroupAlertBehavior(), + record.getChannel().getId())); + mUngroupedAbuseNotifications.put(fullAggregateGroupKey, ungrouped); + // Create/update summary and group if >= mAutoGroupAtCount notifications + // or if aggregate group exists + boolean hasSummary = !mAggregatedNotifications.getOrDefault(fullAggregateGroupKey, + new ArrayMap<>()).isEmpty(); + if (ungrouped.size() >= mAutoGroupAtCount || hasSummary) { + if (DEBUG) { + if (ungrouped.size() >= mAutoGroupAtCount) { + Slog.i(TAG, "Found >=" + mAutoGroupAtCount + + " ungrouped notifications => force grouping"); + } else { + Slog.i(TAG, "Found aggregate summary => force grouping"); + } + } + aggregateUngroupedNotifications(fullAggregateGroupKey, record.getKey(), + ungrouped, hasSummary, sectioner.mSummaryId); + } + } + private static boolean isGroupChildBundled(final NotificationRecord record, final Map<String, NotificationRecord> summaryByGroupKey) { final StatusBarNotification sbn = record.getSbn(); @@ -897,6 +900,73 @@ public class GroupHelper { } } + /** + * Called when a child notification is removed, after some delay, so that this helper can + * trigger a forced grouping if the group has become sparse/singleton + * or only the summary is left. + * + * see also {@link #onNotificationPostedWithDelay(NotificationRecord, List, Map)} + * + * @param summaryRecord the group summary of the notification that was removed + * @param notificationList the full notification list from NotificationManagerService + * @param summaryByGroupKey the map of group summaries from NotificationManagerService + */ + @FlaggedApi(android.service.notification.Flags.FLAG_NOTIFICATION_FORCE_GROUPING) + protected void onGroupedNotificationRemovedWithDelay(final NotificationRecord summaryRecord, + final List<NotificationRecord> notificationList, + final Map<String, NotificationRecord> summaryByGroupKey) { + final StatusBarNotification sbn = summaryRecord.getSbn(); + if (!sbn.isAppGroup()) { + return; + } + + if (summaryRecord.isCanceled) { + return; + } + + if (mIsTestHarnessExempted) { + return; + } + + final NotificationSectioner sectioner = getSection(summaryRecord); + if (sectioner == null) { + if (DEBUG) { + Slog.i(TAG, + "Skipping autogrouping for " + summaryRecord + " no valid section found."); + } + return; + } + + final String pkgName = sbn.getPackageName(); + final FullyQualifiedGroupKey fullAggregateGroupKey = new FullyQualifiedGroupKey( + summaryRecord.getUserId(), pkgName, sectioner); + + // This notification is already aggregated + if (summaryRecord.getGroupKey().equals(fullAggregateGroupKey.toString())) { + return; + } + + synchronized (mAggregatedNotifications) { + if (isGroupSummaryWithoutChildren(summaryRecord, notificationList)) { + if (DEBUG) { + Slog.i(TAG, "isGroupSummaryWithoutChild " + summaryRecord); + } + addToUngroupedAndMaybeAggregate(summaryRecord, fullAggregateGroupKey, sectioner); + return; + } + + // Check if notification removal turned this group into a sparse/singleton group + if (Flags.notificationForceGroupSingletons()) { + try { + groupSparseGroups(summaryRecord, notificationList, summaryByGroupKey, sectioner, + fullAggregateGroupKey); + } catch (Throwable e) { + Slog.wtf(TAG, "Failed to group sparse groups", e); + } + } + } + } + private record NotificationMoveOp(NotificationRecord record, FullyQualifiedGroupKey oldGroup, FullyQualifiedGroupKey newGroup) { } diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 5182dfe60fcc..acfe09351e25 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -10500,6 +10500,27 @@ public class NotificationManagerService extends SystemService { mGroupHelper.onNotificationRemoved(r, mNotificationList); } }); + + // Wait 3 seconds so that the app has a chance to cancel/post + // a group summary or children + final NotificationRecord groupSummary = mSummaryByGroupKey.get(r.getGroupKey()); + if (groupSummary != null + && !GroupHelper.isAggregatedGroup(groupSummary) + && !groupSummary.getKey().equals(canceledKey)) { + // We only care about app-provided valid group summaries + final String summaryKey = groupSummary.getKey(); + mHandler.removeCallbacksAndEqualMessages(summaryKey); + mHandler.postDelayed(() -> { + synchronized (mNotificationLock) { + NotificationRecord summaryRecord = mNotificationsByKey.get( + summaryKey); + if (summaryRecord != null) { + mGroupHelper.onGroupedNotificationRemovedWithDelay( + summaryRecord, mNotificationList, mSummaryByGroupKey); + } + } + }, summaryKey, DELAY_FORCE_REGROUP_TIME); + } } else { mHandler.post(new Runnable() { @Override 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 6af65423415b..dd278fccad14 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/GroupHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/GroupHelperTest.java @@ -2170,6 +2170,174 @@ public class GroupHelperTest extends UiServiceTestCase { } @Test + @EnableFlags({FLAG_NOTIFICATION_FORCE_GROUPING, FLAG_NOTIFICATION_FORCE_GROUP_SINGLETONS}) + public void testRemoveChildNotification_summaryForceGrouped() { + // Check that removing all child notifications from a group will trigger empty summary + // force grouping re-evaluation + final List<NotificationRecord> notificationList = new ArrayList<>(); + final ArrayMap<String, NotificationRecord> summaryByGroup = new ArrayMap<>(); + final String pkg = "package"; + // Post summaries without children, below the force grouping limit + for (int i = 0; i < AUTOGROUP_AT_COUNT - 1; i++) { + NotificationRecord summary = getNotificationRecord(pkg, i + 42, String.valueOf(i + 42), + UserHandle.SYSTEM, "testGrp " + i, true); + notificationList.add(summary); + mGroupHelper.onNotificationPostedWithDelay(summary, notificationList, summaryByGroup); + } + // Post a valid (full) group + final int summaryId = 4242; + final int numChildren = 3; + final ArrayList<NotificationRecord> childrenToRemove = new ArrayList<>(); + NotificationRecord summary = getNotificationRecord(pkg, summaryId, + String.valueOf(summaryId), UserHandle.SYSTEM, "testGrp " + summaryId, true); + notificationList.add(summary); + summaryByGroup.put(summary.getGroupKey(), summary); + for (int i = 0; i < numChildren; i++) { + NotificationRecord child = getNotificationRecord(pkg, summaryId + 42, + String.valueOf(i + 42), UserHandle.SYSTEM, "testGrp " + summaryId, false); + notificationList.add(child); + // schedule all children for removal + childrenToRemove.add(child); + } + mGroupHelper.onNotificationPostedWithDelay(summary, notificationList, summaryByGroup); + verifyZeroInteractions(mCallback); + + // Remove all child notifications from the valid group => summary without children + Mockito.reset(mCallback); + for (NotificationRecord r: childrenToRemove) { + notificationList.remove(r); + mGroupHelper.onNotificationRemoved(r, notificationList); + } + // Only call onGroupedNotificationRemovedWithDelay with the summary notification + mGroupHelper.onGroupedNotificationRemovedWithDelay(summary, notificationList, + summaryByGroup); + + // Check that the summaries were force grouped + final String expectedGroupKey = GroupHelper.getFullAggregateGroupKey(pkg, + AGGREGATE_GROUP_KEY + "AlertingSection", UserHandle.SYSTEM.getIdentifier()); + verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString(), + eq(expectedGroupKey), anyInt(), eq(getNotificationAttributes(BASE_FLAGS))); + verify(mCallback, times(AUTOGROUP_AT_COUNT)).addAutoGroup(anyString(), + eq(expectedGroupKey), eq(true)); + verify(mCallback, never()).removeAutoGroup(anyString()); + verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString(), anyString()); + verify(mCallback, never()).updateAutogroupSummary(anyInt(), anyString(), anyString(), + any()); + } + + @Test + @EnableFlags({FLAG_NOTIFICATION_FORCE_GROUPING, FLAG_NOTIFICATION_FORCE_GROUP_SINGLETONS}) + public void testRemoveChildNotification_groupBecomesSingleton() { + // Check that removing child notifications from a group will trigger singleton force + // grouping re-evaluation + final List<NotificationRecord> notificationList = new ArrayList<>(); + final ArrayMap<String, NotificationRecord> summaryByGroup = new ArrayMap<>(); + final String pkg = "package"; + // Post singleton groups, under forced group limit + for (int i = 0; i < AUTOGROUP_SINGLETONS_AT_COUNT - 1; i++) { + NotificationRecord summary = getNotificationRecord(pkg, i, + String.valueOf(i), UserHandle.SYSTEM, "testGrp " + i, true); + notificationList.add(summary); + NotificationRecord child = getNotificationRecord(pkg, i + 42, + String.valueOf(i + 42), UserHandle.SYSTEM, "testGrp " + i, false); + notificationList.add(child); + summaryByGroup.put(summary.getGroupKey(), summary); + mGroupHelper.onNotificationPostedWithDelay(child, notificationList, summaryByGroup); + mGroupHelper.onNotificationPostedWithDelay(summary, notificationList, summaryByGroup); + } + // Post a valid (full) group + final int summaryId = 4242; + final int numChildren = 3; + final ArrayList<NotificationRecord> childrenToRemove = new ArrayList<>(); + NotificationRecord summary = getNotificationRecord(pkg, summaryId, + String.valueOf(summaryId), UserHandle.SYSTEM, "testGrp " + summaryId, true); + notificationList.add(summary); + summaryByGroup.put(summary.getGroupKey(), summary); + for (int i = 0; i < numChildren; i++) { + NotificationRecord child = getNotificationRecord(pkg, summaryId + 42, + String.valueOf(i + 42), UserHandle.SYSTEM, "testGrp " + summaryId, false); + notificationList.add(child); + + // schedule all children except one for removal + if (i < numChildren - 1) { + childrenToRemove.add(child); + } + } + mGroupHelper.onNotificationPostedWithDelay(summary, notificationList, summaryByGroup); + verifyZeroInteractions(mCallback); + + // Remove some child notifications from the valid group, transform into a singleton group + Mockito.reset(mCallback); + for (NotificationRecord r: childrenToRemove) { + notificationList.remove(r); + mGroupHelper.onNotificationRemoved(r, notificationList); + } + // Only call onGroupedNotificationRemovedWithDelay with the summary notification + mGroupHelper.onGroupedNotificationRemovedWithDelay(summary, notificationList, + summaryByGroup); + + // Check that the singleton groups were force grouped + final String expectedGroupKey = GroupHelper.getFullAggregateGroupKey(pkg, + AGGREGATE_GROUP_KEY + "AlertingSection", UserHandle.SYSTEM.getIdentifier()); + verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString(), + eq(expectedGroupKey), anyInt(), eq(getNotificationAttributes(BASE_FLAGS))); + verify(mCallback, times(AUTOGROUP_SINGLETONS_AT_COUNT)).addAutoGroup(anyString(), + eq(expectedGroupKey), eq(true)); + verify(mCallback, never()).removeAutoGroup(anyString()); + verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString(), anyString()); + verify(mCallback, never()).updateAutogroupSummary(anyInt(), anyString(), anyString(), + any()); + verify(mCallback, times(AUTOGROUP_SINGLETONS_AT_COUNT)).removeAppProvidedSummary( + anyString()); + } + + @Test + @EnableFlags({FLAG_NOTIFICATION_FORCE_GROUPING, FLAG_NOTIFICATION_FORCE_GROUP_SINGLETONS}) + public void testRemoveAllGroupNotifications_noForceGrouping() { + // Check that removing all notifications from a group will not trigger any force grouping + // re-evaluation + final List<NotificationRecord> notificationList = new ArrayList<>(); + final ArrayMap<String, NotificationRecord> summaryByGroup = new ArrayMap<>(); + final String pkg = "package"; + // Post summaries without children, below the force grouping limit + for (int i = 0; i < AUTOGROUP_AT_COUNT - 1; i++) { + NotificationRecord summary = getNotificationRecord(pkg, i + 42, String.valueOf(i + 42), + UserHandle.SYSTEM, "testGrp " + i, true); + notificationList.add(summary); + mGroupHelper.onNotificationPostedWithDelay(summary, notificationList, summaryByGroup); + } + // Post a valid (full) group + final int summaryId = 4242; + final int numChildren = 3; + final String groupToRemove = "testRemoveGrp"; + NotificationRecord summary = getNotificationRecord(pkg, summaryId, + String.valueOf(summaryId), UserHandle.SYSTEM, groupToRemove + summaryId, true); + notificationList.add(summary); + summaryByGroup.put(summary.getGroupKey(), summary); + for (int i = 0; i < numChildren; i++) { + NotificationRecord child = getNotificationRecord(pkg, summaryId + 42, + String.valueOf(i + 42), UserHandle.SYSTEM, groupToRemove + summaryId, false); + notificationList.add(child); + } + mGroupHelper.onNotificationPostedWithDelay(summary, notificationList, summaryByGroup); + verifyZeroInteractions(mCallback); + + // Remove all child notifications from the valid group => summary without children + Mockito.reset(mCallback); + for (NotificationRecord r: notificationList) { + if (r.getGroupKey().contains(groupToRemove)) { + r.isCanceled = true; + mGroupHelper.onNotificationRemoved(r, notificationList); + } + } + // Only call onGroupedNotificationRemovedWithDelay with the summary notification + mGroupHelper.onGroupedNotificationRemovedWithDelay(summary, notificationList, + summaryByGroup); + // Check that nothing was force grouped + verifyZeroInteractions(mCallback); + } + + @Test @EnableFlags(FLAG_NOTIFICATION_FORCE_GROUPING) public void testMoveAggregateGroups_updateChannel() { final String pkg = "package"; diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index 704c1b858b8d..7462f76e7b96 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -3094,6 +3094,92 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { } @Test + @EnableFlags({FLAG_NOTIFICATION_FORCE_GROUPING, + android.app.Flags.FLAG_CHECK_AUTOGROUP_BEFORE_POST}) + public void testScheduleGroupHelperWithDelay_onChildNotificationCanceled() throws Exception { + // Post summary + 2 child notification + final String originalGroupName = "originalGroup"; + final int summaryId = 0; + final NotificationRecord r1 = generateNotificationRecord(mTestNotificationChannel, + summaryId + 1, originalGroupName, false); + mService.addNotification(r1); + final NotificationRecord r2 = generateNotificationRecord(mTestNotificationChannel, + summaryId + 2, originalGroupName, false); + mService.addNotification(r2); + final NotificationRecord summary = generateNotificationRecord(mTestNotificationChannel, + summaryId, originalGroupName, true); + mService.addNotification(summary); + final String originalGroupKey = summary.getGroupKey(); + assertThat(mService.mSummaryByGroupKey).containsEntry(originalGroupKey, summary); + + // Cancel the child notifications + mBinderService.cancelNotificationWithTag(r1.getSbn().getPackageName(), + r1.getSbn().getPackageName(), r1.getSbn().getTag(), + r1.getSbn().getId(), r1.getSbn().getUserId()); + waitForIdle(); + + mBinderService.cancelNotificationWithTag(r2.getSbn().getPackageName(), + r2.getSbn().getPackageName(), r2.getSbn().getTag(), + r2.getSbn().getId(), r2.getSbn().getUserId()); + waitForIdle(); + + mTestableLooper.moveTimeForward(DELAY_FORCE_REGROUP_TIME); + waitForIdle(); + + // Check that onGroupedNotificationRemovedWithDelay was called only once + verify(mGroupHelper, times(1)).onNotificationRemoved(eq(r1), any()); + verify(mGroupHelper, times(1)).onNotificationRemoved(eq(r2), any()); + verify(mGroupHelper, times(1)).onGroupedNotificationRemovedWithDelay(eq(summary), any(), + any()); + } + + @Test + @EnableFlags({FLAG_NOTIFICATION_FORCE_GROUPING, + android.app.Flags.FLAG_CHECK_AUTOGROUP_BEFORE_POST}) + public void testCleanupScheduleGroupHelperWithDelay_onAllNotificationCanceled() + throws Exception { + // Post summary + 2 child notification + final String originalGroupName = "originalGroup"; + final int summaryId = 0; + final NotificationRecord r1 = generateNotificationRecord(mTestNotificationChannel, + summaryId + 1, originalGroupName, false); + mService.addNotification(r1); + final NotificationRecord r2 = generateNotificationRecord(mTestNotificationChannel, + summaryId + 2, originalGroupName, false); + mService.addNotification(r2); + final NotificationRecord summary = generateNotificationRecord(mTestNotificationChannel, + summaryId, originalGroupName, true); + mService.addNotification(summary); + final String originalGroupKey = summary.getGroupKey(); + assertThat(mService.mSummaryByGroupKey).containsEntry(originalGroupKey, summary); + + // Cancel all notifications: children + summary + mBinderService.cancelNotificationWithTag(r1.getSbn().getPackageName(), + r1.getSbn().getPackageName(), r1.getSbn().getTag(), + r1.getSbn().getId(), r1.getSbn().getUserId()); + waitForIdle(); + + mBinderService.cancelNotificationWithTag(r2.getSbn().getPackageName(), + r2.getSbn().getPackageName(), r2.getSbn().getTag(), + r2.getSbn().getId(), r2.getSbn().getUserId()); + waitForIdle(); + + mBinderService.cancelNotificationWithTag(summary.getSbn().getPackageName(), + summary.getSbn().getPackageName(), summary.getSbn().getTag(), + summary.getSbn().getId(), summary.getSbn().getUserId()); + waitForIdle(); + + mTestableLooper.moveTimeForward(DELAY_FORCE_REGROUP_TIME); + waitForIdle(); + + // Check that onGroupedNotificationRemovedWithDelay was never called: summary was canceled + verify(mGroupHelper, times(1)).onNotificationRemoved(eq(r1), any()); + verify(mGroupHelper, times(1)).onNotificationRemoved(eq(r2), any()); + verify(mGroupHelper, times(1)).onNotificationRemoved(eq(summary), any()); + verify(mGroupHelper, never()).onGroupedNotificationRemovedWithDelay(any(), any(), any()); + } + + @Test public void testCancelAllNotifications_IgnoreForegroundService() throws Exception { when(mAmi.applyForegroundServiceNotification( any(), anyString(), anyInt(), anyString(), anyInt())).thenReturn(SHOW_IMMEDIATELY); |