From 07cfaa6b08e99a76c46cda1334f02fca3eb75707 Mon Sep 17 00:00:00 2001 From: Danning Chen Date: Mon, 20 Apr 2020 15:16:41 -0700 Subject: [PATCH] Keep the conversation shortcut criteria in People Service consistent with the one in Notification Manager Before this change, People Service uses the presence of the Person object in ShortcutInfo as the criteria of conversation shortcut. This changes the criteria that the shortcut needs to be a share shortcut instead of having Person object. Change-Id: I1ea52a50c909ca96365c1d4e55af97931d048d8f Test: atest ShortcutHelperTest Test: atest DataManagerTest Bug: 154254830 --- .../server/notification/ShortcutHelper.java | 41 ++++++++++++------- .../server/people/data/DataManager.java | 8 ++-- .../server/people/data/DataManagerTest.java | 2 + .../NotificationManagerServiceTest.java | 12 ++++++ .../notification/ShortcutHelperTest.java | 12 ++++++ 5 files changed, 55 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/notification/ShortcutHelper.java b/services/core/java/com/android/server/notification/ShortcutHelper.java index 1d48438229311..96da649350b04 100644 --- a/services/core/java/com/android/server/notification/ShortcutHelper.java +++ b/services/core/java/com/android/server/notification/ShortcutHelper.java @@ -21,7 +21,6 @@ import static android.content.pm.LauncherApps.ShortcutQuery.FLAG_MATCH_DYNAMIC; import static android.content.pm.LauncherApps.ShortcutQuery.FLAG_MATCH_PINNED; import android.annotation.NonNull; -import android.content.Intent; import android.content.IntentFilter; import android.content.pm.LauncherApps; import android.content.pm.ShortcutInfo; @@ -41,9 +40,18 @@ import java.util.List; /** * Helper for querying shortcuts. */ -class ShortcutHelper { +public class ShortcutHelper { private static final String TAG = "ShortcutHelper"; + private static final IntentFilter SHARING_FILTER = new IntentFilter(); + static { + try { + SHARING_FILTER.addDataType("*/*"); + } catch (IntentFilter.MalformedMimeTypeException e) { + Slog.e(TAG, "Bad mime type", e); + } + } + /** * Listener to call when a shortcut we're tracking has been removed. */ @@ -54,7 +62,6 @@ class ShortcutHelper { private LauncherApps mLauncherAppsService; private ShortcutListener mShortcutListener; private ShortcutServiceInternal mShortcutServiceInternal; - private IntentFilter mSharingFilter; // Key: packageName Value: private HashMap> mActiveShortcutBubbles = new HashMap<>(); @@ -122,12 +129,6 @@ class ShortcutHelper { ShortcutServiceInternal shortcutServiceInternal) { mLauncherAppsService = launcherApps; mShortcutListener = listener; - mSharingFilter = new IntentFilter(); - try { - mSharingFilter.addDataType("*/*"); - } catch (IntentFilter.MalformedMimeTypeException e) { - Slog.e(TAG, "Bad mime type", e); - } mShortcutServiceInternal = shortcutServiceInternal; } @@ -142,7 +143,21 @@ class ShortcutHelper { } /** - * Only returns shortcut info if it's found and if it's {@link ShortcutInfo#isLongLived()}. + * Returns whether the given shortcut info is a conversation shortcut. + */ + public static boolean isConversationShortcut( + ShortcutInfo shortcutInfo, ShortcutServiceInternal mShortcutServiceInternal, + int callingUserId) { + if (shortcutInfo == null || !shortcutInfo.isLongLived() || !shortcutInfo.isEnabled()) { + return false; + } + return mShortcutServiceInternal.isSharingShortcut(callingUserId, "android", + shortcutInfo.getPackage(), shortcutInfo.getId(), shortcutInfo.getUserId(), + SHARING_FILTER); + } + + /** + * Only returns shortcut info if it's found and if it's a conversation shortcut. */ ShortcutInfo getValidShortcutInfo(String shortcutId, String packageName, UserHandle user) { if (mLauncherAppsService == null) { @@ -161,11 +176,7 @@ class ShortcutHelper { ShortcutInfo info = shortcuts != null && shortcuts.size() > 0 ? shortcuts.get(0) : null; - if (info == null || !info.isLongLived() || !info.isEnabled()) { - return null; - } - if (mShortcutServiceInternal.isSharingShortcut(user.getIdentifier(), - "android", packageName, shortcutId, user.getIdentifier(), mSharingFilter)) { + if (isConversationShortcut(info, mShortcutServiceInternal, user.getIdentifier())) { return info; } return null; 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 c87ece29800ce..763e19bd14abf 100644 --- a/services/people/java/com/android/server/people/data/DataManager.java +++ b/services/people/java/com/android/server/people/data/DataManager.java @@ -67,6 +67,7 @@ import com.android.internal.os.BackgroundThread; import com.android.internal.telephony.SmsApplication; import com.android.server.LocalServices; import com.android.server.notification.NotificationManagerInternal; +import com.android.server.notification.ShortcutHelper; import java.util.ArrayList; import java.util.Collections; @@ -497,10 +498,6 @@ public class DataManager { EventStore.CATEGORY_SHORTCUT_BASED, shortcutId); } - private boolean isPersonShortcut(@NonNull ShortcutInfo shortcutInfo) { - return shortcutInfo.getPersons() != null && shortcutInfo.getPersons().length != 0; - } - @VisibleForTesting @WorkerThread void addOrUpdateConversationInfo(@NonNull ShortcutInfo shortcutInfo) { @@ -712,7 +709,8 @@ public class DataManager { @NonNull List shortcuts, @NonNull UserHandle user) { mInjector.getBackgroundExecutor().execute(() -> { for (ShortcutInfo shortcut : shortcuts) { - if (isPersonShortcut(shortcut)) { + if (ShortcutHelper.isConversationShortcut( + shortcut, mShortcutServiceInternal, user.getIdentifier())) { addOrUpdateConversationInfo(shortcut); } } 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 728e1492c0d53..e16f3145de0b0 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 @@ -215,6 +215,8 @@ public final class DataManagerTest { mDataManager = new DataManager(mContext, mInjector); mDataManager.initialize(); + when(mShortcutServiceInternal.isSharingShortcut(anyInt(), anyString(), anyString(), + anyString(), anyInt(), any())).thenReturn(true); verify(mShortcutServiceInternal).addShortcutChangeCallback( mShortcutChangeCallbackCaptor.capture()); mShortcutChangeCallback = mShortcutChangeCallbackCaptor.getValue(); 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 ecdd9e548e6a2..7a5e2266e62ff 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -6074,6 +6074,9 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { // Pretend the shortcut exists List shortcutInfos = new ArrayList<>(); ShortcutInfo info = mock(ShortcutInfo.class); + when(info.getPackage()).thenReturn(PKG); + when(info.getId()).thenReturn("someshortcutId"); + when(info.getUserId()).thenReturn(USER_SYSTEM); when(info.isLongLived()).thenReturn(true); when(info.isEnabled()).thenReturn(true); shortcutInfos.add(info); @@ -6137,6 +6140,9 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { // Pretend the shortcut exists List shortcutInfos = new ArrayList<>(); ShortcutInfo info = mock(ShortcutInfo.class); + when(info.getPackage()).thenReturn(PKG); + when(info.getId()).thenReturn("someshortcutId"); + when(info.getUserId()).thenReturn(USER_SYSTEM); when(info.isLongLived()).thenReturn(true); when(info.isEnabled()).thenReturn(true); shortcutInfos.add(info); @@ -6483,6 +6489,9 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { when(mPreferencesHelper.getConversations(anyString(), anyInt())).thenReturn(convos); ShortcutInfo si = mock(ShortcutInfo.class); + when(si.getPackage()).thenReturn(PKG_P); + when(si.getId()).thenReturn("convo"); + when(si.getUserId()).thenReturn(USER_SYSTEM); when(si.getShortLabel()).thenReturn("Hello"); when(si.isLongLived()).thenReturn(true); when(si.isEnabled()).thenReturn(true); @@ -6514,6 +6523,9 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { when(mPreferencesHelper.getConversations(anyString(), anyInt())).thenReturn(convos); ShortcutInfo si = mock(ShortcutInfo.class); + when(si.getPackage()).thenReturn(PKG_P); + when(si.getId()).thenReturn("convo"); + when(si.getUserId()).thenReturn(USER_SYSTEM); when(si.getShortLabel()).thenReturn("Hello"); when(si.isLongLived()).thenReturn(false); when(mLauncherApps.getShortcuts(any(), any())).thenReturn(Arrays.asList(si)); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/ShortcutHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/ShortcutHelperTest.java index f7304bd0075be..3095c87ba2f67 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ShortcutHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ShortcutHelperTest.java @@ -170,6 +170,9 @@ public class ShortcutHelperTest extends UiServiceTestCase { @Test public void testGetValidShortcutInfo_notLongLived() { ShortcutInfo si = mock(ShortcutInfo.class); + when(si.getPackage()).thenReturn(PKG); + when(si.getId()).thenReturn(SHORTCUT_ID); + when(si.getUserId()).thenReturn(UserHandle.USER_SYSTEM); when(si.isLongLived()).thenReturn(false); when(si.isEnabled()).thenReturn(true); ArrayList shortcuts = new ArrayList<>(); @@ -184,6 +187,9 @@ public class ShortcutHelperTest extends UiServiceTestCase { @Test public void testGetValidShortcutInfo_notSharingShortcut() { ShortcutInfo si = mock(ShortcutInfo.class); + when(si.getPackage()).thenReturn(PKG); + when(si.getId()).thenReturn(SHORTCUT_ID); + when(si.getUserId()).thenReturn(UserHandle.USER_SYSTEM); when(si.isLongLived()).thenReturn(true); when(si.isEnabled()).thenReturn(true); ArrayList shortcuts = new ArrayList<>(); @@ -198,6 +204,9 @@ public class ShortcutHelperTest extends UiServiceTestCase { @Test public void testGetValidShortcutInfo_notEnabled() { ShortcutInfo si = mock(ShortcutInfo.class); + when(si.getPackage()).thenReturn(PKG); + when(si.getId()).thenReturn(SHORTCUT_ID); + when(si.getUserId()).thenReturn(UserHandle.USER_SYSTEM); when(si.isLongLived()).thenReturn(true); when(si.isEnabled()).thenReturn(false); ArrayList shortcuts = new ArrayList<>(); @@ -212,6 +221,9 @@ public class ShortcutHelperTest extends UiServiceTestCase { @Test public void testGetValidShortcutInfo_isValid() { ShortcutInfo si = mock(ShortcutInfo.class); + when(si.getPackage()).thenReturn(PKG); + when(si.getId()).thenReturn(SHORTCUT_ID); + when(si.getUserId()).thenReturn(UserHandle.USER_SYSTEM); when(si.isLongLived()).thenReturn(true); when(si.isEnabled()).thenReturn(true); ArrayList shortcuts = new ArrayList<>();