Don't update publisher version code without scanning manifest

- It was (theoretically) possible for shortcut manager to update
the version code for a publisher package without rescanning manifest
shortcuts, if backup happens right after unlocking a user before
SM searches for updated packages.  If it happens, then SM will not
scan the manifest for this package until it's updated next time.

So don't refresh the version code during backup, which we only
have to do for launchers but not publishers.

- Also fix the owner-user-id for launchers.  (Luckily it's not causing
any issues.)

Bug 31402152

Change-Id: I5d898eb3882b74edaca8b2d5f960849370ffc23b

6 files changed, 84 insertions, 61 deletions

diff --git a/services/core/java/com/android/server/pm/ShortcutLauncher.java b/services/core/java/com/android/server/pm/ShortcutLauncher.java
index df51923c97d3..2af1bcb425ec 100644
--- a/services/core/java/com/android/server/pm/ShortcutLauncher.java
+++ b/services/core/java/com/android/server/pm/ShortcutLauncher.java
@@ -17,6 +17,7 @@ package com.android.server.pm;
 
 import android.annotation.NonNull;
 import android.annotation.UserIdInt;
+import android.content.pm.PackageInfo;
 import android.content.pm.ShortcutInfo;
 import android.util.ArrayMap;
 import android.util.ArraySet;
@@ -151,6 +152,16 @@ class ShortcutLauncher extends ShortcutPackageItem {
         return mPinnedShortcuts.remove(PackageWithUser.of(packageUserId, packageName)) != null;
     }
 
+    public void ensureVersionInfo() {
+        final PackageInfo pi = mShortcutUser.mService.getPackageInfoWithSignatures(
+                getPackageName(), getPackageUserId());
+        if (pi == null) {
+            Slog.w(TAG, "Package not found: " + getPackageName());
+            return;
+        }
+        getPackageInfo().updateVersionInfo(pi);
+    }
+
     /**
      * Persist.
      */
@@ -202,7 +213,7 @@ class ShortcutLauncher extends ShortcutPackageItem {
                 fromBackup ? ownerUserId
                 : ShortcutService.parseIntAttribute(parser, ATTR_LAUNCHER_USER_ID, ownerUserId);
 
-        final ShortcutLauncher ret = new ShortcutLauncher(shortcutUser, launcherUserId,
+        final ShortcutLauncher ret = new ShortcutLauncher(shortcutUser, ownerUserId,
                 launcherPackageName, launcherUserId);
 
         ArraySet<String> ids = null;
diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java
index 1acc9552c0a1..d558b07a7a70 100644
--- a/services/core/java/com/android/server/pm/ShortcutPackage.java
+++ b/services/core/java/com/android/server/pm/ShortcutPackage.java
@@ -23,7 +23,6 @@ import android.content.Intent;
 import android.content.pm.PackageInfo;
 import android.content.pm.ShortcutInfo;
 import android.content.res.Resources;
-import android.os.Bundle;
 import android.os.PersistableBundle;
 import android.text.format.Formatter;
 import android.util.ArrayMap;
@@ -145,30 +144,6 @@ class ShortcutPackage extends ShortcutPackageItem {
         return mPackageUid;
     }
 
-    /**
-     * Called when a shortcut is about to be published.  At this point we know the publisher
-     * package
-     * exists (as opposed to Launcher trying to fetch shortcuts from a non-existent package), so
-     * we do some initialization for the package.
-     */
-    private void ensurePackageVersionInfo() {
-        // Make sure we have the version code for the app.  We need the version code in
-        // handlePackageUpdated().
-        if (getPackageInfo().getVersionCode() < 0) {
-            final ShortcutService s = mShortcutUser.mService;
-
-            final PackageInfo pi = s.getPackageInfo(getPackageName(), getOwnerUserId());
-            if (pi != null) {
-                if (ShortcutService.DEBUG) {
-                    Slog.d(TAG, String.format("Package %s version = %d", getPackageName(),
-                            pi.versionCode));
-                }
-                getPackageInfo().updateVersionInfo(pi);
-                s.scheduleSaveUser(getOwnerUserId());
-            }
-        }
-    }
-
     @Nullable
     public Resources getPackageResources() {
         return mShortcutUser.mService.injectGetResourcesForApplicationAsUser(
@@ -251,8 +226,6 @@ class ShortcutPackage extends ShortcutPackageItem {
         Preconditions.checkArgument(newShortcut.isEnabled(),
                 "add/setDynamicShortcuts() cannot publish disabled shortcuts");
 
-        ensurePackageVersionInfo();
-
         newShortcut.addFlags(ShortcutInfo.FLAG_DYNAMIC);
 
         final ShortcutInfo oldShortcut = mShortcuts.get(newShortcut.getId());
diff --git a/services/core/java/com/android/server/pm/ShortcutPackageInfo.java b/services/core/java/com/android/server/pm/ShortcutPackageInfo.java
index e7b66fca684b..4de15de9ff71 100644
--- a/services/core/java/com/android/server/pm/ShortcutPackageInfo.java
+++ b/services/core/java/com/android/server/pm/ShortcutPackageInfo.java
@@ -20,6 +20,7 @@ import android.annotation.UserIdInt;
 import android.content.pm.PackageInfo;
 import android.util.Slog;
 
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.server.backup.BackupUtils;
 
 import libcore.io.Base64;
@@ -89,6 +90,7 @@ class ShortcutPackageInfo {
         return mLastUpdateTime;
     }
 
+    /** Set {@link #mVersionCode} and {@link #mLastUpdateTime} from a {@link PackageInfo}. */
     public void updateVersionInfo(@NonNull PackageInfo pi) {
         if (pi != null) {
             mVersionCode = pi.versionCode;
@@ -119,7 +121,8 @@ class ShortcutPackageInfo {
         return true;
     }
 
-    public static ShortcutPackageInfo generateForInstalledPackage(
+    @VisibleForTesting
+    public static ShortcutPackageInfo generateForInstalledPackageForTest(
             ShortcutService s, String packageName, @UserIdInt int packageUserId) {
         final PackageInfo pi = s.getPackageInfoWithSignatures(packageName, packageUserId);
         if (pi.signatures == null || pi.signatures.length == 0) {
@@ -132,7 +135,7 @@ class ShortcutPackageInfo {
         return ret;
     }
 
-    public void refresh(ShortcutService s, ShortcutPackageItem pkg) {
+    public void refreshSignature(ShortcutService s, ShortcutPackageItem pkg) {
         if (mIsShadow) {
             s.wtf("Attempted to refresh package info for shadow package " + pkg.getPackageName()
                     + ", user=" + pkg.getOwnerUserId());
@@ -145,8 +148,6 @@ class ShortcutPackageInfo {
             Slog.w(TAG, "Package not found: " + pkg.getPackageName());
             return;
         }
-        mVersionCode = pi.versionCode;
-        mLastUpdateTime = pi.lastUpdateTime;
         mSigHashes = BackupUtils.hashSignatureArray(pi.signatures);
     }
 
diff --git a/services/core/java/com/android/server/pm/ShortcutPackageItem.java b/services/core/java/com/android/server/pm/ShortcutPackageItem.java
index 178005840ab1..1f195a7a544b 100644
--- a/services/core/java/com/android/server/pm/ShortcutPackageItem.java
+++ b/services/core/java/com/android/server/pm/ShortcutPackageItem.java
@@ -65,8 +65,7 @@ abstract class ShortcutPackageItem {
     /**
      * ID of the user who actually has this package running on.  For {@link ShortcutPackage},
      * this is the same thing as {@link #getOwnerUserId}, but if it's a {@link ShortcutLauncher} and
-     * {@link #getOwnerUserId} is of a work profile, then this ID could be the user who owns the
-     * profile.
+     * {@link #getOwnerUserId} is of work profile, then this ID is of the primary user.
      */
     public int getPackageUserId() {
         return mPackageUserId;
@@ -86,12 +85,12 @@ abstract class ShortcutPackageItem {
         return mPackageInfo;
     }
 
-    public void refreshPackageInfoAndSave() {
+    public void refreshPackageSignatureAndSave() {
         if (mPackageInfo.isShadow()) {
             return; // Don't refresh for shadow user.
         }
         final ShortcutService s = mShortcutUser.mService;
-        mPackageInfo.refresh(s, this);
+        mPackageInfo.refreshSignature(s, this);
         s.scheduleSaveUser(getOwnerUserId());
     }
 
diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java
index 2c61f75bd426..13f558e3dd13 100644
--- a/services/core/java/com/android/server/pm/ShortcutService.java
+++ b/services/core/java/com/android/server/pm/ShortcutService.java
@@ -1158,7 +1158,10 @@ public class ShortcutService extends IShortcutService.Stub {
         }
     }
 
-    /** Return the per-user per-package state. */
+    /**
+     * Return the per-user per-package state.  If the caller is a publisher, use
+     * {@link #getPackageShortcutsForPublisherLocked} instead.
+     */
     @GuardedBy("mLock")
     @NonNull
     ShortcutPackage getPackageShortcutsLocked(
@@ -1166,6 +1169,16 @@ public class ShortcutService extends IShortcutService.Stub {
         return getUserShortcutsLocked(userId).getPackageShortcuts(packageName);
     }
 
+    /** Return the per-user per-package state.  Use this when the caller is a publisher. */
+    @GuardedBy("mLock")
+    @NonNull
+    ShortcutPackage getPackageShortcutsForPublisherLocked(
+            @NonNull String packageName, @UserIdInt int userId) {
+        final ShortcutPackage ret = getUserShortcutsLocked(userId).getPackageShortcuts(packageName);
+        ret.getUser().onCalledByPublisher(packageName);
+        return ret;
+    }
+
     @GuardedBy("mLock")
     @NonNull
     ShortcutLauncher getLauncherShortcutsLocked(
@@ -1634,8 +1647,7 @@ public class ShortcutService extends IShortcutService.Stub {
         synchronized (mLock) {
             throwIfUserLockedL(userId);
 
-            final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
-            ps.getUser().onCalledByPublisher(packageName);
+            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId);
 
             ps.ensureImmutableShortcutsNotIncluded(newShortcuts);
 
@@ -1686,8 +1698,7 @@ public class ShortcutService extends IShortcutService.Stub {
         synchronized (mLock) {
             throwIfUserLockedL(userId);
 
-            final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
-            ps.getUser().onCalledByPublisher(packageName);
+            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId);
 
             ps.ensureImmutableShortcutsNotIncluded(newShortcuts);
 
@@ -1767,8 +1778,7 @@ public class ShortcutService extends IShortcutService.Stub {
         synchronized (mLock) {
             throwIfUserLockedL(userId);
 
-            final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
-            ps.getUser().onCalledByPublisher(packageName);
+            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId);
 
             ps.ensureImmutableShortcutsNotIncluded(newShortcuts);
 
@@ -1817,8 +1827,7 @@ public class ShortcutService extends IShortcutService.Stub {
         synchronized (mLock) {
             throwIfUserLockedL(userId);
 
-            final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
-            ps.getUser().onCalledByPublisher(packageName);
+            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId);
 
             ps.ensureImmutableShortcutsNotIncludedWithIds((List<String>) shortcutIds);
 
@@ -1847,8 +1856,7 @@ public class ShortcutService extends IShortcutService.Stub {
         synchronized (mLock) {
             throwIfUserLockedL(userId);
 
-            final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
-            ps.getUser().onCalledByPublisher(packageName);
+            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId);
 
             ps.ensureImmutableShortcutsNotIncludedWithIds((List<String>) shortcutIds);
 
@@ -1870,8 +1878,7 @@ public class ShortcutService extends IShortcutService.Stub {
         synchronized (mLock) {
             throwIfUserLockedL(userId);
 
-            final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
-            ps.getUser().onCalledByPublisher(packageName);
+            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId);
 
             ps.ensureImmutableShortcutsNotIncludedWithIds((List<String>) shortcutIds);
 
@@ -1895,8 +1902,7 @@ public class ShortcutService extends IShortcutService.Stub {
         synchronized (mLock) {
             throwIfUserLockedL(userId);
 
-            final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
-            ps.getUser().onCalledByPublisher(packageName);
+            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId);
             ps.deleteAllDynamicShortcuts();
         }
         packageShortcutsChanged(packageName, userId);
@@ -1951,8 +1957,7 @@ public class ShortcutService extends IShortcutService.Stub {
 
         final ArrayList<ShortcutInfo> ret = new ArrayList<>();
 
-        final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
-        ps.getUser().onCalledByPublisher(packageName);
+        final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId);
         ps.findAll(ret, query, cloneFlags);
 
         return new ParceledListSlice<>(ret);
@@ -1973,8 +1978,7 @@ public class ShortcutService extends IShortcutService.Stub {
         synchronized (mLock) {
             throwIfUserLockedL(userId);
 
-            final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
-            ps.getUser().onCalledByPublisher(packageName);
+            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId);
             return mMaxUpdatesPerInterval - ps.getApiCallCount();
         }
     }
@@ -2013,8 +2017,7 @@ public class ShortcutService extends IShortcutService.Stub {
         synchronized (mLock) {
             throwIfUserLockedL(userId);
 
-            final ShortcutPackage ps = getPackageShortcutsLocked(packageName, userId);
-            ps.getUser().onCalledByPublisher(packageName);
+            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId);
 
             if (ps.findShortcutById(shortcutId) == null) {
                 Log.w(TAG, String.format("reportShortcutUsed: package %s doesn't have shortcut %s",
@@ -3151,9 +3154,19 @@ public class ShortcutService extends IShortcutService.Stub {
                 return null;
             }
 
-            user.forAllPackageItems(spi -> spi.refreshPackageInfoAndSave());
+            // Update the signatures for all packages.
+            user.forAllPackageItems(spi -> spi.refreshPackageSignatureAndSave());
+
+            // Set the version code for the launchers.
+            // We shouldn't do this for publisher packages, because we don't want to update the
+            // version code without rescanning the manifest.
+            user.forAllLaunchers(launcher -> launcher.ensureVersionInfo());
+
+            // Save to the filesystem.
+            scheduleSaveUser(userId);
+            saveDirtyInfo();
 
-            // Then save.
+            // Then create the backup payload.
             final ByteArrayOutputStream os = new ByteArrayOutputStream(32 * 1024);
             try {
                 saveUserInternalLocked(userId, os, /* forBackup */ true);
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 143398f63860..3cfdc329a971 100644
--- a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java
+++ b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java
@@ -3813,9 +3813,9 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
         addPackage(CALLING_PACKAGE_1, CALLING_UID_1, 10, "sig1");
         addPackage(CALLING_PACKAGE_2, CALLING_UID_1, 10, "sig1", "sig2");
 
-        final ShortcutPackageInfo spi1 = ShortcutPackageInfo.generateForInstalledPackage(
+        final ShortcutPackageInfo spi1 = ShortcutPackageInfo.generateForInstalledPackageForTest(
                 mService, CALLING_PACKAGE_1, USER_0);
-        final ShortcutPackageInfo spi2 = ShortcutPackageInfo.generateForInstalledPackage(
+        final ShortcutPackageInfo spi2 = ShortcutPackageInfo.generateForInstalledPackageForTest(
                 mService, CALLING_PACKAGE_2, USER_0);
 
         checkCanRestoreTo(true, spi1, 10, "sig1");
@@ -5661,6 +5661,32 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest {
                                 buildAllQuery(CALLING_PACKAGE_1), HANDLE_USER_P0);
                     });
         });
+        // Check the user-IDs.
+        assertEquals(USER_0,
+                mService.getUserShortcutsLocked(USER_0).getPackageShortcuts(CALLING_PACKAGE_1)
+                        .getOwnerUserId());
+        assertEquals(USER_0,
+                mService.getUserShortcutsLocked(USER_0).getPackageShortcuts(CALLING_PACKAGE_1)
+                        .getPackageUserId());
+        assertEquals(USER_P0,
+                mService.getUserShortcutsLocked(USER_P0).getPackageShortcuts(CALLING_PACKAGE_1)
+                        .getOwnerUserId());
+        assertEquals(USER_P0,
+                mService.getUserShortcutsLocked(USER_P0).getPackageShortcuts(CALLING_PACKAGE_1)
+                        .getPackageUserId());
+
+        assertEquals(USER_0,
+                mService.getUserShortcutsLocked(USER_0).getLauncherShortcuts(LAUNCHER_1, USER_0)
+                        .getOwnerUserId());
+        assertEquals(USER_0,
+                mService.getUserShortcutsLocked(USER_0).getLauncherShortcuts(LAUNCHER_1, USER_0)
+                        .getPackageUserId());
+        assertEquals(USER_P0,
+                mService.getUserShortcutsLocked(USER_P0).getLauncherShortcuts(LAUNCHER_1, USER_0)
+                        .getOwnerUserId());
+        assertEquals(USER_0,
+                mService.getUserShortcutsLocked(USER_P0).getLauncherShortcuts(LAUNCHER_1, USER_0)
+                        .getPackageUserId());
     }
 
     public void testOnApplicationActive_permission() {