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

diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java
index 3028386..1a4e4e0 100644 (file)
--- 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 b6084d5..8b567a6 100644 (file)
--- 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 8e26c10..837622a 100644 (file)
--- 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 7d7285a..f89c4e4 100644 (file)
--- 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;