From ff14f73152775f91fe211729e2cb16a5da6933ab Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Thu, 30 Jun 2016 17:07:25 -0700 Subject: [PATCH] Make sure re-published dynamic shortcuts are always enabled - Originally there was explicit code to take over the disabled flag, which was simply not necessary. - Also fix the startShortcut() tests that have temporarily been disabled. (Also remove the stale TODOs to avoid conflict with Ia18301ba) Bug 29633681 Change-Id: I58b12ad6918d7fef4b79059b0c2c7f2df6e32269 --- .../android/server/pm/ShortcutPackage.java | 10 +-- .../server/pm/BaseShortcutManagerTest.java | 11 ++- .../server/pm/ShortcutManagerTest1.java | 84 ++++++++++++++++--- .../ShortcutManagerTestUtils.java | 4 - 4 files changed, 85 insertions(+), 24 deletions(-) diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java index 30283862a7936..1a4e4e0ad80bf 100644 --- a/services/core/java/com/android/server/pm/ShortcutPackage.java +++ b/services/core/java/com/android/server/pm/ShortcutPackage.java @@ -255,14 +255,8 @@ class ShortcutPackage extends ShortcutPackageItem { oldShortcut.ensureUpdatableWith(newShortcut); wasPinned = oldShortcut.isPinned(); - if (!oldShortcut.isEnabled()) { - newShortcut.addFlags(ShortcutInfo.FLAG_DISABLED); - } } - // TODO Check max dynamic count. - // mShortcutUser.mService.enforceMaxDynamicShortcuts(newDynamicCount); - // If it was originally pinned, the new one should be pinned too. if (wasPinned) { newShortcut.addFlags(ShortcutInfo.FLAG_PINNED); @@ -1425,12 +1419,12 @@ class ShortcutPackage extends ShortcutPackageItem { // Verify each shortcut's status. for (int i = mShortcuts.size() - 1; i >= 0; i--) { final ShortcutInfo si = mShortcuts.valueAt(i); - if (!(si.isManifestShortcut() || si.isDynamic() || si.isPinned())) { + if (!(si.isDeclaredInManifest() || si.isDynamic() || si.isPinned())) { failed = true; Log.e(TAG_VERIFY, "Package " + getPackageName() + ": shortcut " + si.getId() + " is not manifest, dynamic or pinned."); } - if (si.isManifestShortcut() && si.isDynamic()) { + if (si.isDeclaredInManifest() && si.isDynamic()) { failed = true; Log.e(TAG_VERIFY, "Package " + getPackageName() + ": shortcut " + si.getId() + " is both dynamic and manifest at the same time."); 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 b6084d5a13871..8b567a6f1b949 100644 --- a/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java @@ -29,6 +29,7 @@ import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -1375,15 +1376,21 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase { assertNotNull(launchShortcutAndGetIntent_withShortcutInfo(packageName, shortcutId, userId)); } - // TODO Fix all tests using it. protected void assertShortcutNotLaunchable(@NonNull String packageName, @NonNull String shortcutId, int userId) { + reset(mServiceContext); try { mLauncherApps.startShortcut(packageName, shortcutId, null, null, UserHandle.of(userId)); } catch (SecurityException expected) { - // security exception is okay too. + // security exception is okay. + return; } + // This shouldn't have been called. + verify(mServiceContext, times(0)).startActivityAsUser( + any(Intent.class), + any(Bundle.class), + any(UserHandle.class)); } protected void assertBitmapDirectories(int userId, String... expectedDirectories) { 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 8e26c109269da..837622a79f66e 100644 --- a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java +++ b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java @@ -1607,8 +1607,6 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { /** * This is similar to the above test, except it used "disable" instead of "remove". It also * does "enable". - * - * TODO Fix the commented out tests. */ public void testDisableAndEnableShortcuts() { runWithCaller(CALLING_PACKAGE_1, USER_0, () -> { @@ -1682,17 +1680,14 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { mLauncherApps.getShortcuts(buildQuery(/* time =*/ 0, CALLING_PACKAGE_1, /* activity =*/ null, ShortcutQuery.FLAG_GET_PINNED), getCallingUser())))), "s2"); -// assertFalse(mLauncherApps.startShortcut( -// CALLING_PACKAGE_1, "s2", null, null, HANDLE_USER_0)); + assertShortcutNotLaunchable(CALLING_PACKAGE_1, "s2", USER_0); assertShortcutIds(assertAllPinned(assertAllNotKeyFieldsOnly( mLauncherApps.getShortcuts(buildQuery(/* time =*/ 0, CALLING_PACKAGE_2, /* activity =*/ null, ShortcutQuery.FLAG_GET_PINNED), getCallingUser()))), "s3", "s4"); -// assertFalse(mLauncherApps.startShortcut( -// CALLING_PACKAGE_2, "s3", null, null, HANDLE_USER_0)); -// assertTrue(mLauncherApps.startShortcut( -// CALLING_PACKAGE_2, "s4", null, null, HANDLE_USER_0)); + assertShortcutNotLaunchable(CALLING_PACKAGE_2, "s3", USER_0); + assertShortcutLaunchable(CALLING_PACKAGE_2, "s4", USER_0); assertShortcutIds(assertAllPinned(assertAllNotKeyFieldsOnly(assertAllEnabled( mLauncherApps.getShortcuts(buildQuery(/* time =*/ 0, CALLING_PACKAGE_3, @@ -1712,8 +1707,77 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { mLauncherApps.getShortcuts(buildQuery(/* time =*/ 0, CALLING_PACKAGE_1, /* activity =*/ null, ShortcutQuery.FLAG_GET_PINNED), getCallingUser())))), "s2"); -// assertTrue(mLauncherApps.startShortcut( -// CALLING_PACKAGE_1, "s2", null, null, HANDLE_USER_0)); + assertShortcutLaunchable(CALLING_PACKAGE_1, "s2", USER_0); + }); + } + + public void testDisableShortcuts_thenRepublish() { + runWithCaller(CALLING_PACKAGE_1, USER_0, () -> { + assertTrue(mManager.setDynamicShortcuts(list( + makeShortcut("s1"), makeShortcut("s2"), makeShortcut("s3")))); + + runWithCaller(LAUNCHER_1, USER_0, () -> { + mLauncherApps.pinShortcuts( + CALLING_PACKAGE_1, list("s1", "s2", "s3"), HANDLE_USER_0); + }); + + mManager.disableShortcuts(list("s1", "s2", "s3")); + + assertWith(getCallerShortcuts()) + .haveIds("s1", "s2", "s3") + .areAllNotDynamic() + .areAllPinned() + .areAllDisabled(); + + // Make sure updateShortcuts() will not re-enable them. + assertTrue(mManager.updateShortcuts(list( + makeShortcut("s1"), makeShortcut("s2"), makeShortcut("s3")))); + + assertWith(getCallerShortcuts()) + .haveIds("s1", "s2", "s3") + .areAllNotDynamic() + .areAllPinned() + .areAllDisabled(); + + // Re-publish s1 with setDynamicShortcuts. + mInjectedCurrentTimeMillis += INTERVAL; // reset throttling + + assertTrue(mManager.setDynamicShortcuts(list( + makeShortcut("s1")))); + + assertWith(getCallerShortcuts()) + .haveIds("s1", "s2", "s3") + + .selectByIds("s1") + .areAllDynamic() + .areAllPinned() + .areAllEnabled() + + .revertToOriginalList() + .selectByIds("s2", "s3") + .areAllNotDynamic() + .areAllPinned() + .areAllDisabled(); + + // Re-publish s2 with addDynamicShortcuts. + mInjectedCurrentTimeMillis += INTERVAL; // reset throttling + + assertTrue(mManager.addDynamicShortcuts(list( + makeShortcut("s2")))); + + assertWith(getCallerShortcuts()) + .haveIds("s1", "s2", "s3") + + .selectByIds("s1", "s2") + .areAllDynamic() + .areAllPinned() + .areAllEnabled() + + .revertToOriginalList() + .selectByIds("s3") + .areAllNotDynamic() + .areAllPinned() + .areAllDisabled(); }); } 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 7d7285a24a044..f89c4e4044337 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 @@ -18,13 +18,11 @@ package com.android.server.pm.shortcutmanagertest; import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertNotNull; -import static junit.framework.Assert.assertNotSame; import static junit.framework.Assert.assertNull; import static junit.framework.Assert.assertTrue; import static junit.framework.Assert.fail; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyList; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; @@ -50,11 +48,9 @@ import android.os.ParcelFileDescriptor; import android.os.PersistableBundle; import android.os.UserHandle; import android.test.MoreAsserts; -import android.util.ArraySet; import android.util.Log; import junit.framework.Assert; -import junit.framework.AssertionFailedError; import org.hamcrest.BaseMatcher; import org.hamcrest.Description;