Cache the shortcut when the associated notifications are posted and uncache when they are gone

Change-Id: Ia569f9b31a0fe73242f2e6350805afe7c06d2cdf
Bug: 153357090
Test: Manual test
Test: atest DataManagerTest
Test: atest NotificationManagerServiceTest

7 files changed, 177 insertions, 28 deletions

diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index 8e9af1c63dc4..b83c0d20e2e5 100755
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -5683,6 +5683,14 @@ public class NotificationManagerService extends SystemService {
             return;
         }
 
+        if (info != null) {
+            // Cache the shortcut synchronously after the associated notification is posted in case
+            // the app unpublishes this shortcut immediately after posting the notification. If the
+            // user does not modify the notification settings on this conversation, the shortcut
+            // will be uncached by People Service when all the associated notifications are removed.
+            mShortcutHelper.cacheShortcut(info, user);
+        }
+
         // Whitelist pending intents.
         if (notification.allPendingIntents != null) {
             final int intentCount = notification.allPendingIntents.size();
diff --git a/services/core/java/com/android/server/notification/ShortcutHelper.java b/services/core/java/com/android/server/notification/ShortcutHelper.java
index 96da649350b0..13892ba0e480 100644
--- a/services/core/java/com/android/server/notification/ShortcutHelper.java
+++ b/services/core/java/com/android/server/notification/ShortcutHelper.java
@@ -34,6 +34,7 @@ import com.android.internal.annotations.VisibleForTesting;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 
@@ -186,6 +187,17 @@ public class ShortcutHelper {
     }
 
     /**
+     * Caches the given shortcut in Shortcut Service.
+     */
+    void cacheShortcut(ShortcutInfo shortcutInfo, UserHandle user) {
+        if (shortcutInfo.isLongLived() && !shortcutInfo.isCached()) {
+            mShortcutServiceInternal.cacheShortcuts(user.getIdentifier(), "android",
+                    shortcutInfo.getPackage(), Collections.singletonList(shortcutInfo.getId()),
+                    shortcutInfo.getUserId());
+        }
+    }
+
+    /**
      * Shortcut based bubbles require some extra work to listen for shortcut changes.
      *
      * @param r the notification record to check
diff --git a/services/people/java/com/android/server/people/data/ConversationInfo.java b/services/people/java/com/android/server/people/data/ConversationInfo.java
index dc3fa2a048f6..27fa36b70b8f 100644
--- a/services/people/java/com/android/server/people/data/ConversationInfo.java
+++ b/services/people/java/com/android/server/people/data/ConversationInfo.java
@@ -62,6 +62,8 @@ public class ConversationInfo {
 
     private static final int FLAG_DEMOTED = 1 << 6;
 
+    private static final int FLAG_NOTIFICATION_SETTING_CHANGED = 1 << 7;
+
     @IntDef(flag = true, prefix = {"FLAG_"}, value = {
             FLAG_IMPORTANT,
             FLAG_NOTIFICATION_SILENCED,
@@ -70,6 +72,7 @@ public class ConversationInfo {
             FLAG_PERSON_BOT,
             FLAG_CONTACT_STARRED,
             FLAG_DEMOTED,
+            FLAG_NOTIFICATION_SETTING_CHANGED,
     })
     @Retention(RetentionPolicy.SOURCE)
     private @interface ConversationFlags {
@@ -185,6 +188,11 @@ public class ConversationInfo {
         return hasConversationFlags(FLAG_CONTACT_STARRED);
     }
 
+    /** Whether the conversation's notification setting has ever been changed by the user. */
+    boolean isNotificationSettingChanged() {
+        return hasConversationFlags(FLAG_NOTIFICATION_SETTING_CHANGED);
+    }
+
     @Override
     public boolean equals(Object obj) {
         if (this == obj) {
@@ -491,6 +499,10 @@ public class ConversationInfo {
             return setConversationFlag(FLAG_CONTACT_STARRED, value);
         }
 
+        Builder setNotificationSettingChanged(boolean value) {
+            return setConversationFlag(FLAG_NOTIFICATION_SETTING_CHANGED, value);
+        }
+
         private Builder setConversationFlag(@ConversationFlags int flags, boolean value) {
             if (value) {
                 return addConversationFlags(flags);
diff --git a/services/people/java/com/android/server/people/data/DataManager.java b/services/people/java/com/android/server/people/data/DataManager.java
index 763e19bd14ab..8e1141da9df1 100644
--- a/services/people/java/com/android/server/people/data/DataManager.java
+++ b/services/people/java/com/android/server/people/data/DataManager.java
@@ -16,6 +16,8 @@
 
 package com.android.server.people.data;
 
+import static android.app.NotificationChannel.USER_LOCKED_ALLOW_BUBBLE;
+
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.UserIdInt;
@@ -56,6 +58,7 @@ import android.service.notification.NotificationListenerService;
 import android.service.notification.StatusBarNotification;
 import android.telecom.TelecomManager;
 import android.text.format.DateUtils;
+import android.util.ArrayMap;
 import android.util.ArraySet;
 import android.util.Slog;
 import android.util.SparseArray;
@@ -482,7 +485,8 @@ public class DataManager {
     }
 
     @Nullable
-    private EventHistoryImpl getEventHistoryIfEligible(StatusBarNotification sbn) {
+    private PackageData getPackageIfConversationExists(StatusBarNotification sbn,
+            Consumer<ConversationInfo> conversationConsumer) {
         Notification notification = sbn.getNotification();
         String shortcutId = notification.getShortcutId();
         if (shortcutId == null) {
@@ -490,12 +494,16 @@ public class DataManager {
         }
         PackageData packageData = getPackage(sbn.getPackageName(),
                 sbn.getUser().getIdentifier());
-        if (packageData == null
-                || packageData.getConversationStore().getConversation(shortcutId) == null) {
+        if (packageData == null) {
+            return null;
+        }
+        ConversationInfo conversationInfo =
+                packageData.getConversationStore().getConversation(shortcutId);
+        if (conversationInfo == null) {
             return null;
         }
-        return packageData.getEventStore().getOrCreateEventHistory(
-                EventStore.CATEGORY_SHORTCUT_BASED, shortcutId);
+        conversationConsumer.accept(conversationInfo);
+        return packageData;
     }
 
     @VisibleForTesting
@@ -745,10 +753,19 @@ public class DataManager {
     /** Listener for the notifications and their settings changes. */
     private class NotificationListener extends NotificationListenerService {
 
+        // Conversation shortcut ID -> Number of active notifications
+        private final Map<String, Integer> mActiveNotifCounts = new ArrayMap<>();
+
         @Override
         public void onNotificationPosted(StatusBarNotification sbn) {
-            EventHistoryImpl eventHistory = getEventHistoryIfEligible(sbn);
-            if (eventHistory != null) {
+            String shortcutId = sbn.getNotification().getShortcutId();
+            PackageData packageData = getPackageIfConversationExists(sbn, conversationInfo -> {
+                mActiveNotifCounts.merge(shortcutId, 1, Integer::sum);
+            });
+
+            if (packageData != null) {
+                EventHistoryImpl eventHistory = packageData.getEventStore().getOrCreateEventHistory(
+                        EventStore.CATEGORY_SHORTCUT_BASED, shortcutId);
                 eventHistory.addEvent(new Event(sbn.getPostTime(), Event.TYPE_NOTIFICATION_POSTED));
             }
         }
@@ -756,13 +773,32 @@ public class DataManager {
         @Override
         public void onNotificationRemoved(StatusBarNotification sbn, RankingMap rankingMap,
                 int reason) {
-            if (reason != REASON_CLICK) {
-                return;
-            }
-            EventHistoryImpl eventHistory = getEventHistoryIfEligible(sbn);
-            if (eventHistory == null) {
+            String shortcutId = sbn.getNotification().getShortcutId();
+            PackageData packageData = getPackageIfConversationExists(sbn, conversationInfo -> {
+                int count = mActiveNotifCounts.getOrDefault(shortcutId, 0) - 1;
+                if (count <= 0) {
+                    mActiveNotifCounts.remove(sbn.getNotification().getShortcutId());
+                    // The shortcut was cached by Notification Manager synchronously when the
+                    // associated notification was posted. Uncache it here when all the associated
+                    // notifications are removed.
+                    if (conversationInfo.isShortcutCached()
+                            && !conversationInfo.isNotificationSettingChanged()) {
+                        int userId = sbn.getUser().getIdentifier();
+                        mShortcutServiceInternal.uncacheShortcuts(userId,
+                                mContext.getPackageName(), sbn.getPackageName(),
+                                Collections.singletonList(conversationInfo.getShortcutId()),
+                                userId);
+                    }
+                } else {
+                    mActiveNotifCounts.put(shortcutId, count);
+                }
+            });
+
+            if (reason != REASON_CLICK || packageData == null) {
                 return;
             }
+            EventHistoryImpl eventHistory = packageData.getEventStore().getOrCreateEventHistory(
+                    EventStore.CATEGORY_SHORTCUT_BASED, shortcutId);
             long currentTime = System.currentTimeMillis();
             eventHistory.addEvent(new Event(currentTime, Event.TYPE_NOTIFICATION_OPENED));
         }
@@ -780,7 +816,16 @@ public class DataManager {
             if (conversationInfo == null) {
                 return;
             }
+            boolean isNotificationSettingChanged =
+                    conversationInfo.isImportant() != channel.isImportantConversation()
+                            || conversationInfo.isDemoted() != channel.isDemoted()
+                            || channel.hasUserSetImportance()
+                            || (channel.getUserLockedFields() & USER_LOCKED_ALLOW_BUBBLE) != 0;
             ConversationInfo.Builder builder = new ConversationInfo.Builder(conversationInfo);
+            if (modificationType == NOTIFICATION_CHANNEL_OR_GROUP_UPDATED
+                    && isNotificationSettingChanged) {
+                builder.setNotificationSettingChanged(true);
+            }
             switch (modificationType) {
                 case NOTIFICATION_CHANNEL_OR_GROUP_ADDED:
                 case NOTIFICATION_CHANNEL_OR_GROUP_UPDATED:
@@ -802,15 +847,6 @@ public class DataManager {
                     break;
             }
             conversationStore.addOrUpdate(builder.build());
-
-            if (modificationType == NOTIFICATION_CHANNEL_OR_GROUP_UPDATED
-                    && conversationInfo.isShortcutLongLived()
-                    && !conversationInfo.isShortcutCached()) {
-                mShortcutServiceInternal.cacheShortcuts(user.getIdentifier(),
-                        mContext.getPackageName(), pkg,
-                        Collections.singletonList(conversationInfo.getShortcutId()),
-                        user.getIdentifier());
-            }
         }
     }
 
diff --git a/services/tests/servicestests/src/com/android/server/people/data/ConversationInfoTest.java b/services/tests/servicestests/src/com/android/server/people/data/ConversationInfoTest.java
index 70d6cf81c3b0..ea8aa6bb5c03 100644
--- a/services/tests/servicestests/src/com/android/server/people/data/ConversationInfoTest.java
+++ b/services/tests/servicestests/src/com/android/server/people/data/ConversationInfoTest.java
@@ -54,6 +54,7 @@ public final class ConversationInfoTest {
                 .setPersonImportant(true)
                 .setPersonBot(true)
                 .setContactStarred(true)
+                .setNotificationSettingChanged(true)
                 .build();
 
         assertEquals(SHORTCUT_ID, conversationInfo.getShortcutId());
@@ -70,6 +71,7 @@ public final class ConversationInfoTest {
         assertTrue(conversationInfo.isPersonImportant());
         assertTrue(conversationInfo.isPersonBot());
         assertTrue(conversationInfo.isContactStarred());
+        assertTrue(conversationInfo.isNotificationSettingChanged());
     }
 
     @Test
@@ -92,6 +94,7 @@ public final class ConversationInfoTest {
         assertFalse(conversationInfo.isPersonImportant());
         assertFalse(conversationInfo.isPersonBot());
         assertFalse(conversationInfo.isContactStarred());
+        assertFalse(conversationInfo.isNotificationSettingChanged());
     }
 
     @Test
@@ -109,6 +112,7 @@ public final class ConversationInfoTest {
                 .setPersonImportant(true)
                 .setPersonBot(true)
                 .setContactStarred(true)
+                .setNotificationSettingChanged(true)
                 .build();
 
         ConversationInfo destination = new ConversationInfo.Builder(source)
@@ -128,5 +132,6 @@ public final class ConversationInfoTest {
         assertTrue(destination.isPersonImportant());
         assertTrue(destination.isPersonBot());
         assertFalse(destination.isContactStarred());
+        assertTrue(destination.isNotificationSettingChanged());
     }
 }
diff --git a/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java b/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java
index e16f3145de0b..e51ab9dbdd9b 100644
--- a/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java
@@ -34,6 +34,7 @@ import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -407,6 +408,7 @@ public final class DataManagerTest {
 
         ShortcutInfo shortcut = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, TEST_SHORTCUT_ID,
                 buildPerson());
+        shortcut.setCached();
         mDataManager.addOrUpdateConversationInfo(shortcut);
 
         NotificationListenerService listenerService =
@@ -418,6 +420,68 @@ public final class DataManagerTest {
         List<Range<Long>> activeNotificationOpenTimeSlots = getActiveSlotsForTestShortcut(
                 Event.NOTIFICATION_EVENT_TYPES);
         assertEquals(1, activeNotificationOpenTimeSlots.size());
+        verify(mShortcutServiceInternal).uncacheShortcuts(
+                anyInt(), any(), eq(TEST_PKG_NAME),
+                eq(Collections.singletonList(TEST_SHORTCUT_ID)), eq(USER_ID_PRIMARY));
+    }
+
+    @Test
+    public void testNotificationDismissed() {
+        mDataManager.onUserUnlocked(USER_ID_PRIMARY);
+
+        ShortcutInfo shortcut = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, TEST_SHORTCUT_ID,
+                buildPerson());
+        mDataManager.addOrUpdateConversationInfo(shortcut);
+
+        NotificationListenerService listenerService =
+                mDataManager.getNotificationListenerServiceForTesting(USER_ID_PRIMARY);
+
+        // Post one notification.
+        shortcut.setCached();
+        mDataManager.addOrUpdateConversationInfo(shortcut);
+        listenerService.onNotificationPosted(mStatusBarNotification);
+
+        // Post another notification.
+        listenerService.onNotificationPosted(mStatusBarNotification);
+
+        // Removing one of the two notifications does not un-cache the shortcut.
+        listenerService.onNotificationRemoved(mStatusBarNotification, null,
+                NotificationListenerService.REASON_CANCEL);
+        verify(mShortcutServiceInternal, never()).uncacheShortcuts(
+                anyInt(), any(), anyString(), any(), anyInt());
+
+        // Removing the second notification un-caches the shortcut.
+        listenerService.onNotificationRemoved(mStatusBarNotification, null,
+                NotificationListenerService.REASON_CANCEL_ALL);
+        verify(mShortcutServiceInternal).uncacheShortcuts(
+                anyInt(), any(), eq(TEST_PKG_NAME),
+                eq(Collections.singletonList(TEST_SHORTCUT_ID)), eq(USER_ID_PRIMARY));
+    }
+
+    @Test
+    public void testShortcutNotUncachedIfSettingChanged() {
+        mDataManager.onUserUnlocked(USER_ID_PRIMARY);
+
+        ShortcutInfo shortcut = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, TEST_SHORTCUT_ID,
+                buildPerson());
+        mDataManager.addOrUpdateConversationInfo(shortcut);
+
+        NotificationListenerService listenerService =
+                mDataManager.getNotificationListenerServiceForTesting(USER_ID_PRIMARY);
+
+        listenerService.onNotificationPosted(mStatusBarNotification);
+        shortcut.setCached();
+        mDataManager.addOrUpdateConversationInfo(shortcut);
+
+        mNotificationChannel.setImportantConversation(true);
+        listenerService.onNotificationChannelModified(TEST_PKG_NAME, UserHandle.of(USER_ID_PRIMARY),
+                mNotificationChannel, NOTIFICATION_CHANNEL_OR_GROUP_UPDATED);
+
+        listenerService.onNotificationRemoved(mStatusBarNotification, null,
+                NotificationListenerService.REASON_CANCEL_ALL);
+        verify(mShortcutServiceInternal, never()).uncacheShortcuts(
+                anyInt(), any(), eq(TEST_PKG_NAME),
+                eq(Collections.singletonList(TEST_SHORTCUT_ID)), eq(USER_ID_PRIMARY));
     }
 
     @Test
@@ -466,9 +530,7 @@ public final class DataManagerTest {
         assertTrue(conversationInfo.isImportant());
         assertFalse(conversationInfo.isNotificationSilenced());
         assertFalse(conversationInfo.isDemoted());
-        verify(mShortcutServiceInternal).cacheShortcuts(
-                anyInt(), any(), eq(TEST_PKG_NAME),
-                eq(Collections.singletonList(TEST_SHORTCUT_ID)), eq(USER_ID_PRIMARY));
+        assertTrue(conversationInfo.isNotificationSettingChanged());
     }
 
     @Test
diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
index 7a5e2266e62f..509d242f69cf 100755
--- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -6053,6 +6053,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
     @Test
     public void testNotificationBubbles_flagRemoved_whenShortcutRemoved()
             throws RemoteException {
+        final String shortcutId = "someshortcutId";
         setUpPrefsForBubbles(PKG, mUid,
                 true /* global */,
                 BUBBLE_PREFERENCE_ALL /* app */,
@@ -6063,9 +6064,10 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
 
         // Messaging notification with shortcut info
         Notification.BubbleMetadata metadata =
-                new Notification.BubbleMetadata.Builder("someshortcutId").build();
+                new Notification.BubbleMetadata.Builder(shortcutId).build();
         Notification.Builder nb = getMessageStyleNotifBuilder(false /* addDefaultMetadata */,
                 null /* groupKey */, false /* isSummary */);
+        nb.setShortcutId(shortcutId);
         nb.setBubbleMetadata(metadata);
         StatusBarNotification sbn = new StatusBarNotification(PKG, PKG, 1,
                 "tag", mUid, 0, nb.build(), new UserHandle(mUid), null, 0);
@@ -6075,7 +6077,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
         List<ShortcutInfo> shortcutInfos = new ArrayList<>();
         ShortcutInfo info = mock(ShortcutInfo.class);
         when(info.getPackage()).thenReturn(PKG);
-        when(info.getId()).thenReturn("someshortcutId");
+        when(info.getId()).thenReturn(shortcutId);
         when(info.getUserId()).thenReturn(USER_SYSTEM);
         when(info.isLongLived()).thenReturn(true);
         when(info.isEnabled()).thenReturn(true);
@@ -6098,6 +6100,11 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
         Notification notif = mService.getNotificationRecord(nr.getSbn().getKey()).getNotification();
         assertTrue(notif.isBubbleNotification());
 
+        // Make sure the shortcut is cached.
+        verify(mShortcutServiceInternal).cacheShortcuts(
+                anyInt(), any(), eq(PKG), eq(Collections.singletonList(shortcutId)),
+                eq(USER_SYSTEM));
+
         // Test: Remove the shortcut
         when(mLauncherApps.getShortcuts(any(), any())).thenReturn(null);
         launcherAppsCallback.getValue().onShortcutsChanged(PKG, Collections.emptyList(),
@@ -6119,6 +6126,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
     @Test
     public void testNotificationBubbles_shortcut_stopListeningWhenNotifRemoved()
             throws RemoteException {
+        final String shortcutId = "someshortcutId";
         setUpPrefsForBubbles(PKG, mUid,
                 true /* global */,
                 BUBBLE_PREFERENCE_ALL /* app */,
@@ -6129,9 +6137,10 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
 
         // Messaging notification with shortcut info
         Notification.BubbleMetadata metadata = new Notification.BubbleMetadata.Builder(
-                "someshortcutId").build();
+                shortcutId).build();
         Notification.Builder nb = getMessageStyleNotifBuilder(false /* addDefaultMetadata */,
                 null /* groupKey */, false /* isSummary */);
+        nb.setShortcutId(shortcutId);
         nb.setBubbleMetadata(metadata);
         StatusBarNotification sbn = new StatusBarNotification(PKG, PKG, 1,
                 "tag", mUid, 0, nb.build(), new UserHandle(mUid), null, 0);
@@ -6141,7 +6150,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
         List<ShortcutInfo> shortcutInfos = new ArrayList<>();
         ShortcutInfo info = mock(ShortcutInfo.class);
         when(info.getPackage()).thenReturn(PKG);
-        when(info.getId()).thenReturn("someshortcutId");
+        when(info.getId()).thenReturn(shortcutId);
         when(info.getUserId()).thenReturn(USER_SYSTEM);
         when(info.isLongLived()).thenReturn(true);
         when(info.isEnabled()).thenReturn(true);
@@ -6164,6 +6173,11 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
         Notification notif = mService.getNotificationRecord(nr.getSbn().getKey()).getNotification();
         assertTrue(notif.isBubbleNotification());
 
+        // Make sure the shortcut is cached.
+        verify(mShortcutServiceInternal).cacheShortcuts(
+                anyInt(), any(), eq(PKG), eq(Collections.singletonList(shortcutId)),
+                eq(USER_SYSTEM));
+
         // Test: Remove the notification
         mBinderService.cancelNotificationWithTag(PKG, PKG, nr.getSbn().getTag(),
                 nr.getSbn().getId(), nr.getSbn().getUserId());