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

Commit da650505 authored by Valentin Iftime's avatar Valentin Iftime
Browse files

Cancel the notification group summary when the last non-regrouped child notification is canceled 

 Cache its delete intent and trigger it when all original group child notifications are canceled.

 This avoids empty summary notifications when some child notifications have their group keys overriden and the client app tracks all child notifications as being part of the original group.

Flag: EXEMPT bugfix

Test: atest GroupHelperTest
Test: atest NotificationManagerServiceTest
Bug: 422482327
Change-Id: I82e011c6bead3e456b2cdd90789861c0086b9006
parent 2c4f809a

services/core/java/com/android/server/notification/GroupHelper.java

+15 −2
Original line number Diff line number Diff line
@@ -872,7 +872,7 @@ public class GroupHelper { 
    }



    @GuardedBy("mAggregatedNotifications")

    private void addToUngroupedAndMaybeAggregate(NotificationRecord record,

    private boolean addToUngroupedAndMaybeAggregate(NotificationRecord record,

            FullyQualifiedGroupKey fullAggregateGroupKey, NotificationSectioner sectioner) {

        ArrayMap<String, NotificationAttributes> ungrouped =

                mUngroupedAbuseNotifications.getOrDefault(fullAggregateGroupKey,
@@ -901,7 +901,9 @@ public class GroupHelper { 
            }

            aggregateUngroupedNotifications(fullAggregateGroupKey, record.getKey(),

                    ungrouped, hasSummary, sectioner.mSummaryId);

            return true;

        }

        return false;

    }



    private static boolean isGroupChildBundled(final NotificationRecord record,
@@ -1099,7 +1101,18 @@ public class GroupHelper { 
                if (DEBUG) {

                    Slog.i(TAG, "isGroupSummaryWithoutChild " + summaryRecord);

                }

                addToUngroupedAndMaybeAggregate(summaryRecord, fullAggregateGroupKey, sectioner);

                boolean aggregated = addToUngroupedAndMaybeAggregate(summaryRecord,

                        fullAggregateGroupKey, sectioner);

                if (!aggregated) {

                    // Cancel the summary and cache it if does not get aggregated

                    // in order to avoid empty summaries

                    if (DEBUG) {

                        Slog.i(TAG,

                                "Empty group summary to be canceled and cached: " + summaryRecord);

                    }

                    mCallback.removeAppProvidedSummary(summaryRecord.getKey());

                    cacheCanceledSummary(summaryRecord);

                }

                return;

            }

services/tests/uiservicestests/src/com/android/server/notification/GroupHelperTest.java

+106 −1
Original line number Diff line number Diff line
@@ -1863,7 +1863,8 @@ public class GroupHelperTest extends UiServiceTestCase { 
        // Remove all child notifications from the valid group => summary without children

        Mockito.reset(mCallback);

        for (NotificationRecord r: notificationList) {

            if (r.getGroupKey().contains(groupToRemove)) {

            if (r.getGroupKey().contains(groupToRemove)

                    && r.getSbn().getNotification().isGroupChild()) {

                r.isCanceled = true;

                mGroupHelper.onNotificationRemoved(r, notificationList, false);

            }
@@ -1875,6 +1876,110 @@ public class GroupHelperTest extends UiServiceTestCase { 
        verifyNoMoreInteractions(mCallback);

    }



    @Test

    @EnableFlags({FLAG_NOTIFICATION_FORCE_GROUPING, FLAG_NOTIFICATION_FORCE_GROUP_SINGLETONS})

    public void testRemoveAllGroupChildNotifications_emptySummaryCanceled() {

        // 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 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, true);

        notificationList.add(summary);

        summaryByGroup.put(summary.getGroupKey(), summary);

        final ArrayList<NotificationRecord> childrenToRemove = new ArrayList<>();

        for (int i = 0; i < numChildren; i++) {

            NotificationRecord child = getNotificationRecord(pkg, i + 42,

                    String.valueOf(i + 42), UserHandle.SYSTEM, groupToRemove, false);

            notificationList.add(child);

            childrenToRemove.add(child);

        }

        mGroupHelper.onNotificationPostedWithDelay(summary, notificationList, summaryByGroup);

        verifyNoMoreInteractions(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, false);

        }



        // Only call onGroupedNotificationRemovedWithDelay with the summary notification

        mGroupHelper.onGroupedNotificationRemovedWithDelay(summary, notificationList,

                summaryByGroup);



        // Check that the summary was canceled

        verify(mCallback, times(1)).removeAppProvidedSummary(summary.getKey());

        // Check that nothing else was calledback

        verifyNoMoreInteractions(mCallback);

    }



    @Test

    @EnableFlags({FLAG_NOTIFICATION_FORCE_GROUPING, FLAG_NOTIFICATION_FORCE_GROUP_SINGLETONS})

    public void testRemoveAllGroupChildNotifications_emptySummaryCanceledAndCached() {

        // 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 a valid (full) group

        final int summaryId = 4242;

        final int numChildren = AUTOGROUP_AT_COUNT - 1;

        final String groupToRemove = "testRemoveGrp";

        PendingIntent summaryDeleteIntent = PendingIntent.getActivity(mContext, 1,

                new Intent(), PendingIntent.FLAG_IMMUTABLE);

        NotificationRecord summary = getNotificationRecord(pkg, summaryId,

                String.valueOf(summaryId), UserHandle.SYSTEM, groupToRemove, true);

        summary.getNotification().deleteIntent = summaryDeleteIntent;

        notificationList.add(summary);

        summaryByGroup.put(summary.getGroupKey(), summary);

        final ArrayList<NotificationRecord> childrenToRemove = new ArrayList<>();

        for (int i = 0; i < numChildren; i++) {

            NotificationRecord child = getNotificationRecord(pkg, i + 42,

                    String.valueOf(i + 42), UserHandle.SYSTEM, groupToRemove, false);

            notificationList.add(child);

            childrenToRemove.add(child);

        }



        NotificationRecord childBundled = getNotificationRecord(pkg, numChildren + 42,

                String.valueOf(numChildren + 42), UserHandle.SYSTEM, groupToRemove, false);

        final String expectedGroupKey = GroupHelper.getFullAggregateGroupKey(pkg,

                AGGREGATE_GROUP_KEY + "AlertingSection", UserHandle.SYSTEM.getIdentifier());

        childBundled.setOverrideGroupKey(expectedGroupKey);

        notificationList.add(childBundled);



        mGroupHelper.onNotificationPostedWithDelay(summary, notificationList, summaryByGroup);

        verifyNoMoreInteractions(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, false);

        }



        // Only call onGroupedNotificationRemovedWithDelay with the summary notification

        notificationList.remove(summary);

        mGroupHelper.onGroupedNotificationRemovedWithDelay(summary, notificationList,

                summaryByGroup);



        // Check that the summary was canceled

        verify(mCallback, times(1)).removeAppProvidedSummary(summary.getKey());



        // Cancel the last child

        childBundled.isCanceled = true;

        notificationList.remove(childBundled);

        mGroupHelper.onNotificationRemoved(childBundled, notificationList, true);



        // Check that the summary delete intent was triggered

        verify(mCallback).sendAppProvidedSummaryDeleteIntent(eq(pkg), eq(summaryDeleteIntent));

    }



    @Test

    @EnableFlags({FLAG_NOTIFICATION_FORCE_GROUPING, FLAG_NOTIFICATION_FORCE_GROUP_SINGLETONS})

    public void testUpdateToUngroupableSection_cleanupUngrouped() {

services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java

+87 −1
Original line number Diff line number Diff line
@@ -134,7 +134,6 @@ import static android.service.notification.NotificationListenerService.FLAG_FILT 
import static android.service.notification.NotificationListenerService.HINT_HOST_DISABLE_CALL_EFFECTS;

import static android.service.notification.NotificationListenerService.HINT_HOST_DISABLE_EFFECTS;

import static android.service.notification.NotificationListenerService.HINT_HOST_DISABLE_NOTIFICATION_EFFECTS;

import static android.service.notification.NotificationListenerService.REASON_APP_CANCEL;

import static android.service.notification.NotificationListenerService.REASON_CANCEL;

import static android.service.notification.NotificationListenerService.REASON_LOCKDOWN;

import static android.service.notification.NotificationListenerService.Ranking.USER_SENTIMENT_NEGATIVE;
@@ -2941,6 +2940,93 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
        assertThat(mService.mSummaryByGroupKey).hasSize(0);

    }













    @Test













    @EnableFlags({FLAG_NOTIFICATION_FORCE_GROUPING, FLAG_NOTIFICATION_CLASSIFICATION})













    public void testCancelGroupChildrenAfterBundling_summaryCanceled() throws Exception {













        when(mAssistants.isSameUser(any(), anyInt())).thenReturn(true);













        when(mAssistants.isServiceTokenValidLocked(any())).thenReturn(true);













        when(mAssistants.isClassificationTypeAllowed(anyInt(), anyInt())).thenReturn(true);













        when(mAssistants.isAdjustmentAllowedForPackage(anyInt(), anyString(),













                anyString())).thenReturn(true);

























        // Post grouped notifications













        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);













        PendingIntent summaryDeleteIntent = PendingIntent.getActivity(mContext, 1,













                new Intent(), PendingIntent.FLAG_IMMUTABLE);













        summary.getNotification().deleteIntent = summaryDeleteIntent;













        mService.addNotification(summary);













        final String originalGroupKey = summary.getGroupKey();













        assertThat(mService.mSummaryByGroupKey).containsEntry(originalGroupKey, summary);

























        doAnswer(invocationOnMock -> {













            RankingReconsideration recon =













                    ((RankingReconsideration) invocationOnMock.getArguments()[0]);













            final NotificationRecord r = mService.mNotificationsByKey.get(recon.getKey());













            if (r != null) {













                recon.applyChangesLocked(r);













            }













            return null;













        }).when(mRankingHandler).requestReconsideration(any());

























        // Classify a child notification into the NEWS bundle













        final String keyToUnbundle = r1.getKey();













        final boolean hasOriginalSummary = true;













        Bundle signals = new Bundle();













        signals.putInt(Adjustment.KEY_TYPE, Adjustment.TYPE_NEWS);













        Adjustment adjustment = new Adjustment(r1.getSbn().getPackageName(), r1.getKey(), signals,













                "", r1.getUser().getIdentifier());













        mBinderService.applyAdjustmentFromAssistant(null, adjustment);













        mService.handleRankingSort();













        waitForIdle();

























        // Check that the notification was bundled and a group summary was created













        assertThat(mService.mSummaryByGroupKey).hasSize(2);













        assertThat(r1.getChannel().getId()).isEqualTo(NEWS_ID);













        assertThat(r1.getBundleType()).isEqualTo(Adjustment.TYPE_NEWS);













        assertThat(r1.getGroupKey()).isNotEqualTo(originalGroupKey);













        final NotificationRecord bundleSummary = mService.mSummaryByGroupKey.get(r1.getGroupKey());













        assertThat(bundleSummary).isNotNull();













        assertThat(GroupHelper.isAggregatedGroup(bundleSummary)).isTrue();

























        // Cancel unbundled child notification













        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 the summary was canceled













        verify(mGroupHelper, times(1)).onNotificationRemoved(eq(r2), any(), eq(false));













        verify(mGroupHelper, times(1)).onGroupedNotificationRemovedWithDelay(eq(summary), any(),













                any());













        verify(mGroupHelper, times(1)).onNotificationRemoved(eq(summary), any(), eq(false));













        assertThat(mService.mSummaryByGroupKey).doesNotContainKey(originalGroupKey);

























        // Cancel the remaining (bundled) child notification













        final NotificationVisibility nv = NotificationVisibility.obtain(r1.getKey(), 1, 2, true);













        mService.mNotificationDelegate.onNotificationClear(mUid, 0, r1.getSbn().getPackageName(),













                r1.getUserId(), r1.getKey(), NotificationStats.DISMISSAL_SHADE,













                NotificationStats.DISMISS_SENTIMENT_POSITIVE, nv, false);













        waitForIdle();

























        // Check that the original summary delete intent was triggered













        verify(mGroupHelper, times(1)).onNotificationRemoved(eq(r1), any(), eq(true));

























        // Check that all notifications were canceled













        assertThat(mService.mNotificationList).isEmpty();













    }



























    @Test















    @EnableFlags(FLAG_NOTIFICATION_FORCE_GROUPING)















    public void testUpdateChannel_notifyGroupHelper() throws Exception {