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

Commit 65d46964 authored by Julia Reynolds's avatar Julia Reynolds
Browse files

Fix ongoing autogenerated summaries 

Fixes an issue where the autogroup summary wasn't being
marked with the ongoing flag if the ongoing notifications
were posted before the summary was created and then never updated

Test: GroupHelperTest, manually look at "android" group when two
system USB notifications are shown
Bug: 225393724

Change-Id: Id7953bafa4f709bb053f24b72be3925465931ec5
parent 362dcaab

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

+23 −29
Original line number Diff line number Diff line
@@ -42,7 +42,7 @@ public class GroupHelper { 
    private final int mAutoGroupAtCount;



    // count the number of ongoing notifications per group

    // userId -> (package name -> (group Id -> (set of notification keys)))

    // userId|packageName -> (set of ongoing notifications that aren't in an app group)

    final ArrayMap<String, ArraySet<String>>

            mOngoingGroupCount = new ArrayMap<>();
@@ -55,52 +55,43 @@ public class GroupHelper { 
        mCallback = callback;

    }



    private String generatePackageGroupKey(int userId, String pkg, String group) {

        return userId + "|" + pkg + "|" + group;

    private String generatePackageKey(int userId, String pkg) {

        return userId + "|" + pkg;

    }



    @VisibleForTesting

    protected int getOngoingGroupCount(int userId, String pkg, String group) {

        String key = generatePackageGroupKey(userId, pkg, group);

    protected int getOngoingGroupCount(int userId, String pkg) {

        String key = generatePackageKey(userId, pkg);

        return mOngoingGroupCount.getOrDefault(key, new ArraySet<>(0)).size();

    }



    private void addToOngoingGroupCount(StatusBarNotification sbn, boolean add) {

        if (sbn.getNotification().isGroupSummary()) return;

        if (!sbn.isOngoing() && add) return;

        String group = sbn.getGroup();

        if (group == null) return;

        int userId = sbn.getUser().getIdentifier();

        String key = generatePackageGroupKey(userId, sbn.getPackageName(), group);

    private void updateOngoingGroupCount(StatusBarNotification sbn, boolean add) {

        if (sbn.getNotification().isGroupSummary()) {

            return;

        }

        String key = generatePackageKey(sbn.getUserId(), sbn.getPackageName());

        ArraySet<String> notifications = mOngoingGroupCount.getOrDefault(key, new ArraySet<>(0));

        if (add) {

            notifications.add(sbn.getKey());

            mOngoingGroupCount.put(key, notifications);

        } else {

            notifications.remove(sbn.getKey());

            // we dont need to put it back if it is default

            // we don't need to put it back if it is default

        }

        String combinedKey = generatePackageGroupKey(userId, sbn.getPackageName(), group);



        boolean needsOngoingFlag = notifications.size() > 0;

        mCallback.updateAutogroupSummary(userId, sbn.getPackageName(), needsOngoingFlag);

        mCallback.updateAutogroupSummary(sbn.getUserId(), sbn.getPackageName(), needsOngoingFlag);

    }



    public void onNotificationUpdated(StatusBarNotification childSbn,

            boolean autogroupSummaryExists) {

        if (childSbn.getGroup() != AUTOGROUP_KEY

                || childSbn.getNotification().isGroupSummary()) return;

        if (childSbn.isOngoing()) {

            addToOngoingGroupCount(childSbn, true);

        } else {

            addToOngoingGroupCount(childSbn, false);

        }

    public void onNotificationUpdated(StatusBarNotification childSbn) {

        updateOngoingGroupCount(childSbn, childSbn.isOngoing() && !childSbn.isAppGroup());

    }



    public void onNotificationPosted(StatusBarNotification sbn, boolean autogroupSummaryExists) {

        if (DEBUG) Log.i(TAG, "POSTED " + sbn.getKey());

        try {

            updateOngoingGroupCount(sbn, sbn.isOngoing() && !sbn.isAppGroup());



            List<String> notificationsToGroup = new ArrayList<>();

            if (autogroupSummaryExists) addToOngoingGroupCount(sbn, true);

            if (!sbn.isAppGroup()) {

                // Not grouped by the app, add to the list of notifications for the app;

                // send grouping update if app exceeds the autogrouping limit.
@@ -134,6 +125,7 @@ public class GroupHelper { 
                // Grouped, but not by us. Send updates to un-autogroup, if we grouped it.

                maybeUngroup(sbn, false, sbn.getUserId());

            }



        } catch (Exception e) {

            Slog.e(TAG, "Failure processing new notification", e);

        }
@@ -141,7 +133,7 @@ public class GroupHelper { 


    public void onNotificationRemoved(StatusBarNotification sbn) {

        try {

            addToOngoingGroupCount(sbn, false);

            updateOngoingGroupCount(sbn, false);

            maybeUngroup(sbn, true, sbn.getUserId());

        } catch (Exception e) {

            Slog.e(TAG, "Error processing canceled notification", e);
@@ -189,7 +181,8 @@ public class GroupHelper { 
    private void adjustAutogroupingSummary(int userId, String packageName, String triggeringKey,

            boolean summaryNeeded) {

        if (summaryNeeded) {

            mCallback.addAutoGroupSummary(userId, packageName, triggeringKey);

            mCallback.addAutoGroupSummary(userId, packageName, triggeringKey,

                    getOngoingGroupCount(userId, packageName) > 0);

        } else {

            mCallback.removeAutoGroupSummary(userId, packageName);

        }
@@ -209,7 +202,8 @@ public class GroupHelper { 
    protected interface Callback {

        void addAutoGroup(String key);

        void removeAutoGroup(String key);

        void addAutoGroupSummary(int userId, String pkg, String triggeringKey);

        void addAutoGroupSummary(int userId, String pkg, String triggeringKey,

                boolean needsOngoingFlag);

        void removeAutoGroupSummary(int user, String pkg);

        void updateAutogroupSummary(int userId, String pkg, boolean needsOngoingFlag);

    }

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

+12 −8
Original line number Diff line number Diff line
@@ -2557,8 +2557,10 @@ public class NotificationManagerService extends SystemService { 
            }















            @Override













            public void addAutoGroupSummary(int userId, String pkg, String triggeringKey) {













                NotificationRecord r = createAutoGroupSummary(userId, pkg, triggeringKey);













            public void addAutoGroupSummary(int userId, String pkg, String triggeringKey,













                    boolean needsOngoingFlag) {













                NotificationRecord r = createAutoGroupSummary(













                        userId, pkg, triggeringKey, needsOngoingFlag);















                if (r != null) {















                    final boolean isAppForeground =















                            mActivityManager.getPackageImportance(pkg) == IMPORTANCE_FOREGROUND;










@@ -5739,6 +5741,7 @@ public class NotificationManagerService extends SystemService {













    void removeAutogroupKeyLocked(String key) {















        NotificationRecord r = mNotificationsByKey.get(key);















        if (r == null) {













            Slog.w(TAG, "Failed to remove autogroup " + key);















            return;















        }















        if (r.getSbn().getOverrideGroupKey() != null) {










@@ -5778,7 +5781,8 @@ public class NotificationManagerService extends SystemService {













    }





























    // Creates a 'fake' summary for a package that has exceeded the solo-notification limit.













    NotificationRecord createAutoGroupSummary(int userId, String pkg, String triggeringKey) {













    NotificationRecord createAutoGroupSummary(int userId, String pkg, String triggeringKey,













            boolean needsOngoingFlag) {















        NotificationRecord summaryRecord = null;















        boolean isPermissionFixed = mPermissionHelper.isMigrationEnabled()















                ? mPermissionHelper.isPermissionFixed(pkg, userId) : false;










@@ -5818,6 +5822,7 @@ public class NotificationManagerService extends SystemService {













                                .setGroup(GroupHelper.AUTOGROUP_KEY)















                                .setFlag(FLAG_AUTOGROUP_SUMMARY, true)















                                .setFlag(Notification.FLAG_GROUP_SUMMARY, true)













                                .setFlag(FLAG_ONGOING_EVENT, needsOngoingFlag)















                                .setColor(adjustedSbn.getNotification().color)















                                .setLocalOnly(true)















                                .build();










@@ -7356,17 +7361,16 @@ public class NotificationManagerService extends SystemService {













                        mListeners.notifyPostedLocked(r, old);















                        if ((oldSbn == null || !Objects.equals(oldSbn.getGroup(), n.getGroup()))















                                && !isCritical(r)) {













                            mHandler.post(new Runnable() {













                                @Override













                                public void run() {













                            mHandler.post(() -> {













                                synchronized (mNotificationLock) {















                                    mGroupHelper.onNotificationPosted(















                                            n, hasAutoGroupSummaryLocked(n));















                                }















                            });















                        } else if (oldSbn != null) {















                            final NotificationRecord finalRecord = r;













                            mHandler.post(() -> mGroupHelper.onNotificationUpdated(













                                    finalRecord.getSbn(), hasAutoGroupSummaryLocked(n)));













                            mHandler.post(() ->













                                    mGroupHelper.onNotificationUpdated(finalRecord.getSbn()));















                        }















                    } else {















                        Slog.e(TAG, "Not posting notification without small icon: " + notification);

































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



+46
−36




























Original line number
Diff line number
Diff line








@@ -20,6 +20,7 @@ import static com.android.server.notification.GroupHelper.AUTOGROUP_KEY;













import static junit.framework.Assert.assertEquals;















import static junit.framework.Assert.assertNotNull;





























import static org.mockito.ArgumentMatchers.anyBoolean;















import static org.mockito.Matchers.anyInt;















import static org.mockito.Matchers.anyString;















import static org.mockito.Matchers.eq;










@@ -53,7 +54,7 @@ import java.util.Map;













public class GroupHelperTest extends UiServiceTestCase {















    private @Mock GroupHelper.Callback mCallback;





























    private final static int AUTOGROUP_AT_COUNT = 4;













    private final static int AUTOGROUP_AT_COUNT = 7;















    private GroupHelper mGroupHelper;































    @Before










@@ -88,7 +89,7 @@ public class GroupHelperTest extends UiServiceTestCase {













                    false);















        }















        verify(mCallback, never()).addAutoGroupSummary(













                eq(UserHandle.USER_SYSTEM), eq(pkg), anyString());













                eq(UserHandle.USER_SYSTEM), eq(pkg), anyString(), anyBoolean());















        verify(mCallback, never()).addAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());









@@ -105,7 +106,7 @@ public class GroupHelperTest extends UiServiceTestCase {













        mGroupHelper.onNotificationPosted(















                getSbn(pkg2, AUTOGROUP_AT_COUNT, "four", UserHandle.SYSTEM), false);















        verify(mCallback, never()).addAutoGroupSummary(













                eq(UserHandle.USER_SYSTEM), eq(pkg), anyString());













                eq(UserHandle.USER_SYSTEM), eq(pkg), anyString(), anyBoolean());















        verify(mCallback, never()).addAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());









@@ -120,7 +121,8 @@ public class GroupHelperTest extends UiServiceTestCase {













        }















        mGroupHelper.onNotificationPosted(















                getSbn(pkg, AUTOGROUP_AT_COUNT, "four", UserHandle.ALL), false);













        verify(mCallback, never()).addAutoGroupSummary(anyInt(), eq(pkg), anyString());













        verify(mCallback, never()).addAutoGroupSummary(













                anyInt(), eq(pkg), anyString(), anyBoolean());















        verify(mCallback, never()).addAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());









@@ -136,13 +138,12 @@ public class GroupHelperTest extends UiServiceTestCase {













        mGroupHelper.onNotificationPosted(















                getSbn(pkg, AUTOGROUP_AT_COUNT, "four", UserHandle.SYSTEM, "a"), false);















        verify(mCallback, never()).addAutoGroupSummary(













                eq(UserHandle.USER_SYSTEM), eq(pkg), anyString());













                eq(UserHandle.USER_SYSTEM), eq(pkg), anyString(), anyBoolean());















        verify(mCallback, never()).addAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());















    }













































    @Test















    public void testPostingOverLimit() throws Exception {















        final String pkg = "package";









@@ -150,7 +151,23 @@ public class GroupHelperTest extends UiServiceTestCase {













            mGroupHelper.onNotificationPosted(















                    getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM), false);















        }













        verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString());













        verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString(), eq(false));













        verify(mCallback, times(AUTOGROUP_AT_COUNT)).addAutoGroup(anyString());













        verify(mCallback, never()).removeAutoGroup(anyString());













        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());













    }



























    @Test













    public void testPostingOverLimit_addsOngoingFlag() throws Exception {













        final String pkg = "package";













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













            StatusBarNotification sbn = getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM);













            if (i == 0) {













                sbn.getNotification().flags |= Notification.FLAG_ONGOING_EVENT;













            }













            mGroupHelper.onNotificationPosted(sbn, false);













        }













        verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString(), eq(true));















        verify(mCallback, times(AUTOGROUP_AT_COUNT)).addAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());










@@ -178,7 +195,7 @@ public class GroupHelperTest extends UiServiceTestCase {





























        int userId = UserHandle.SYSTEM.getIdentifier();















        assertEquals(mGroupHelper.getOngoingGroupCount(













                userId, pkg, AUTOGROUP_KEY), AUTOGROUP_AT_COUNT + 1);













                userId, pkg), AUTOGROUP_AT_COUNT + 1);















    }































    @Test









@@ -199,15 +216,14 @@ public class GroupHelperTest extends UiServiceTestCase {













        }































        notifications.get(0).getNotification().flags &= ~Notification.FLAG_ONGOING_EVENT;



























        mGroupHelper.onNotificationUpdated(notifications.get(0), true);













        mGroupHelper.onNotificationUpdated(notifications.get(0));































        verify(mCallback, times(AUTOGROUP_AT_COUNT + 2))















                .updateAutogroupSummary(anyInt(), anyString(), eq(true));































        int userId = UserHandle.SYSTEM.getIdentifier();















        assertEquals(mGroupHelper.getOngoingGroupCount(













                userId, pkg, AUTOGROUP_KEY), AUTOGROUP_AT_COUNT);













                userId, pkg), AUTOGROUP_AT_COUNT);















    }































    @Test









@@ -229,18 +245,18 @@ public class GroupHelperTest extends UiServiceTestCase {





























        notifications.get(0).getNotification().flags &= ~Notification.FLAG_ONGOING_EVENT;





























        mGroupHelper.onNotificationUpdated(notifications.get(0), true);













        mGroupHelper.onNotificationUpdated(notifications.get(0));































        notifications.get(0).getNotification().flags |= Notification.FLAG_ONGOING_EVENT;





























        mGroupHelper.onNotificationUpdated(notifications.get(0), true);













        mGroupHelper.onNotificationUpdated(notifications.get(0));































        verify(mCallback, times(AUTOGROUP_AT_COUNT + 3))















                .updateAutogroupSummary(anyInt(), anyString(), eq(true));































        int userId = UserHandle.SYSTEM.getIdentifier();















        assertEquals(mGroupHelper.getOngoingGroupCount(













                userId, pkg, AUTOGROUP_KEY), AUTOGROUP_AT_COUNT + 1);













                userId, pkg), AUTOGROUP_AT_COUNT + 1);















    }































    @Test









@@ -267,7 +283,7 @@ public class GroupHelperTest extends UiServiceTestCase {





























        int userId = UserHandle.SYSTEM.getIdentifier();















        assertEquals(mGroupHelper.getOngoingGroupCount(













                userId, pkg, AUTOGROUP_KEY), AUTOGROUP_AT_COUNT);













                userId, pkg), AUTOGROUP_AT_COUNT);















    }









































@@ -288,14 +304,14 @@ public class GroupHelperTest extends UiServiceTestCase {













        }































        notifications.get(0).getNotification().flags |= Notification.FLAG_ONGOING_EVENT;













        mGroupHelper.onNotificationUpdated(notifications.get(0), true);













        mGroupHelper.onNotificationUpdated(notifications.get(0));































        verify(mCallback, times(1))















                .updateAutogroupSummary(anyInt(), anyString(), eq(true));































        int userId = UserHandle.SYSTEM.getIdentifier();















        assertEquals(mGroupHelper.getOngoingGroupCount(













                userId, pkg, AUTOGROUP_KEY), 1);













                userId, pkg), 1);















    }































    @Test









@@ -305,7 +321,7 @@ public class GroupHelperTest extends UiServiceTestCase {













        for (int i = 0; i < AUTOGROUP_AT_COUNT + 1; i++) {















            notifications.add(getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM));















        }













        StatusBarNotification sbn = notifications.get(0);













        StatusBarNotification sbn = notifications.get(AUTOGROUP_AT_COUNT);















        sbn.getNotification().flags |= Notification.FLAG_ONGOING_EVENT;















        sbn.setOverrideGroupKey(AUTOGROUP_KEY);

























@@ -319,7 +335,7 @@ public class GroupHelperTest extends UiServiceTestCase {





























        int userId = UserHandle.SYSTEM.getIdentifier();















        assertEquals(mGroupHelper.getOngoingGroupCount(













                userId, pkg, AUTOGROUP_KEY), 1);













                userId, pkg), 1);















    }































    @Test









@@ -342,7 +358,7 @@ public class GroupHelperTest extends UiServiceTestCase {













                .updateAutogroupSummary(anyInt(), anyString(), eq(true));































        int userId = UserHandle.SYSTEM.getIdentifier();













        assertEquals(mGroupHelper.getOngoingGroupCount(userId, pkg, AUTOGROUP_KEY), 0);













        assertEquals(mGroupHelper.getOngoingGroupCount(userId, pkg), 0);















    }









































@@ -355,7 +371,7 @@ public class GroupHelperTest extends UiServiceTestCase {













            posted.add(sbn);















            mGroupHelper.onNotificationPosted(sbn, false);















        }













        verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString());













        verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString(), eq(false));















        verify(mCallback, times(AUTOGROUP_AT_COUNT)).addAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());









@@ -382,28 +398,22 @@ public class GroupHelperTest extends UiServiceTestCase {













            posted.add(sbn);















            mGroupHelper.onNotificationPosted(sbn, false);















        }













        verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString());













        verify(mCallback, times(1)).addAutoGroupSummary(













                anyInt(), eq(pkg), anyString(), anyBoolean());















        verify(mCallback, times(AUTOGROUP_AT_COUNT)).addAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());















        Mockito.reset(mCallback);





























        int i = 0;













        for (i = 0; i < AUTOGROUP_AT_COUNT - 2; i++) {













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















            final StatusBarNotification sbn =















                    getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM, "app group");















            mGroupHelper.onNotificationPosted(sbn, false);













        }













        verify(mCallback, times(AUTOGROUP_AT_COUNT - 2)).removeAutoGroup(anyString());













            verify(mCallback, times(1)).removeAutoGroup(sbn.getKey());













            if (i < AUTOGROUP_AT_COUNT -1) {















                verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());













        Mockito.reset(mCallback);



























        for (; i < AUTOGROUP_AT_COUNT; i++) {













            final StatusBarNotification sbn =













                    getSbn(pkg, i, String.valueOf(i), UserHandle.SYSTEM, "app group");













            mGroupHelper.onNotificationPosted(sbn, false);















            }













        verify(mCallback, times(2)).removeAutoGroup(anyString());













        }















        verify(mCallback, times(1)).removeAutoGroupSummary(anyInt(), anyString());















    }

























@@ -417,7 +427,7 @@ public class GroupHelperTest extends UiServiceTestCase {













            posted.add(sbn);















            mGroupHelper.onNotificationPosted(sbn, false);















        }













        verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString());













        verify(mCallback, times(1)).addAutoGroupSummary(anyInt(), eq(pkg), anyString(), eq(false));















        verify(mCallback, times(AUTOGROUP_AT_COUNT)).addAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());









@@ -441,7 +451,7 @@ public class GroupHelperTest extends UiServiceTestCase {













        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, times(1)).addAutoGroup(sbn.getKey());















        verify(mCallback, never()).removeAutoGroup(anyString());















        verify(mCallback, never()).removeAutoGroupSummary(anyInt(), anyString());















    }

































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



+1
−1




























Original line number
Diff line number
Diff line








@@ -722,7 +722,7 @@ public class NotificationPermissionMigrationTest extends UiServiceTestCase {













        when(mPermissionHelper.isPermissionFixed(PKG, temp.getUserId())).thenReturn(true);































        NotificationRecord r = mService.createAutoGroupSummary(













                temp.getUserId(), temp.getSbn().getPackageName(), temp.getKey());













                temp.getUserId(), temp.getSbn().getPackageName(), temp.getKey(), false);































        assertThat(r.isImportanceFixed()).isTrue();















    }