Merge "Uncache the shortcuts when Android system shuts down if the notification settings are not changed" into rvc-dev

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

@@ -62,8 +62,6 @@ 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,
@@ -72,7 +70,6 @@ public class ConversationInfo {
             FLAG_PERSON_BOT,
             FLAG_CONTACT_STARRED,
             FLAG_DEMOTED,
-            FLAG_NOTIFICATION_SETTING_CHANGED,
     })
     @Retention(RetentionPolicy.SOURCE)
     private @interface ConversationFlags {
@@ -188,11 +185,6 @@ 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) {
@@ -499,10 +491,6 @@ 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);

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

@@ -16,8 +16,6 @@
 
 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;
@@ -60,9 +58,11 @@ import android.telecom.TelecomManager;
 import android.text.format.DateUtils;
 import android.util.ArrayMap;
 import android.util.ArraySet;
+import android.util.Pair;
 import android.util.Slog;
 import android.util.SparseArray;
 
+import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.app.ChooserActivity;
 import com.android.internal.content.PackageMonitor;
@@ -106,8 +106,7 @@ public class DataManager {
     private final SparseArray<BroadcastReceiver> mBroadcastReceivers = new SparseArray<>();
     private final SparseArray<ContentObserver> mContactsContentObservers = new SparseArray<>();
     private final SparseArray<ScheduledFuture<?>> mUsageStatsQueryFutures = new SparseArray<>();
-    private final SparseArray<NotificationListenerService> mNotificationListeners =
+    private final SparseArray<NotificationListener> mNotificationListeners = new SparseArray<>();
-            new SparseArray<>();
     private final SparseArray<PackageMonitor> mPackageMonitors = new SparseArray<>();
     private ContentObserver mCallLogContentObserver;
     private ContentObserver mMmsSmsContentObserver;
@@ -272,6 +271,7 @@ public class DataManager {
         }
         pruneUninstalledPackageData(userData);
 
+        final NotificationListener notificationListener = mNotificationListeners.get(userId);
         userData.forAllPackages(packageData -> {
             if (signal.isCanceled()) {
                 return;
@@ -284,6 +284,20 @@ public class DataManager {
                 packageData.getEventStore().deleteEventHistories(EventStore.CATEGORY_SMS);
             }
             packageData.pruneOrphanEvents();
+            if (notificationListener != null) {
+                String packageName = packageData.getPackageName();
+                packageData.forAllConversations(conversationInfo -> {
+                    if (conversationInfo.isShortcutCached()
+                            && conversationInfo.getNotificationChannelId() == null
+                            && !notificationListener.hasActiveNotifications(
+                                    packageName, conversationInfo.getShortcutId())) {
+                        mShortcutServiceInternal.uncacheShortcuts(userId,
+                                mContext.getPackageName(), packageName,
+                                Collections.singletonList(conversationInfo.getShortcutId()),
+                                userId);
+                    }
+                });
+            }
         });
     }
 
@@ -337,7 +351,7 @@ public class DataManager {
                     Contacts.CONTENT_URI, /* notifyForDescendants= */ true,
                     contactsContentObserver, userId);
 
-            NotificationListener notificationListener = new NotificationListener();
+            NotificationListener notificationListener = new NotificationListener(userId);
             mNotificationListeners.put(userId, notificationListener);
             try {
                 notificationListener.registerAsSystemService(mContext,
@@ -753,14 +767,27 @@ 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 int mUserId;
-        private final Map<String, Integer> mActiveNotifCounts = new ArrayMap<>();
+
+        // Conversation package name + shortcut ID -> Number of active notifications
+        @GuardedBy("this")
+        private final Map<Pair<String, String>, Integer> mActiveNotifCounts = new ArrayMap<>();
+
+        private NotificationListener(int userId) {
+            mUserId = userId;
+        }
 
         @Override
         public void onNotificationPosted(StatusBarNotification sbn) {
+            if (sbn.getUser().getIdentifier() != mUserId) {
+                return;
+            }
             String shortcutId = sbn.getNotification().getShortcutId();
             PackageData packageData = getPackageIfConversationExists(sbn, conversationInfo -> {
-                mActiveNotifCounts.merge(shortcutId, 1, Integer::sum);
+                synchronized (this) {
+                    mActiveNotifCounts.merge(
+                            Pair.create(sbn.getPackageName(), shortcutId), 1, Integer::sum);
+                }
             });
 
             if (packageData != null) {
@@ -771,26 +798,32 @@ public class DataManager {
         }
 
         @Override
-        public void onNotificationRemoved(StatusBarNotification sbn, RankingMap rankingMap,
+        public synchronized void onNotificationRemoved(StatusBarNotification sbn,
-                int reason) {
+                RankingMap rankingMap, int reason) {
+            if (sbn.getUser().getIdentifier() != mUserId) {
+                return;
+            }
             String shortcutId = sbn.getNotification().getShortcutId();
             PackageData packageData = getPackageIfConversationExists(sbn, conversationInfo -> {
-                int count = mActiveNotifCounts.getOrDefault(shortcutId, 0) - 1;
+                Pair<String, String> conversationKey =
+                        Pair.create(sbn.getPackageName(), shortcutId);
+                synchronized (this) {
+                    int count = mActiveNotifCounts.getOrDefault(conversationKey, 0) - 1;
                     if (count <= 0) {
-                    mActiveNotifCounts.remove(sbn.getNotification().getShortcutId());
+                        mActiveNotifCounts.remove(conversationKey);
                         // The shortcut was cached by Notification Manager synchronously when the
-                    // associated notification was posted. Uncache it here when all the associated
+                        // associated notification was posted. Uncache it here when all the
-                    // notifications are removed.
+                        // associated notifications are removed.
                         if (conversationInfo.isShortcutCached()
-                            && !conversationInfo.isNotificationSettingChanged()) {
+                                && conversationInfo.getNotificationChannelId() == null) {
-                        int userId = sbn.getUser().getIdentifier();
+                            mShortcutServiceInternal.uncacheShortcuts(mUserId,
-                        mShortcutServiceInternal.uncacheShortcuts(userId,
                                     mContext.getPackageName(), sbn.getPackageName(),
                                     Collections.singletonList(conversationInfo.getShortcutId()),
-                                userId);
+                                    mUserId);
                         }
                     } else {
-                    mActiveNotifCounts.put(shortcutId, count);
+                        mActiveNotifCounts.put(conversationKey, count);
+                    }
                 }
             });
 
@@ -806,6 +839,9 @@ public class DataManager {
         @Override
         public void onNotificationChannelModified(String pkg, UserHandle user,
                 NotificationChannel channel, int modificationType) {
+            if (user.getIdentifier() != mUserId) {
+                return;
+            }
             PackageData packageData = getPackage(pkg, user.getIdentifier());
             String shortcutId = channel.getConversationId();
             if (packageData == null || shortcutId == null) {
@@ -816,16 +852,7 @@ 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:
@@ -848,6 +875,28 @@ public class DataManager {
             }
             conversationStore.addOrUpdate(builder.build());
         }
+
+        synchronized void cleanupCachedShortcuts() {
+            for (Pair<String, String> conversationKey : mActiveNotifCounts.keySet()) {
+                String packageName = conversationKey.first;
+                String shortcutId = conversationKey.second;
+                PackageData packageData = getPackage(packageName, mUserId);
+                ConversationInfo conversationInfo =
+                        packageData != null ? packageData.getConversationInfo(shortcutId) : null;
+                if (conversationInfo != null
+                        && conversationInfo.isShortcutCached()
+                        && conversationInfo.getNotificationChannelId() == null) {
+                    mShortcutServiceInternal.uncacheShortcuts(mUserId,
+                            mContext.getPackageName(), packageName,
+                            Collections.singletonList(shortcutId),
+                            mUserId);
+                }
+            }
+        }
+
+        synchronized boolean hasActiveNotifications(String packageName, String shortcutId) {
+            return mActiveNotifCounts.containsKey(Pair.create(packageName, shortcutId));
+        }
     }
 
     /**
@@ -917,7 +966,16 @@ public class DataManager {
 
         @Override
         public void onReceive(Context context, Intent intent) {
-            forAllUnlockedUsers(userData -> userData.forAllPackages(PackageData::saveToDisk));
+            forAllUnlockedUsers(userData -> {
+                NotificationListener listener = mNotificationListeners.get(userData.getUserId());
+                // Clean up the cached shortcuts because all the notifications are cleared after
+                // system shutdown. The associated shortcuts need to be uncached to keep in sync
+                // unless the settings are changed by the user.
+                if (listener != null) {
+                    listener.cleanupCachedShortcuts();
+                }
+                userData.forAllPackages(PackageData::saveToDisk);
+            });
         }
     }
 

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

@@ -54,7 +54,6 @@ public final class ConversationInfoTest {
                 .setPersonImportant(true)
                 .setPersonBot(true)
                 .setContactStarred(true)
-                .setNotificationSettingChanged(true)
                 .build();
 
         assertEquals(SHORTCUT_ID, conversationInfo.getShortcutId());
@@ -71,7 +70,6 @@ public final class ConversationInfoTest {
         assertTrue(conversationInfo.isPersonImportant());
         assertTrue(conversationInfo.isPersonBot());
         assertTrue(conversationInfo.isContactStarred());
-        assertTrue(conversationInfo.isNotificationSettingChanged());
     }
 
     @Test
@@ -94,7 +92,6 @@ public final class ConversationInfoTest {
         assertFalse(conversationInfo.isPersonImportant());
         assertFalse(conversationInfo.isPersonBot());
         assertFalse(conversationInfo.isContactStarred());
-        assertFalse(conversationInfo.isNotificationSettingChanged());
     }
 
     @Test
@@ -112,7 +109,6 @@ public final class ConversationInfoTest {
                 .setPersonImportant(true)
                 .setPersonBot(true)
                 .setContactStarred(true)
-                .setNotificationSettingChanged(true)
                 .build();
 
         ConversationInfo destination = new ConversationInfo.Builder(source)
@@ -132,6 +128,5 @@ public final class ConversationInfoTest {
         assertTrue(destination.isPersonImportant());
         assertTrue(destination.isPersonBot());
         assertFalse(destination.isContactStarred());
-        assertTrue(destination.isNotificationSettingChanged());
     }
 }

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

@@ -49,6 +49,7 @@ import android.app.prediction.AppTarget;
 import android.app.prediction.AppTargetEvent;
 import android.app.prediction.AppTargetId;
 import android.app.usage.UsageStatsManagerInternal;
+import android.content.BroadcastReceiver;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.content.Intent;
@@ -95,8 +96,6 @@ import java.util.List;
 import java.util.Set;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.ScheduledFuture;
-import java.util.concurrent.TimeUnit;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 
@@ -125,18 +124,19 @@ public final class DataManagerTest {
     @Mock private TelephonyManager mTelephonyManager;
     @Mock private TelecomManager mTelecomManager;
     @Mock private ContentResolver mContentResolver;
-    @Mock private ScheduledExecutorService mExecutorService;
     @Mock private JobScheduler mJobScheduler;
-    @Mock private ScheduledFuture mScheduledFuture;
     @Mock private StatusBarNotification mStatusBarNotification;
     @Mock private Notification mNotification;
 
     @Captor private ArgumentCaptor<ShortcutChangeCallback> mShortcutChangeCallbackCaptor;
+    @Captor private ArgumentCaptor<BroadcastReceiver> mBroadcastReceiverCaptor;
 
+    private ScheduledExecutorService mExecutorService;
     private NotificationChannel mNotificationChannel;
     private DataManager mDataManager;
     private CancellationSignal mCancellationSignal;
     private ShortcutChangeCallback mShortcutChangeCallback;
+    private BroadcastReceiver mShutdownBroadcastReceiver;
     private TestInjector mInjector;
 
     @Before
@@ -182,13 +182,7 @@ public final class DataManagerTest {
         when(mContext.getSystemServiceName(JobScheduler.class)).thenReturn(
                 Context.JOB_SCHEDULER_SERVICE);
 
-        when(mExecutorService.scheduleAtFixedRate(any(Runnable.class), anyLong(), anyLong(), any(
+        mExecutorService = new MockScheduledExecutorService();
-                TimeUnit.class))).thenReturn(mScheduledFuture);
-        doAnswer(ans -> {
-            Runnable runnable = (Runnable) ans.getArguments()[0];
-            runnable.run();
-            return null;
-        }).when(mExecutorService).execute(any(Runnable.class));
 
         when(mUserManager.getEnabledProfiles(USER_ID_PRIMARY))
                 .thenReturn(Arrays.asList(
@@ -221,6 +215,9 @@ public final class DataManagerTest {
         verify(mShortcutServiceInternal).addShortcutChangeCallback(
                 mShortcutChangeCallbackCaptor.capture());
         mShortcutChangeCallback = mShortcutChangeCallbackCaptor.getValue();
+
+        verify(mContext).registerReceiver(mBroadcastReceiverCaptor.capture(), any());
+        mShutdownBroadcastReceiver = mBroadcastReceiverCaptor.getValue();
     }
 
     @After
@@ -459,7 +456,7 @@ public final class DataManagerTest {
     }
 
     @Test
-    public void testShortcutNotUncachedIfSettingChanged() {
+    public void testShortcutNotUncachedIfNotificationChannelCreated() {
         mDataManager.onUserUnlocked(USER_ID_PRIMARY);
 
         ShortcutInfo shortcut = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, TEST_SHORTCUT_ID,
@@ -473,7 +470,6 @@ public final class DataManagerTest {
         shortcut.setCached();
         mDataManager.addOrUpdateConversationInfo(shortcut);
 
-        mNotificationChannel.setImportantConversation(true);
         listenerService.onNotificationChannelModified(TEST_PKG_NAME, UserHandle.of(USER_ID_PRIMARY),
                 mNotificationChannel, NOTIFICATION_CHANNEL_OR_GROUP_UPDATED);
 
@@ -530,7 +526,6 @@ public final class DataManagerTest {
         assertTrue(conversationInfo.isImportant());
         assertFalse(conversationInfo.isNotificationSilenced());
         assertFalse(conversationInfo.isDemoted());
-        assertTrue(conversationInfo.isNotificationSettingChanged());
     }
 
     @Test
@@ -562,6 +557,51 @@ public final class DataManagerTest {
         assertFalse(conversationInfo.isDemoted());
     }
 
+    @Test
+    public void testUncacheShortcutWhenShutdown() {
+        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);
+
+        mShutdownBroadcastReceiver.onReceive(mContext, new Intent());
+        verify(mShortcutServiceInternal).uncacheShortcuts(
+                anyInt(), any(), eq(TEST_PKG_NAME),
+                eq(Collections.singletonList(TEST_SHORTCUT_ID)), eq(USER_ID_PRIMARY));
+    }
+
+    @Test
+    public void testDoNotUncacheShortcutWhenShutdownIfNotificationChannelCreated() {
+        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);
+
+        listenerService.onNotificationChannelModified(TEST_PKG_NAME, UserHandle.of(USER_ID_PRIMARY),
+                mNotificationChannel, NOTIFICATION_CHANNEL_OR_GROUP_UPDATED);
+
+        mShutdownBroadcastReceiver.onReceive(mContext, new Intent());
+        verify(mShortcutServiceInternal, never()).uncacheShortcuts(
+                anyInt(), any(), eq(TEST_PKG_NAME),
+                eq(Collections.singletonList(TEST_SHORTCUT_ID)), eq(USER_ID_PRIMARY));
+    }
+
     @Test
     public void testShortcutAddedOrUpdated() {
         mDataManager.onUserUnlocked(USER_ID_PRIMARY);
@@ -721,6 +761,22 @@ public final class DataManagerTest {
         assertTrue(activeTimeSlots.isEmpty());
     }
 
+    @Test
+    public void testPruneInactiveCachedShortcuts() {
+        mDataManager.onUserUnlocked(USER_ID_PRIMARY);
+
+        ShortcutInfo shortcut = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, TEST_SHORTCUT_ID,
+                buildPerson());
+        shortcut.setCached();
+        mDataManager.addOrUpdateConversationInfo(shortcut);
+
+        mDataManager.pruneDataForUser(USER_ID_PRIMARY, mCancellationSignal);
+
+        verify(mShortcutServiceInternal).uncacheShortcuts(
+                anyInt(), any(), eq(TEST_PKG_NAME),
+                eq(Collections.singletonList(TEST_SHORTCUT_ID)), eq(USER_ID_PRIMARY));
+    }
+
     @Test
     public void testBackupAndRestoration()
             throws IntentFilter.MalformedMimeTypeException {

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

@@ -96,7 +96,6 @@ class MockScheduledExecutorService implements ScheduledExecutorService {
     @Override
     public ScheduledFuture<?> scheduleAtFixedRate(Runnable command, long initialDelay, long period,
             TimeUnit unit) {
-        Preconditions.checkState(unit == TimeUnit.MILLISECONDS);
         return new MockScheduledFuture<>(command, period, unit);
     }