Merge "ShortcutManager: Floating shortcuts shouldn't have target activities."
diff --git a/core/java/android/content/pm/ShortcutInfo.java b/core/java/android/content/pm/ShortcutInfo.java
index a854b89..56f914e 100644
--- a/core/java/android/content/pm/ShortcutInfo.java
+++ b/core/java/android/content/pm/ShortcutInfo.java
@@ -314,9 +314,11 @@
*
* @hide
*/
- public void enforceMandatoryFields() {
+ public void enforceMandatoryFields(boolean forPinned) {
Preconditions.checkStringNotEmpty(mId, "Shortcut ID must be provided");
- Preconditions.checkNotNull(mActivity, "Activity must be provided");
+ if (!forPinned) {
+ Preconditions.checkNotNull(mActivity, "Activity must be provided");
+ }
if (mTitle == null && mTitleResId == 0) {
throw new IllegalArgumentException("Short label must be provided");
}
@@ -1055,6 +1057,11 @@
* Launcher apps should show the launcher icon for the returned activity alongside
* this shortcut.
*
+ * <p>When a shortcut is dynamic or manifest
+ * (i.e. either {@link #isDynamic()} or {@link #isDeclaredInManifest()} returns {@code TRUE}),
+ * then it should always have a non-null target activity.
+ * Otherwise it will return null.
+ *
* @see Builder#setActivity
*/
@Nullable
@@ -1361,7 +1368,7 @@
}
/**
- * @return true if pinned but neither static nor dynamic.
+ * Return {@code TRUE} if a shortcut is pinned but neither manifest nor dynamic.
* @hide
*/
public boolean isFloating() {
diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java
index d558b07..2eb0778 100644
--- a/services/core/java/com/android/server/pm/ShortcutPackage.java
+++ b/services/core/java/com/android/server/pm/ShortcutPackage.java
@@ -259,6 +259,11 @@
for (int i = mShortcuts.size() - 1; i >= 0; i--) {
final ShortcutInfo si = mShortcuts.valueAt(i);
+ if (si.isFloating()) {
+ si.setRank(0);
+ si.setActivity(null);
+ }
+
if (si.isAlive()) continue;
if (removeList == null) {
@@ -288,6 +293,7 @@
si.setTimestamp(now);
si.clearFlags(ShortcutInfo.FLAG_DYNAMIC);
si.setRank(0); // It may still be pinned, so clear the rank.
+ si.setActivity(null);
}
}
if (changed) {
@@ -355,6 +361,7 @@
if (oldShortcut.isPinned()) {
oldShortcut.setRank(0);
+ oldShortcut.setActivity(null);
oldShortcut.clearFlags(ShortcutInfo.FLAG_DYNAMIC | ShortcutInfo.FLAG_MANIFEST);
if (disable) {
oldShortcut.addFlags(ShortcutInfo.FLAG_DISABLED);
@@ -595,6 +602,10 @@
for (int i = mShortcuts.size() - 1; i >= 0; i--) {
final ShortcutInfo si = mShortcuts.valueAt(i);
+
+ if (si.isFloating()) {
+ continue; // Ignore floating shortcuts, which are not tied to any activities.
+ }
final ComponentName activity = si.getActivity();
if (checked.contains(activity)) {
@@ -1356,6 +1367,10 @@
case TAG_SHORTCUT:
final ShortcutInfo si = parseShortcut(parser, packageName,
shortcutUser.getUserId());
+ // Floating shortcut used to have target activities, but not anymore.
+ if (si.isFloating()) { // Not really needed by just in case.
+ si.setActivity(null);
+ }
// Don't use addShortcut(), we don't need to save the icon.
ret.mShortcuts.put(si.getId(), si);
@@ -1462,7 +1477,6 @@
intents.clear();
intents.add(intentLegacy);
}
-
return new ShortcutInfo(
userId, id, packageName, activityComponent, /* icon =*/ null,
title, titleResId, titleResName, text, textResId, textResName,
@@ -1553,12 +1567,17 @@
Log.e(TAG_VERIFY, "Package " + getPackageName() + ": shortcut " + si.getId()
+ " is both dynamic and manifest at the same time.");
}
- if (si.getActivity() == null) {
+ if (!si.isFloating() && si.getActivity() == null) {
failed = true;
Log.e(TAG_VERIFY, "Package " + getPackageName() + ": shortcut " + si.getId()
- + " has null activity.");
+ + " is not floating, but has null activity.");
}
- if ((si.isDynamic() || si.isManifestShortcut()) && !si.isEnabled()) {
+ if (si.isFloating() && si.getActivity() != null) {
+ failed = true;
+ Log.e(TAG_VERIFY, "Package " + getPackageName() + ": shortcut " + si.getId()
+ + " is floating, but has non-null activity.");
+ }
+ if (!si.isFloating() && !si.isEnabled()) {
failed = true;
Log.e(TAG_VERIFY, "Package " + getPackageName() + ": shortcut " + si.getId()
+ " is not floating, but is disabled.");
@@ -1581,7 +1600,7 @@
}
if (failed) {
- throw new IllegalStateException("See logcat for errors");
+ mShortcutUser.mService.verifyError();
}
}
diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java
index 16f209b..c5c1c0c 100644
--- a/services/core/java/com/android/server/pm/ShortcutService.java
+++ b/services/core/java/com/android/server/pm/ShortcutService.java
@@ -124,6 +124,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import java.util.function.Predicate;
@@ -401,6 +402,9 @@
@VisibleForTesting
ShortcutService(Context context, Looper looper, boolean onlyForPackageManagerApis) {
+ if (DEBUG) {
+ Binder.LOG_RUNTIME_EXCEPTION = true;
+ }
mContext = Preconditions.checkNotNull(context);
LocalServices.addService(ShortcutServiceInternal.class, new LocalService());
mHandler = new Handler(looper);
@@ -1604,7 +1608,7 @@
}
if (!forUpdate) {
- shortcut.enforceMandatoryFields();
+ shortcut.enforceMandatoryFields(/* forPinned= */ false);
Preconditions.checkArgument(
injectIsMainActivity(shortcut.getActivity(), shortcut.getUserId()),
"Cannot publish shortcut: " + shortcut.getActivity() + " is not main activity");
@@ -1752,6 +1756,9 @@
// Note copyNonNullFieldsFrom() does the "updatable with?" check too.
target.copyNonNullFieldsFrom(source);
+ if (target.isFloating()) {
+ target.setActivity(null);
+ }
target.setTimestamp(injectCurrentTimeMillis());
if (replacingIcon) {
@@ -2320,8 +2327,7 @@
return false;
}
if (componentName != null) {
- if (si.getActivity() != null
- && !si.getActivity().equals(componentName)) {
+ if (!Objects.equals(componentName, si.getActivity())) {
return false;
}
}
@@ -3771,4 +3777,8 @@
forEachLoadedUserLocked(u -> u.forAllPackageItems(ShortcutPackageItem::verifyStates));
}
}
+
+ void verifyError() {
+ Slog.e(TAG, "See logcat for errors");
+ }
}
diff --git a/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java b/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java
index 9f01773..99af9e8 100644
--- a/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java
@@ -376,6 +376,7 @@
@Override
boolean injectIsActivityEnabledAndExported(ComponentName activity, @UserIdInt int userId) {
+ assertNotNull(activity);
return mEnabledActivityChecker.test(activity, userId);
}
@@ -416,6 +417,11 @@
// During tests, WTF is fatal.
fail(message + " exception: " + th + "\n" + Log.getStackTraceString(th));
}
+
+ @Override
+ void verifyError() {
+ fail("Verify error");
+ }
}
/** ShortcutManager with injection override methods. */
diff --git a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java
index 74c1ca5..97bcaf0 100644
--- a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java
+++ b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java
@@ -1292,7 +1292,43 @@
/* activity =*/ null, /* flags */ 0), getCallingUser());
});
- // TODO More tests: pinned but dynamic.
+ // Make sure floating shortcuts don't match with an activity.
+ // At this point, s1 is dynamic and pinned, so it still has a target activity.
+ runWithCaller(LAUNCHER_1, USER_0, () -> {
+ assertWith(mLauncherApps.getShortcuts(buildQuery(
+ /* time =*/ 0,
+ CALLING_PACKAGE_2,
+ /* activity =*/ new ComponentName(CALLING_PACKAGE_2,
+ ShortcutActivity2.class.getName()),
+ ShortcutQuery.FLAG_GET_PINNED),
+ getCallingUser()))
+ .haveIds("s3")
+ .areAllPinned()
+ .areAllDynamic()
+ .areAllWithActivity(new ComponentName(CALLING_PACKAGE_2,
+ ShortcutActivity2.class.getName()));
+ });
+
+ // Now remove as a dynamic, making it floating.
+ runWithCaller(CALLING_PACKAGE_2, USER_0, () -> {
+ mManager.removeDynamicShortcuts(list("s3"));
+ assertWith(mManager.getPinnedShortcuts())
+ .selectFloating()
+ .areAllWithNoActivity();
+ });
+
+ runWithCaller(LAUNCHER_1, USER_0, () -> {
+ // This shouldn't match now.
+ assertWith(mLauncherApps.getShortcuts(buildQuery(
+ /* time =*/ 0,
+ CALLING_PACKAGE_2,
+ /* activity =*/ new ComponentName(CALLING_PACKAGE_2,
+ ShortcutActivity2.class.getName()),
+ ShortcutQuery.FLAG_GET_PINNED),
+ getCallingUser()))
+ .isEmpty();
+ });
+
}
public void testGetShortcuts_shortcutKinds() throws Exception {
diff --git a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java
index d25923c..7486858 100644
--- a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java
+++ b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java
@@ -1052,7 +1052,7 @@
assertEquals(CALLING_PACKAGE_1, si.getPackage());
assertEquals("id", si.getId());
- assertEquals(ShortcutActivity2.class.getName(), si.getActivity().getClassName());
+ assertNull(si.getActivity()); // It's now floating, so no target activity.
assertEquals(null, si.getIcon());
assertEquals("title", si.getTitle());
assertEquals("text", si.getText());
@@ -1116,7 +1116,7 @@
assertEquals(CALLING_PACKAGE_1, si.getPackage());
assertEquals("id", si.getId());
- assertEquals(ShortcutActivity2.class.getName(), si.getActivity().getClassName());
+ assertNull(si.getActivity()); // It's now floating, so no target activity.
assertEquals(null, si.getIcon());
assertEquals(10, si.getTitleResId());
assertEquals("r10", si.getTitleResName());
diff --git a/services/tests/shortcutmanagerutils/src/com/android/server/pm/shortcutmanagertest/ShortcutManagerTestUtils.java b/services/tests/shortcutmanagerutils/src/com/android/server/pm/shortcutmanagertest/ShortcutManagerTestUtils.java
index 1fe5cb7..6e74deb 100644
--- a/services/tests/shortcutmanagerutils/src/com/android/server/pm/shortcutmanagertest/ShortcutManagerTestUtils.java
+++ b/services/tests/shortcutmanagerutils/src/com/android/server/pm/shortcutmanagertest/ShortcutManagerTestUtils.java
@@ -764,6 +764,12 @@
filter(mList, ShortcutInfo::isPinned));
}
+ public ShortcutListAsserter selectFloating() {
+ return new ShortcutListAsserter(this,
+ filter(mList, (si -> si.isPinned()
+ && !(si.isDynamic() || si.isDeclaredInManifest()))));
+ }
+
public ShortcutListAsserter selectByActivity(ComponentName activity) {
return new ShortcutListAsserter(this,
ShortcutManagerTestUtils.filterByActivity(mList, activity));
@@ -895,6 +901,11 @@
return this;
}
+ public ShortcutListAsserter areAllWithNoActivity() {
+ forAllShortcuts(s -> assertNull("id=" + s.getId(), s.getActivity()));
+ return this;
+ }
+
public ShortcutListAsserter forAllShortcuts(Consumer<ShortcutInfo> sa) {
boolean found = false;
for (int i = 0; i < mList.size(); i++) {