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

Commit 0fef5ff2 authored by Jeff Nainaparampil's avatar Jeff Nainaparampil Committed by jeffnainap
Browse files

[People Service] Fix `hasActiveNotifications` to track when notifications are modified 

Instead of tracking the number of active notifications, we track notifications for a package by their id. This is more robust and allows People Service to keep track of notifications that are updated, such as Messages which updates an existing notification for new incoming messages.

This fixes an issue in which recent conversations are retained even if they do not contain active notifications.

Bug: 263634015
Change-Id: I187161391a44f9e03840b89d88c4d3c083e1b9bc
parent bb994c9b

services/people/java/com/android/server/people/data/DataManager.java

+12 −10
Original line number Diff line number Diff line
@@ -1162,9 +1162,9 @@ public class DataManager { 


        private final int mUserId;



        // Conversation package name + shortcut ID -> Number of active notifications

        // Conversation package name + shortcut ID -> Keys of active notifications

        @GuardedBy("this")

        private final Map<Pair<String, String>, Integer> mActiveNotifCounts = new ArrayMap<>();

        private final Map<Pair<String, String>, Set<String>> mActiveNotifKeys = new ArrayMap<>();



        private NotificationListener(int userId) {

            mUserId = userId;
@@ -1178,8 +1178,10 @@ public class DataManager { 
            String shortcutId = sbn.getNotification().getShortcutId();

            PackageData packageData = getPackageIfConversationExists(sbn, conversationInfo -> {

                synchronized (this) {

                    mActiveNotifCounts.merge(

                            Pair.create(sbn.getPackageName(), shortcutId), 1, Integer::sum);

                    Set<String> notificationKeys = mActiveNotifKeys.computeIfAbsent(

                            Pair.create(sbn.getPackageName(), shortcutId),

                            (unusedKey) -> new HashSet<>());

                    notificationKeys.add(sbn.getKey());

                }

            });
@@ -1218,12 +1220,12 @@ public class DataManager { 
                Pair<String, String> conversationKey =

                        Pair.create(sbn.getPackageName(), shortcutId);

                synchronized (this) {

                    int count = mActiveNotifCounts.getOrDefault(conversationKey, 0) - 1;

                    if (count <= 0) {

                        mActiveNotifCounts.remove(conversationKey);

                    Set<String> notificationKeys = mActiveNotifKeys.computeIfAbsent(

                            conversationKey, (unusedKey) -> new HashSet<>());

                    notificationKeys.remove(sbn.getKey());

                    if (notificationKeys.isEmpty()) {

                        mActiveNotifKeys.remove(conversationKey);

                        cleanupCachedShortcuts(mUserId, MAX_CACHED_RECENT_SHORTCUTS);

                    } else {

                        mActiveNotifCounts.put(conversationKey, count);

                    }

                }

            });
@@ -1289,7 +1291,7 @@ public class DataManager { 
        }



        synchronized boolean hasActiveNotifications(String packageName, String shortcutId) {

            return mActiveNotifCounts.containsKey(Pair.create(packageName, shortcutId));

            return mActiveNotifKeys.containsKey(Pair.create(packageName, shortcutId));

        }

    }

services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java

+43 −4
Original line number Diff line number Diff line
@@ -500,6 +500,7 @@ public final class DataManagerTest { 
        // The cached conversations are above the limit because every conversation has active

        // notifications. To uncache one of them, the notifications for that conversation need to

        // be dismissed.

        String notificationKey = "";

        for (int i = 0; i < DataManager.MAX_CACHED_RECENT_SHORTCUTS + 1; i++) {

            String shortcutId = TEST_SHORTCUT_ID + i;

            ShortcutInfo shortcut = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, shortcutId,
@@ -507,11 +508,13 @@ public final class DataManagerTest { 
            shortcut.setCached(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS);

            mDataManager.addOrUpdateConversationInfo(shortcut);

            when(mNotification.getShortcutId()).thenReturn(shortcutId);

            sendGenericNotification();

            notificationKey = String.format("notification-key-%d", i);

            sendGenericNotificationWithKey(notificationKey);

        }



        // Post another notification for the last conversation.

        sendGenericNotification();

        String otherNotificationKey = "other-notification-key";

        sendGenericNotificationWithKey(otherNotificationKey);



        // Removing one of the two notifications does not un-cache the shortcut.

        listenerService.onNotificationRemoved(mGenericSbn, null,
@@ -520,6 +523,7 @@ public final class DataManagerTest { 
                anyInt(), any(), anyString(), any(), anyInt(), anyInt());



        // Removing the second notification un-caches the shortcut.

        when(mGenericSbn.getKey()).thenReturn(notificationKey);

        listenerService.onNotificationRemoved(mGenericSbn, null,

                NotificationListenerService.REASON_CANCEL_ALL);

        verify(mShortcutServiceInternal).uncacheShortcuts(
@@ -686,6 +690,35 @@ public final class DataManagerTest { 
        assertThat(result.getStatuses()).containsExactly(cs);

    }



    @Test

    public void testGetConversation_trackActiveConversations() {

        mDataManager.onUserUnlocked(USER_ID_PRIMARY);

        assertThat(mDataManager.getConversation(TEST_PKG_NAME, USER_ID_PRIMARY,

                TEST_SHORTCUT_ID)).isNull();

        ShortcutInfo shortcut = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, TEST_SHORTCUT_ID,

                buildPerson());

        shortcut.setCached(ShortcutInfo.FLAG_PINNED);

        mDataManager.addOrUpdateConversationInfo(shortcut);

        assertThat(mDataManager.getConversation(TEST_PKG_NAME, USER_ID_PRIMARY,

                TEST_SHORTCUT_ID)).isNotNull();



        sendGenericNotification();

        sendGenericNotification();

        ConversationChannel result = mDataManager.getConversation(TEST_PKG_NAME, USER_ID_PRIMARY,

                TEST_SHORTCUT_ID);

        assertTrue(result.hasActiveNotifications());



        // Both generic notifications have the same notification key, so a single dismiss will

        // remove both of them.

        NotificationListenerService listenerService =

                mDataManager.getNotificationListenerServiceForTesting(USER_ID_PRIMARY);

        listenerService.onNotificationRemoved(mGenericSbn, null,

                NotificationListenerService.REASON_CANCEL);

        ConversationChannel resultTwo = mDataManager.getConversation(TEST_PKG_NAME, USER_ID_PRIMARY,

                TEST_SHORTCUT_ID);

        assertFalse(resultTwo.hasActiveNotifications());

    }



    @Test

    public void testGetConversation_unsyncedShortcut() {

        mDataManager.onUserUnlocked(USER_ID_PRIMARY);
@@ -1693,6 +1726,12 @@ public final class DataManagerTest { 
    // "Sends" a notification to a non-customized notification channel - the notification channel

    // is something generic like "messages" and the notification has a  shortcut id

    private void sendGenericNotification() {

        sendGenericNotificationWithKey(GENERIC_KEY);

    }



    // "Sends" a notification to a non-customized notification channel with the specified key.

    private void sendGenericNotificationWithKey(String key) {

        when(mGenericSbn.getKey()).thenReturn(key);

        when(mNotification.getChannelId()).thenReturn(PARENT_NOTIFICATION_CHANNEL_ID);

        doAnswer(invocationOnMock -> {

            NotificationListenerService.Ranking ranking = (NotificationListenerService.Ranking)
@@ -1708,7 +1747,7 @@ public final class DataManagerTest { 
                    mParentNotificationChannel, null, null, true, 0, false, -1, false, null, null,

                    false, false, false, null, 0, false, 0);

            return true;

        }).when(mRankingMap).getRanking(eq(GENERIC_KEY),

        }).when(mRankingMap).getRanking(eq(key),

                any(NotificationListenerService.Ranking.class));

        NotificationListenerService listenerService =

                mDataManager.getNotificationListenerServiceForTesting(USER_ID_PRIMARY);