Merge "Cache the shortcut when the associated notifications are posted and uncache when they are gone" into rvc-dev

This commit is contained in:
Danning Chen
2020-04-23 23:23:40 +00:00
committed by Android (Google) Code Review
7 changed files with 177 additions and 28 deletions

View File

@@ -5699,6 +5699,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();

View File

@@ -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;
@@ -185,6 +186,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.
*

View File

@@ -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);

View File

@@ -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;
}
return packageData.getEventStore().getOrCreateEventHistory(
EventStore.CATEGORY_SHORTCUT_BASED, shortcutId);
ConversationInfo conversationInfo =
packageData.getConversationStore().getConversation(shortcutId);
if (conversationInfo == null) {
return null;
}
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());
}
}
}

View File

@@ -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());
}
}

View File

@@ -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

View File

@@ -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());