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

Commit a13b3e25 authored by Julia Reynolds's avatar Julia Reynolds
Browse files

Group new free notis if autogroup summary exists

In the case where an app posted enough notifications to
get an autogrpup summary, but then canceled most of them,
add all new group-less notifications to the autogroup summary
even if the total number of notifications currently posted
by the app is < the autogroup trigger number

Test: runtest systemui-notification, cts
Change-Id: I6f7a8a0140abcff2b0f78eacfbf23cfab12897fa
Fixes: 64158706
parent edf33843
Loading
Loading
Loading
Loading
+5 −3
Original line number Diff line number Diff line
@@ -38,14 +38,14 @@ public class GroupHelper {
    private final Callback mCallback;

    // Map of user : <Map of package : notification keys>. Only contains notifications that are not
    // groupd by the app (aka no group or sort key).
    // grouped by the app (aka no group or sort key).
    Map<Integer, Map<String, LinkedHashSet<String>>> mUngroupedNotifications = new HashMap<>();

    public GroupHelper(Callback callback) {;
        mCallback = callback;
    }

    public void onNotificationPosted(StatusBarNotification sbn) {
    public void onNotificationPosted(StatusBarNotification sbn, boolean autogroupSummaryExists) {
        if (DEBUG) Log.i(TAG, "POSTED " + sbn.getKey());
        try {
            List<String> notificationsToGroup = new ArrayList<>();
@@ -68,7 +68,8 @@ public class GroupHelper {
                    notificationsForPackage.add(sbn.getKey());
                    ungroupedNotificationsByUser.put(sbn.getPackageName(), notificationsForPackage);

                    if (notificationsForPackage.size() >= AUTOGROUP_AT_COUNT) {
                    if (notificationsForPackage.size() >= AUTOGROUP_AT_COUNT
                            || autogroupSummaryExists) {
                        notificationsToGroup.addAll(notificationsForPackage);
                    }
                }
@@ -120,6 +121,7 @@ public class GroupHelper {
            // If the status change of this notification has brought the number of loose
            // notifications to zero, remove the summary and un-autogroup.
            if (notificationsForPackage.size() == 0) {
                ungroupedNotificationsByUser.remove(sbn.getPackageName());
                removeSummary = true;
            }
        }
+9 −5
Original line number Diff line number Diff line
@@ -187,7 +187,6 @@ import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileDescriptor;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
@@ -2051,9 +2050,7 @@ public class NotificationManagerService extends SystemService {

        private StatusBarNotification sanitizeSbn(String pkg, int userId,
                StatusBarNotification sbn) {
            if (sbn.getPackageName().equals(pkg) && sbn.getUserId() == userId
                    && (sbn.getNotification().flags
                    & Notification.FLAG_AUTOGROUP_SUMMARY) == 0) {
            if (sbn.getPackageName().equals(pkg) && sbn.getUserId() == userId) {
                // We could pass back a cloneLight() but clients might get confused and
                // try to send this thing back to notify() again, which would not work
                // very well.
@@ -3061,6 +3058,12 @@ public class NotificationManagerService extends SystemService {
        }
    }

    @GuardedBy("mNotificationLock")
    private boolean hasAutoGroupSummaryLocked(StatusBarNotification sbn) {
        ArrayMap<String, String> summaries = mAutobundledSummaries.get(sbn.getUserId());
        return summaries != null && summaries.containsKey(sbn.getPackageName());
    }

    // Posts a 'fake' summary for a package that has exceeded the solo-notification limit.
    private void createAutoGroupSummary(int userId, String pkg, String triggeringKey) {
        NotificationRecord summaryRecord = null;
@@ -3836,7 +3839,8 @@ public class NotificationManagerService extends SystemService {
                            mHandler.post(new Runnable() {
                                @Override
                                public void run() {
                                    mGroupHelper.onNotificationPosted(n);
                                    mGroupHelper.onNotificationPosted(
                                            n, hasAutoGroupSummaryLocked(n));
                                }
                            });
                        }
+63 −16
Original line number Diff line number Diff line
@@ -15,6 +15,9 @@
 */
package com.android.server.notification;

import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertNotNull;

import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
@@ -29,18 +32,16 @@ import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;

import android.app.AlarmManager;
import android.app.Notification;
import android.app.NotificationChannel;
import android.app.NotificationManager;
import android.app.PendingIntent;
import android.os.UserHandle;
import android.service.notification.StatusBarNotification;
import android.support.test.runner.AndroidJUnit4;
import android.test.suitebuilder.annotation.SmallTest;

import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;

@SmallTest
@RunWith(AndroidJUnit4.class)
@@ -77,7 +78,8 @@ public class GroupHelperTest extends NotificationTestCase {
    public void testNoGroup_postingUnderLimit() throws Exception {
        final String pkg = "package";
        for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 1; i++) {
            mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM));
            mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM),
                    false);
        }
        verify(mCallback, never()).addAutoGroupSummary(
                eq(UserHandle.USER_SYSTEM), eq(pkg), anyString());
@@ -91,10 +93,11 @@ public class GroupHelperTest extends NotificationTestCase {
        final String pkg = "package";
        final String pkg2 = "package2";
        for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 1; i++) {
            mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM));
            mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM),
                    false);
        }
        mGroupHelper.onNotificationPosted(
                getSbn(pkg2, GroupHelper.AUTOGROUP_AT_COUNT, "four", UserHandle.SYSTEM));
                getSbn(pkg2, GroupHelper.AUTOGROUP_AT_COUNT, "four", UserHandle.SYSTEM), false);
        verify(mCallback, never()).addAutoGroupSummary(
                eq(UserHandle.USER_SYSTEM), eq(pkg), anyString());
        verify(mCallback, never()).addAutoGroup(anyString());
@@ -106,10 +109,12 @@ public class GroupHelperTest extends NotificationTestCase {
    public void testNoGroup_multiUser() throws Exception {
        final String pkg = "package";
        for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 1; i++) {
            mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM));
            mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM),
                    false);
        }
        mGroupHelper.onNotificationPosted(
                getSbn(pkg, GroupHelper.AUTOGROUP_AT_COUNT, "four", UserHandle.ALL));
                getSbn(pkg, GroupHelper.AUTOGROUP_AT_COUNT, "four", UserHandle.ALL),
                false);
        verify(mCallback, never()).addAutoGroupSummary(anyInt(), eq(pkg), anyString());
        verify(mCallback, never()).addAutoGroup(anyString());
        verify(mCallback, never()).removeAutoGroup(anyString());
@@ -120,10 +125,12 @@ public class GroupHelperTest extends NotificationTestCase {
    public void testNoGroup_someAreGrouped() throws Exception {
        final String pkg = "package";
        for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 1; i++) {
            mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM));
            mGroupHelper.onNotificationPosted(
                    getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM), false);
        }
        mGroupHelper.onNotificationPosted(
                getSbn(pkg, GroupHelper.AUTOGROUP_AT_COUNT, "four", UserHandle.SYSTEM, "a"));
                getSbn(pkg, GroupHelper.AUTOGROUP_AT_COUNT, "four", UserHandle.SYSTEM, "a"),
                false);
        verify(mCallback, never()).addAutoGroupSummary(
                eq(UserHandle.USER_SYSTEM), eq(pkg), anyString());
        verify(mCallback, never()).addAutoGroup(anyString());
@@ -136,7 +143,8 @@ public class GroupHelperTest extends NotificationTestCase {
    public void testPostingOverLimit() throws Exception {
        final String pkg = "package";
        for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) {
            mGroupHelper.onNotificationPosted(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM));
            mGroupHelper.onNotificationPosted(
                    getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM), false);
        }
        verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString());
        verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT)).addAutoGroup(anyString());
@@ -151,7 +159,7 @@ public class GroupHelperTest extends NotificationTestCase {
        for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) {
            final StatusBarNotification sbn = getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM);
            posted.add(sbn);
            mGroupHelper.onNotificationPosted(sbn);
            mGroupHelper.onNotificationPosted(sbn, false);
        }
        verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString());
        verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT)).addAutoGroup(anyString());
@@ -178,7 +186,7 @@ public class GroupHelperTest extends NotificationTestCase {
        for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) {
            final StatusBarNotification sbn = getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM);
            posted.add(sbn);
            mGroupHelper.onNotificationPosted(sbn);
            mGroupHelper.onNotificationPosted(sbn, false);
        }
        verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString());
        verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT)).addAutoGroup(anyString());
@@ -190,7 +198,7 @@ public class GroupHelperTest extends NotificationTestCase {
        for (i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT - 2; i++) {
            final StatusBarNotification sbn =
                    getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM, "app group");
            mGroupHelper.onNotificationPosted(sbn);
            mGroupHelper.onNotificationPosted(sbn, false);
        }
        verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT - 2)).removeAutoGroup(anyString());
        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());
@@ -199,9 +207,48 @@ public class GroupHelperTest extends NotificationTestCase {
        for (; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) {
            final StatusBarNotification sbn =
                    getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM, "app group");
            mGroupHelper.onNotificationPosted(sbn);
            mGroupHelper.onNotificationPosted(sbn, false);
        }
        verify(mCallback, times(2)).removeAutoGroup(anyString());
        verify(mCallback, times(1)).removeAutoGroupSummary(anyInt(), anyString());
    }

    @Test
    public void testNewNotificationsAddedToAutogroup_ifOriginalNotificationsCanceled()
            throws Exception {
        final String pkg = "package";
        List<StatusBarNotification> posted = new ArrayList<>();
        for (int i = 0; i < GroupHelper.AUTOGROUP_AT_COUNT; i++) {
            final StatusBarNotification sbn = getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM);
            posted.add(sbn);
            mGroupHelper.onNotificationPosted(sbn, false);
        }
        verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString());
        verify(mCallback, times(GroupHelper.AUTOGROUP_AT_COUNT)).addAutoGroup(anyString());
        verify(mCallback, never()).removeAutoGroup(anyString());
        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());
        Mockito.reset(mCallback);

        for (int i = posted.size() - 2; i >= 0; i--) {
            mGroupHelper.onNotificationRemoved(posted.remove(i));
        }
        verify(mCallback, never()).removeAutoGroup(anyString());
        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());
        Mockito.reset(mCallback);

        // only one child remains
        Map<String, LinkedHashSet<String>> ungroupedForUser =
                mGroupHelper.mUngroupedNotifications.get(UserHandle.USER_SYSTEM);
        assertNotNull(ungroupedForUser);
        assertEquals(1, ungroupedForUser.get(pkg).size());

        // Add new notification; it should be autogrouped even though the total count is
        // < AUTOGROUP_AT_COUNT
        final StatusBarNotification sbn = getSbn(pkg, 5, String.valueOf(5), UserHandle.SYSTEM);
        posted.add(sbn);
        mGroupHelper.onNotificationPosted(sbn, true);
        verify(mCallback, times(posted.size())).addAutoGroup(anyString());
        verify(mCallback, never()).removeAutoGroup(anyString());
        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());
    }
}
+3 −3
Original line number Diff line number Diff line
@@ -1339,7 +1339,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
        runnable.run();
        waitForIdle();

        verify(mGroupHelper, times(1)).onNotificationPosted(any());
        verify(mGroupHelper, times(1)).onNotificationPosted(any(), anyBoolean());
    }

    @Test
@@ -1355,7 +1355,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
        runnable.run();
        waitForIdle();

        verify(mGroupHelper, times(1)).onNotificationPosted(any());
        verify(mGroupHelper, times(1)).onNotificationPosted(any(), anyBoolean());
    }

    @Test
@@ -1371,7 +1371,7 @@ public class NotificationManagerServiceTest extends NotificationTestCase {
        runnable.run();
        waitForIdle();

        verify(mGroupHelper, never()).onNotificationPosted(any());
        verify(mGroupHelper, never()).onNotificationPosted(any(), anyBoolean());
    }

    @Test