Merge "[People Service] Fix hasActiveNotifications to track when notifications are modified" into tm-qpr-dev

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

@@ -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(
+                    Set<String> notificationKeys = mActiveNotifKeys.computeIfAbsent(
-                            Pair.create(sbn.getPackageName(), shortcutId), 1, Integer::sum);
+                            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;
+                    Set<String> notificationKeys = mActiveNotifKeys.computeIfAbsent(
-                    if (count <= 0) {
+                            conversationKey, (unusedKey) -> new HashSet<>());
-                        mActiveNotifCounts.remove(conversationKey);
+                    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

@@ -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);
@@ -1322,7 +1355,7 @@ public final class DataManagerTest {
 
         sendGenericNotification();
 
-       mDataManager.getRecentConversations(USER_ID_PRIMARY);
+        mDataManager.getRecentConversations(USER_ID_PRIMARY);
 
         verify(mShortcutServiceInternal).getShortcuts(
                 anyInt(), anyString(), anyLong(), anyString(), anyList(), any(), any(),
@@ -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);