diff options
author | Danning Chen <danningc@google.com> | 2020-04-27 18:07:02 -0700 |
---|---|---|
committer | Danning Chen <danningc@google.com> | 2020-04-30 23:37:56 -0700 |
commit | f4ba1c22a6c95e5250bd065c76106190d77d9a93 (patch) | |
tree | 6a3b96e85ed2611b29d37c753c405d85b549e0ea | |
parent | 761c9334805a6bd6973446f5dc099a85c0769267 (diff) |
Uncache the shortcuts when Android system shuts down if the notification settings are not changed
Change-Id: Ib7e23f4b15ccca9c162cea187a26e7d89b8976a8
Test: Manual test
Test: atest DataManagerTest
Bug: 153357090
5 files changed, 163 insertions, 67 deletions
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 27fa36b70b8f..dc3fa2a048f6 100644 --- a/services/people/java/com/android/server/people/data/ConversationInfo.java +++ b/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); 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 8e1141da9df1..d9ca4152026e 100644 --- a/services/people/java/com/android/server/people/data/DataManager.java +++ b/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 = - new SparseArray<>(); + private final SparseArray<NotificationListener> mNotificationListeners = 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 Map<String, Integer> mActiveNotifCounts = new ArrayMap<>(); + private final int mUserId; + + // 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, - int reason) { + public synchronized void onNotificationRemoved(StatusBarNotification sbn, + 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; - 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); + Pair<String, String> conversationKey = + Pair.create(sbn.getPackageName(), shortcutId); + synchronized (this) { + int count = mActiveNotifCounts.getOrDefault(conversationKey, 0) - 1; + if (count <= 0) { + mActiveNotifCounts.remove(conversationKey); + // 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.getNotificationChannelId() == null) { + mShortcutServiceInternal.uncacheShortcuts(mUserId, + mContext.getPackageName(), sbn.getPackageName(), + Collections.singletonList(conversationInfo.getShortcutId()), + mUserId); + } + } else { + mActiveNotifCounts.put(conversationKey, count); } - } else { - mActiveNotifCounts.put(shortcutId, 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); + }); } } 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 ea8aa6bb5c03..70d6cf81c3b0 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,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()); } } 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 e51ab9dbdd9b..1a2032ac15d0 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 @@ -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( - TimeUnit.class))).thenReturn(mScheduledFuture); - doAnswer(ans -> { - Runnable runnable = (Runnable) ans.getArguments()[0]; - runnable.run(); - return null; - }).when(mExecutorService).execute(any(Runnable.class)); + mExecutorService = new MockScheduledExecutorService(); 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 @@ -563,6 +558,51 @@ public final class DataManagerTest { } @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); @@ -722,6 +762,22 @@ public final class DataManagerTest { } @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 { mDataManager.onUserUnlocked(USER_ID_PRIMARY); diff --git a/services/tests/servicestests/src/com/android/server/people/data/MockScheduledExecutorService.java b/services/tests/servicestests/src/com/android/server/people/data/MockScheduledExecutorService.java index aecbc8d031e1..8cb846ff9b14 100644 --- a/services/tests/servicestests/src/com/android/server/people/data/MockScheduledExecutorService.java +++ b/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); } |