From fc4cf2da34335fd7e84c020f51eea1a341d3f134 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Wed, 24 Aug 2016 11:10:26 -0700 Subject: [PATCH] Shortcut: Improve backup & restore * Catch RuntimeException from restore, in case restoring from a partner device with an incompatible file format. * When a restore target app is already installed, and - if it has allowBackup=true, we'll restore normally, so all existing shortcuts will be replaced. (but manifest shortcuts will be re-published anyway.) We log a warning on logcat. - if it has allowBackup=false, we don't touch any of the existing shortcuts. Bug 31057974 Bug 30766177 Change-Id: Ic3f7e860e7ea0d086fc589d8cbed8c4cebdd4bc6 --- .../com/android/server/pm/ShortcutPackage.java | 11 ++ .../com/android/server/pm/ShortcutPackageItem.java | 9 +- .../com/android/server/pm/ShortcutService.java | 20 ++- .../java/com/android/server/pm/ShortcutUser.java | 161 ++++++++++++++------- .../android/server/pm/ShortcutManagerTest1.java | 30 ++++ 5 files changed, 174 insertions(+), 57 deletions(-) diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java index 6f6fd7c88dcd..1acc9552c0a1 100644 --- a/services/core/java/com/android/server/pm/ShortcutPackage.java +++ b/services/core/java/com/android/server/pm/ShortcutPackage.java @@ -1151,6 +1151,17 @@ class ShortcutPackage extends ShortcutPackageItem { } } + /** @return true if there's any shortcuts that are not manifest shortcuts. */ + public boolean hasNonManifestShortcuts() { + for (int i = mShortcuts.size() - 1; i >= 0; i--) { + final ShortcutInfo si = mShortcuts.valueAt(i); + if (!si.isDeclaredInManifest()) { + return true; + } + } + return false; + } + public void dump(@NonNull PrintWriter pw, @NonNull String prefix) { pw.println(); diff --git a/services/core/java/com/android/server/pm/ShortcutPackageItem.java b/services/core/java/com/android/server/pm/ShortcutPackageItem.java index 79b5c4eae1dc..178005840ab1 100644 --- a/services/core/java/com/android/server/pm/ShortcutPackageItem.java +++ b/services/core/java/com/android/server/pm/ShortcutPackageItem.java @@ -40,7 +40,7 @@ abstract class ShortcutPackageItem { private final ShortcutPackageInfo mPackageInfo; - protected final ShortcutUser mShortcutUser; + protected ShortcutUser mShortcutUser; protected ShortcutPackageItem(@NonNull ShortcutUser shortcutUser, int packageUserId, @NonNull String packageName, @@ -51,6 +51,13 @@ abstract class ShortcutPackageItem { mPackageInfo = Preconditions.checkNotNull(packageInfo); } + /** + * Change the parent {@link ShortcutUser}. Need it in the restore code. + */ + public void replaceUser(ShortcutUser user) { + mShortcutUser = user; + } + public ShortcutUser getUser() { return mShortcutUser; } diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java index adf19dc8295e..2c61f75bd426 100644 --- a/services/core/java/com/android/server/pm/ShortcutService.java +++ b/services/core/java/com/android/server/pm/ShortcutService.java @@ -380,6 +380,12 @@ public class ShortcutService extends IShortcutService.Stub { @GuardedBy("mLock") private Exception mLastWtfStacktrace; + static class InvalidFileFormatException extends Exception { + public InvalidFileFormatException(String message, Throwable cause) { + super(message, cause); + } + } + public ShortcutService(Context context) { this(context, BackgroundThread.get().getLooper(), /*onyForPackgeManagerApis*/ false); } @@ -961,7 +967,7 @@ public class ShortcutService extends IShortcutService.Stub { try { final ShortcutUser ret = loadUserInternal(userId, in, /* forBackup= */ false); return ret; - } catch (IOException | XmlPullParserException e) { + } catch (IOException | XmlPullParserException | InvalidFileFormatException e) { Slog.e(TAG, "Failed to read file " + file.getBaseFile(), e); return null; } finally { @@ -970,7 +976,8 @@ public class ShortcutService extends IShortcutService.Stub { } private ShortcutUser loadUserInternal(@UserIdInt int userId, InputStream is, - boolean fromBackup) throws XmlPullParserException, IOException { + boolean fromBackup) throws XmlPullParserException, IOException, + InvalidFileFormatException { final BufferedInputStream bis = new BufferedInputStream(is); @@ -3170,15 +3177,16 @@ public class ShortcutService extends IShortcutService.Stub { wtf("Can't restore: user " + userId + " is locked or not running"); return; } - final ShortcutUser user; + // Actually do restore. + final ShortcutUser restored; final ByteArrayInputStream is = new ByteArrayInputStream(payload); try { - user = loadUserInternal(userId, is, /* fromBackup */ true); - } catch (XmlPullParserException | IOException e) { + restored = loadUserInternal(userId, is, /* fromBackup */ true); + } catch (XmlPullParserException | IOException | InvalidFileFormatException e) { Slog.w(TAG, "Restoration failed.", e); return; } - mUsers.put(userId, user); + getUserShortcutsLocked(userId).mergeRestoredFile(restored); // Rescan all packages to re-publish manifest shortcuts and do other checks. rescanUpdatedPackagesLocked(userId, diff --git a/services/core/java/com/android/server/pm/ShortcutUser.java b/services/core/java/com/android/server/pm/ShortcutUser.java index c05c66fedbf8..5d4bfa4dcb3e 100644 --- a/services/core/java/com/android/server/pm/ShortcutUser.java +++ b/services/core/java/com/android/server/pm/ShortcutUser.java @@ -23,12 +23,14 @@ import android.content.pm.ShortcutManager; import android.text.TextUtils; import android.text.format.Formatter; import android.util.ArrayMap; +import android.util.Log; import android.util.Slog; import android.util.SparseArray; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.Preconditions; +import com.android.server.pm.ShortcutService.InvalidFileFormatException; import libcore.util.Objects; @@ -164,6 +166,11 @@ class ShortcutUser { return mPackages.containsKey(packageName); } + private void addPackage(@NonNull ShortcutPackage p) { + p.replaceUser(this); + mPackages.put(p.getPackageName(), p); + } + public ShortcutPackage removePackage(@NonNull String packageName) { final ShortcutPackage removed = mPackages.remove(packageName); @@ -179,7 +186,8 @@ class ShortcutUser { return mLaunchers; } - public void addLauncher(ShortcutLauncher launcher) { + private void addLauncher(ShortcutLauncher launcher) { + launcher.replaceUser(this); mLaunchers.put(PackageWithUser.of(launcher.getPackageUserId(), launcher.getPackageName()), launcher); } @@ -326,13 +334,16 @@ class ShortcutUser { throws IOException, XmlPullParserException { out.startTag(null, TAG_ROOT); - ShortcutService.writeAttr(out, ATTR_KNOWN_LOCALES, mKnownLocales); - ShortcutService.writeAttr(out, ATTR_LAST_APP_SCAN_TIME, - mLastAppScanTime); - ShortcutService.writeAttr(out, ATTR_LAST_APP_SCAN_OS_FINGERPRINT, - mLastAppScanOsFingerprint); + if (!forBackup) { + // Don't have to back them up. + ShortcutService.writeAttr(out, ATTR_KNOWN_LOCALES, mKnownLocales); + ShortcutService.writeAttr(out, ATTR_LAST_APP_SCAN_TIME, + mLastAppScanTime); + ShortcutService.writeAttr(out, ATTR_LAST_APP_SCAN_OS_FINGERPRINT, + mLastAppScanOsFingerprint); - ShortcutService.writeTagValue(out, TAG_LAUNCHER, mLastKnownLauncher); + ShortcutService.writeTagValue(out, TAG_LAUNCHER, mLastKnownLauncher); + } // Can't use forEachPackageItem due to the checked exceptions. { @@ -365,54 +376,59 @@ class ShortcutUser { } public static ShortcutUser loadFromXml(ShortcutService s, XmlPullParser parser, int userId, - boolean fromBackup) throws IOException, XmlPullParserException { + boolean fromBackup) throws IOException, XmlPullParserException, InvalidFileFormatException { final ShortcutUser ret = new ShortcutUser(s, userId); - ret.mKnownLocales = ShortcutService.parseStringAttribute(parser, - ATTR_KNOWN_LOCALES); - - // If lastAppScanTime is in the future, that means the clock went backwards. - // Just scan all apps again. - final long lastAppScanTime = ShortcutService.parseLongAttribute(parser, - ATTR_LAST_APP_SCAN_TIME); - final long currentTime = s.injectCurrentTimeMillis(); - ret.mLastAppScanTime = lastAppScanTime < currentTime ? lastAppScanTime : 0; - ret.mLastAppScanOsFingerprint = ShortcutService.parseStringAttribute(parser, - ATTR_LAST_APP_SCAN_OS_FINGERPRINT); - final int outerDepth = parser.getDepth(); - int type; - while ((type = parser.next()) != XmlPullParser.END_DOCUMENT - && (type != XmlPullParser.END_TAG || parser.getDepth() > outerDepth)) { - if (type != XmlPullParser.START_TAG) { - continue; - } - final int depth = parser.getDepth(); - final String tag = parser.getName(); - - if (depth == outerDepth + 1) { - switch (tag) { - case TAG_LAUNCHER: { - ret.mLastKnownLauncher = ShortcutService.parseComponentNameAttribute( - parser, ATTR_VALUE); - continue; - } - case ShortcutPackage.TAG_ROOT: { - final ShortcutPackage shortcuts = ShortcutPackage.loadFromXml( - s, ret, parser, fromBackup); - - // Don't use addShortcut(), we don't need to save the icon. - ret.mPackages.put(shortcuts.getPackageName(), shortcuts); - continue; - } - - case ShortcutLauncher.TAG_ROOT: { - ret.addLauncher( - ShortcutLauncher.loadFromXml(parser, ret, userId, fromBackup)); - continue; + try { + ret.mKnownLocales = ShortcutService.parseStringAttribute(parser, + ATTR_KNOWN_LOCALES); + + // If lastAppScanTime is in the future, that means the clock went backwards. + // Just scan all apps again. + final long lastAppScanTime = ShortcutService.parseLongAttribute(parser, + ATTR_LAST_APP_SCAN_TIME); + final long currentTime = s.injectCurrentTimeMillis(); + ret.mLastAppScanTime = lastAppScanTime < currentTime ? lastAppScanTime : 0; + ret.mLastAppScanOsFingerprint = ShortcutService.parseStringAttribute(parser, + ATTR_LAST_APP_SCAN_OS_FINGERPRINT); + final int outerDepth = parser.getDepth(); + int type; + while ((type = parser.next()) != XmlPullParser.END_DOCUMENT + && (type != XmlPullParser.END_TAG || parser.getDepth() > outerDepth)) { + if (type != XmlPullParser.START_TAG) { + continue; + } + final int depth = parser.getDepth(); + final String tag = parser.getName(); + + if (depth == outerDepth + 1) { + switch (tag) { + case TAG_LAUNCHER: { + ret.mLastKnownLauncher = ShortcutService.parseComponentNameAttribute( + parser, ATTR_VALUE); + continue; + } + case ShortcutPackage.TAG_ROOT: { + final ShortcutPackage shortcuts = ShortcutPackage.loadFromXml( + s, ret, parser, fromBackup); + + // Don't use addShortcut(), we don't need to save the icon. + ret.mPackages.put(shortcuts.getPackageName(), shortcuts); + continue; + } + + case ShortcutLauncher.TAG_ROOT: { + ret.addLauncher( + ShortcutLauncher.loadFromXml(parser, ret, userId, fromBackup)); + continue; + } } } + ShortcutService.warnForInvalidTag(depth, tag); } - ShortcutService.warnForInvalidTag(depth, tag); + } catch (RuntimeException e) { + throw new ShortcutService.InvalidFileFormatException( + "Unable to parse file", e); } return ret; } @@ -461,6 +477,51 @@ class ShortcutUser { } } + public void mergeRestoredFile(ShortcutUser restored) { + final ShortcutService s = mService; + // Note, a restore happens only at the end of setup wizard. At this point, no apps are + // installed from Play Store yet, but it's still possible that system apps have already + // published dynamic shortcuts, since some apps do so on BOOT_COMPLETED. + // When such a system app has allowbackup=true, then we go ahead and replace all existing + // shortcuts with the restored shortcuts. (Then we'll re-publish manifest shortcuts later + // in the call site.) + // When such a system app has allowbackup=false, then we'll keep the shortcuts that have + // already been published. So we selectively add restored ShortcutPackages here. + // + // The same logic applies to launchers, but since launchers shouldn't pin shortcuts + // without users interaction it's really not a big deal, so we just clear existing + // ShortcutLauncher instances in mLaunchers and add all the restored ones here. + + mLaunchers.clear(); + restored.forAllLaunchers(sl -> { + // If the app is already installed and allowbackup = false, then ignore the restored + // data. + if (s.isPackageInstalled(sl.getPackageName(), getUserId()) + && !s.shouldBackupApp(sl.getPackageName(), getUserId())) { + return; + } + addLauncher(sl); + }); + restored.forAllPackages(sp -> { + // If the app is already installed and allowbackup = false, then ignore the restored + // data. + if (s.isPackageInstalled(sp.getPackageName(), getUserId()) + && !s.shouldBackupApp(sp.getPackageName(), getUserId())) { + return; + } + + final ShortcutPackage previous = getPackageShortcutsIfExists(sp.getPackageName()); + if (previous != null && previous.hasNonManifestShortcuts()) { + Log.w(TAG, "Shortcuts for package " + sp.getPackageName() + " are being restored." + + " Existing non-manifeset shortcuts will be overwritten."); + } + addPackage(sp); + }); + // Empty the launchers and packages in restored to avoid accidentally using them. + restored.mLaunchers.clear(); + restored.mPackages.clear(); + } + public void dump(@NonNull PrintWriter pw, @NonNull String prefix) { pw.print(prefix); pw.print("User: "); 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 46ac7ce88b90..143398f63860 100644 --- a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java +++ b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java @@ -5359,6 +5359,12 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { /** * It's the case with preintalled apps -- when applyRestore() is called, the system * apps are already installed, so manifest shortcuts need to be re-published. + * + * Also, when a restore target app is already installed, and + * - if it has allowBackup=true, we'll restore normally, so all existing shortcuts will be + * replaced. (but manifest shortcuts will be re-published anyway.) We log a warning on + * logcat. + * - if it has allowBackup=false, we don't touch any of the existing shortcuts. */ public void testBackupAndRestore_appAlreadyInstalledWhenRestored() { // Pre-backup. Same as testBackupAndRestore_manifestRePublished(). @@ -5390,6 +5396,19 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { mService.mPackageMonitor.onReceive(mServiceContext, genPackageAddIntent(CALLING_PACKAGE_1, USER_0)); + // Set up shortcuts for package 3, which won't be backed up / restored. + addManifestShortcutResource( + new ComponentName(CALLING_PACKAGE_3, ShortcutActivity.class.getName()), + R.xml.shortcut_1); + updatePackageVersion(CALLING_PACKAGE_3, 1); + mService.mPackageMonitor.onReceive(mServiceContext, + genPackageAddIntent(CALLING_PACKAGE_3, USER_0)); + + runWithCaller(CALLING_PACKAGE_3, USER_0, () -> { + assertTrue(getManager().setDynamicShortcuts(list( + makeShortcut("s1")))); + }); + // Make sure the manifest shortcuts have been published. runWithCaller(CALLING_PACKAGE_1, USER_0, () -> { assertWith(getCallerShortcuts()) @@ -5415,6 +5434,11 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { .areAllDisabled(); }); + runWithCaller(CALLING_PACKAGE_3, USER_0, () -> { + assertWith(getCallerShortcuts()) + .haveIds("s1", "ms1"); + }); + // Backup and *without restarting the service, just call applyRestore()*. { int prevUid = mInjectedCallingUid; @@ -5454,6 +5478,12 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { .areAllNotDynamic() ; }); + + // Package 3 still has the same shortcuts. + runWithCaller(CALLING_PACKAGE_3, USER_0, () -> { + assertWith(getCallerShortcuts()) + .haveIds("s1", "ms1"); + }); } public void testSaveAndLoad_crossProfile() { -- 2.11.0