Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2dca76f0 authored by Valentin Iftime's avatar Valentin Iftime Committed by Iavor-Valentin Iftime
Browse files

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
parent 6d657891
Loading
Loading
Loading
Loading
+100 −30
Original line number Diff line number Diff line
@@ -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) { }

+21 −0
Original line number Diff line number Diff line
@@ -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
+168 −0
Original line number Diff line number Diff line
@@ -2169,6 +2169,174 @@ public class GroupHelperTest extends UiServiceTestCase {
                eq(expectedGroupKey));
    }

    @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() {
+86 −0
Original line number Diff line number Diff line
@@ -3093,6 +3093,92 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
        assertThat(mService.mNotificationList.get(0).getFlags() & FLAG_GROUP_SUMMARY).isEqualTo(0);
    }
    @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(