diff --git a/services/core/java/com/android/server/pm/ShortcutLauncher.java b/services/core/java/com/android/server/pm/ShortcutLauncher.java index 2960bc9a3790c..c0c2349532978 100644 --- a/services/core/java/com/android/server/pm/ShortcutLauncher.java +++ b/services/core/java/com/android/server/pm/ShortcutLauncher.java @@ -420,4 +420,14 @@ class ShortcutLauncher extends ShortcutPackageItem { ArraySet getAllPinnedShortcutsForTest(String packageName, int packageUserId) { return new ArraySet<>(mPinnedShortcuts.get(PackageWithUser.of(packageUserId, packageName))); } + + @Override + protected File getShortcutPackageItemFile() { + final File path = new File(mShortcutUser.mService.injectUserDataPath( + mShortcutUser.getUserId()), ShortcutUser.DIRECTORY_LUANCHERS); + // Package user id and owner id can have different values for ShortcutLaunchers. Adding + // user Id to the file name to create a unique path. Owner id is used in the root path. + final String fileName = getPackageName() + getPackageUserId() + ".xml"; + return new File(path, fileName); + } } diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java index f57eaaef25a46..fef6ce1f67b39 100644 --- a/services/core/java/com/android/server/pm/ShortcutPackage.java +++ b/services/core/java/com/android/server/pm/ShortcutPackage.java @@ -160,8 +160,6 @@ class ShortcutPackage extends ShortcutPackageItem { private static final String KEY_BITMAPS = "bitmaps"; private static final String KEY_BITMAP_BYTES = "bitmapBytes"; - private final Object mLock = new Object(); - private final Executor mExecutor; /** @@ -779,7 +777,7 @@ class ShortcutPackage extends ShortcutPackageItem { return false; } mApiCallCount++; - s.scheduleSaveUser(getOwnerUserId()); + scheduleSave(); return true; } @@ -789,7 +787,7 @@ class ShortcutPackage extends ShortcutPackageItem { } if (mApiCallCount > 0) { mApiCallCount = 0; - mShortcutUser.mService.scheduleSaveUser(getOwnerUserId()); + scheduleSave(); } } @@ -1890,15 +1888,12 @@ class ShortcutPackage extends ShortcutPackageItem { final ShortcutPackage ret = new ShortcutPackage(shortcutUser, shortcutUser.getUserId(), packageName); - synchronized (ret.mLock) { ret.mIsAppSearchSchemaUpToDate = ShortcutService.parseIntAttribute( parser, ATTR_SCHEMA_VERSON, 0) == AppSearchShortcutInfo.SCHEMA_VERSION; } - ret.mApiCallCount = - ShortcutService.parseIntAttribute(parser, ATTR_CALL_COUNT); - ret.mLastResetTime = - ShortcutService.parseLongAttribute(parser, ATTR_LAST_RESET); + ret.mApiCallCount = ShortcutService.parseIntAttribute(parser, ATTR_CALL_COUNT); + ret.mLastResetTime = ShortcutService.parseLongAttribute(parser, ATTR_LAST_RESET); final int outerDepth = parser.getDepth(); @@ -2440,16 +2435,15 @@ class ShortcutPackage extends ShortcutPackageItem { }))); } - void persistsAllShortcutsAsync() { - synchronized (mLock) { - final Map copy = mShortcuts; - if (!mTransientShortcuts.isEmpty()) { - copy.putAll(mTransientShortcuts); - mTransientShortcuts.clear(); - } - saveShortcutsAsync(copy.values().stream().filter(ShortcutInfo::usesQuota).collect( - Collectors.toList())); + @Override + void scheduleSaveToAppSearchLocked() { + final Map copy = new ArrayMap<>(mShortcuts); + if (!mTransientShortcuts.isEmpty()) { + copy.putAll(mTransientShortcuts); + mTransientShortcuts.clear(); } + saveShortcutsAsync(copy.values().stream().filter(ShortcutInfo::usesQuota).collect( + Collectors.toList())); } private void saveShortcutsAsync( @@ -2548,4 +2542,12 @@ class ShortcutPackage extends ShortcutPackageItem { Binder.restoreCallingIdentity(callingIdentity); } } + + @Override + protected File getShortcutPackageItemFile() { + final File path = new File(mShortcutUser.mService.injectUserDataPath( + mShortcutUser.getUserId()), ShortcutUser.DIRECTORY_PACKAGES); + final String fileName = getPackageName() + ".xml"; + return new File(path, fileName); + } } diff --git a/services/core/java/com/android/server/pm/ShortcutPackageItem.java b/services/core/java/com/android/server/pm/ShortcutPackageItem.java index 829133c9854ae..6e0436f208e33 100644 --- a/services/core/java/com/android/server/pm/ShortcutPackageItem.java +++ b/services/core/java/com/android/server/pm/ShortcutPackageItem.java @@ -23,6 +23,7 @@ import android.util.Slog; import android.util.TypedXmlSerializer; import android.util.Xml; +import com.android.internal.annotations.GuardedBy; import com.android.internal.util.Preconditions; import org.json.JSONException; @@ -36,7 +37,7 @@ import java.nio.charset.StandardCharsets; import java.util.Objects; /** - * All methods should be guarded by {@code #mShortcutUser.mService.mLock}. + * All methods should be either guarded by {@code #mShortcutUser.mService.mLock} or {@code #mLock}. */ abstract class ShortcutPackageItem { private static final String TAG = ShortcutService.TAG; @@ -49,6 +50,8 @@ abstract class ShortcutPackageItem { protected ShortcutUser mShortcutUser; + protected final Object mLock = new Object(); + protected ShortcutPackageItem(@NonNull ShortcutUser shortcutUser, int packageUserId, @NonNull String packageName, @NonNull ShortcutPackageInfo packageInfo) { @@ -98,7 +101,7 @@ abstract class ShortcutPackageItem { } final ShortcutService s = mShortcutUser.mService; mPackageInfo.refreshSignature(s, this); - s.scheduleSaveUser(getOwnerUserId()); + scheduleSave(); } public void attemptToRestoreIfNeededAndSave() { @@ -138,7 +141,7 @@ abstract class ShortcutPackageItem { // Either way, it's no longer a shadow. mPackageInfo.setShadow(false); - s.scheduleSaveUser(mPackageUserId); + scheduleSave(); } protected abstract boolean canRestoreAnyVersion(); @@ -148,7 +151,8 @@ abstract class ShortcutPackageItem { public abstract void saveToXml(@NonNull TypedXmlSerializer out, boolean forBackup) throws IOException, XmlPullParserException; - public void saveToFile(File path, boolean forBackup) { + @GuardedBy("mLock") + public void saveToFileLocked(File path, boolean forBackup) { final AtomicFile file = new AtomicFile(path); FileOutputStream os = null; try { @@ -176,6 +180,11 @@ abstract class ShortcutPackageItem { } } + @GuardedBy("mLock") + void scheduleSaveToAppSearchLocked() { + + } + public JSONObject dumpCheckin(boolean clear) throws JSONException { final JSONObject result = new JSONObject(); result.put(KEY_NAME, mPackageName); @@ -187,4 +196,36 @@ abstract class ShortcutPackageItem { */ public void verifyStates() { } + + public void scheduleSave() { + mShortcutUser.mService.injectPostToHandlerDebounced( + mSaveShortcutPackageRunner, mSaveShortcutPackageRunner); + } + + private final Runnable mSaveShortcutPackageRunner = this::saveShortcutPackageItem; + + void saveShortcutPackageItem() { + // Wait for bitmap saves to conclude before proceeding to saving shortcuts. + mShortcutUser.mService.waitForBitmapSaves(); + // Save each ShortcutPackageItem in a separate Xml file. + final File path = getShortcutPackageItemFile(); + if (ShortcutService.DEBUG || ShortcutService.DEBUG_REBOOT) { + Slog.d(TAG, "Saving package item " + getPackageName() + " to " + path); + } + synchronized (mLock) { + path.getParentFile().mkdirs(); + // TODO: Since we are persisting shortcuts into AppSearch, we should read from/write to + // AppSearch as opposed to maintaining a separate XML file. + saveToFileLocked(path, false /*forBackup*/); + scheduleSaveToAppSearchLocked(); + } + } + + void removeShortcutPackageItem() { + synchronized (mLock) { + getShortcutPackageItemFile().delete(); + } + } + + protected abstract File getShortcutPackageItemFile(); } diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java index 9627c4394db74..780f976d2a400 100644 --- a/services/core/java/com/android/server/pm/ShortcutService.java +++ b/services/core/java/com/android/server/pm/ShortcutService.java @@ -748,7 +748,7 @@ public class ShortcutService extends IShortcutService.Stub { getUserShortcutsLocked(userId).cancelAllInFlightTasks(); // Save all dirty information. - saveDirtyInfo(false); + saveDirtyInfo(); // Unload mUsers.delete(userId); @@ -1203,10 +1203,6 @@ public class ShortcutService extends IShortcutService.Stub { @VisibleForTesting void saveDirtyInfo() { - saveDirtyInfo(true); - } - - private void saveDirtyInfo(boolean saveShortcutsInAppSearch) { if (DEBUG || DEBUG_REBOOT) { Slog.d(TAG, "saveDirtyInfo"); } @@ -1221,10 +1217,6 @@ public class ShortcutService extends IShortcutService.Stub { if (userId == UserHandle.USER_NULL) { // USER_NULL for base state. saveBaseStateLocked(); } else { - if (saveShortcutsInAppSearch) { - getUserShortcutsLocked(userId).forAllPackages( - ShortcutPackage::persistsAllShortcutsAsync); - } saveUserLocked(userId); } } @@ -1816,7 +1808,7 @@ public class ShortcutService extends IShortcutService.Stub { } injectPostToHandlerDebounced(sp, notifyListenerRunnable(packageName, userId)); notifyShortcutChangeCallbacks(packageName, userId, changedShortcuts, removedShortcuts); - scheduleSaveUser(userId); + sp.scheduleSave(); } private void notifyListeners(@NonNull final String packageName, @UserIdInt final int userId) { @@ -2878,12 +2870,11 @@ public class ShortcutService extends IShortcutService.Stub { final ShortcutUser user = getUserShortcutsLocked(owningUserId); boolean doNotify = false; - // First, remove the package from the package list (if the package is a publisher). - if (packageUserId == owningUserId) { - if (user.removePackage(packageName) != null) { - doNotify = true; - } + final ShortcutPackage sp = (packageUserId == owningUserId) + ? user.removePackage(packageName) : null; + if (sp != null) { + doNotify = true; } // Also remove from the launcher list (if the package is a launcher). @@ -2906,6 +2897,10 @@ public class ShortcutService extends IShortcutService.Stub { // notifyListeners. user.rescanPackageIfNeeded(packageName, /* forceRescan=*/ true); } + if (!appStillExists && (packageUserId == owningUserId) && sp != null) { + // If the app is removed altogether, we can get rid of the xml as well + injectPostToHandler(() -> sp.removeShortcutPackageItem()); + } if (!wasUserLoaded) { // Note this will execute the scheduled save. @@ -3788,7 +3783,7 @@ public class ShortcutService extends IShortcutService.Stub { if (mHandler.hasCallbacks(mSaveDirtyInfoRunner)) { mHandler.removeCallbacks(mSaveDirtyInfoRunner); forEachLoadedUserLocked(ShortcutUser::cancelAllInFlightTasks); - saveDirtyInfo(false); + saveDirtyInfo(); } mShutdown.set(true); } @@ -4457,7 +4452,7 @@ public class ShortcutService extends IShortcutService.Stub { // Save to the filesystem. scheduleSaveUser(userId); - saveDirtyInfo(false); + saveDirtyInfo(); // Note, in case of backup, we don't have to wait on bitmap saving, because we don't // back up bitmaps anyway. @@ -5352,8 +5347,7 @@ public class ShortcutService extends IShortcutService.Stub { } } - @VisibleForTesting - void waitForBitmapSavesForTest() { + void waitForBitmapSaves() { synchronized (mLock) { mShortcutBitmapSaver.waitForAllSavesLocked(); } diff --git a/services/core/java/com/android/server/pm/ShortcutUser.java b/services/core/java/com/android/server/pm/ShortcutUser.java index 4bb5dcfa4b269..75e18b547c553 100644 --- a/services/core/java/com/android/server/pm/ShortcutUser.java +++ b/services/core/java/com/android/server/pm/ShortcutUser.java @@ -407,35 +407,10 @@ class ShortcutUser { } spi.saveToXml(out, forBackup); } else { - // Save each ShortcutPackageItem in a separate Xml file. - final File path = getShortcutPackageItemFile(spi); - if (ShortcutService.DEBUG || ShortcutService.DEBUG_REBOOT) { - Slog.d(TAG, "Saving package item " + spi.getPackageName() + " to " + path); - } - - path.getParentFile().mkdirs(); - spi.saveToFile(path, forBackup); + spi.saveShortcutPackageItem(); } } - private File getShortcutPackageItemFile(ShortcutPackageItem spi) { - boolean isShortcutLauncher = spi instanceof ShortcutLauncher; - - final File path = new File(mService.injectUserDataPath(mUserId), - isShortcutLauncher ? DIRECTORY_LUANCHERS : DIRECTORY_PACKAGES); - - final String fileName; - if (isShortcutLauncher) { - // Package user id and owner id can have different values for ShortcutLaunchers. Adding - // user Id to the file name to create a unique path. Owner id is used in the root path. - fileName = spi.getPackageName() + spi.getPackageUserId() + ".xml"; - } else { - fileName = spi.getPackageName() + ".xml"; - } - - return new File(path, fileName); - } - public static ShortcutUser loadFromXml(ShortcutService s, TypedXmlPullParser parser, int userId, boolean fromBackup) throws IOException, XmlPullParserException, InvalidFileFormatException { final ShortcutUser ret = new ShortcutUser(s, userId); 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 fdf9354747a08..40943774c0afc 100644 --- a/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java @@ -1975,7 +1975,7 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase { if (si == null) { return null; } - mService.waitForBitmapSavesForTest(); + mService.waitForBitmapSaves(); return new File(si.getBitmapPath()).getName(); } @@ -1984,7 +1984,7 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase { if (si == null) { return null; } - mService.waitForBitmapSavesForTest(); + mService.waitForBitmapSaves(); return new File(si.getBitmapPath()).getAbsolutePath(); } @@ -2139,7 +2139,7 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase { } protected boolean bitmapDirectoryExists(String packageName, int userId) { - mService.waitForBitmapSavesForTest(); + mService.waitForBitmapSaves(); final File path = new File(mService.getUserBitmapFilePath(userId), packageName); return path.isDirectory(); } 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 867890f938ba6..411b52155abb2 100644 --- a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java +++ b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest1.java @@ -1040,7 +1040,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { dumpsysOnLogcat(); - mService.waitForBitmapSavesForTest(); + mService.waitForBitmapSaves(); // Check files and directories. // Package 3 has no bitmaps, so we don't create a directory. assertBitmapDirectories(USER_0, CALLING_PACKAGE_1, CALLING_PACKAGE_2); @@ -1096,7 +1096,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { makeFile(mService.getUserBitmapFilePath(USER_10), CALLING_PACKAGE_2, "3").createNewFile(); makeFile(mService.getUserBitmapFilePath(USER_10), CALLING_PACKAGE_2, "4").createNewFile(); - mService.waitForBitmapSavesForTest(); + mService.waitForBitmapSaves(); assertBitmapDirectories(USER_0, CALLING_PACKAGE_1, CALLING_PACKAGE_2, CALLING_PACKAGE_3, "a.b.c", "d.e.f"); @@ -1111,7 +1111,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { // The below check is the same as above, except this time USER_0 use the CALLING_PACKAGE_3 // directory. - mService.waitForBitmapSavesForTest(); + mService.waitForBitmapSaves(); assertBitmapDirectories(USER_0, CALLING_PACKAGE_1, CALLING_PACKAGE_2, CALLING_PACKAGE_3); assertBitmapDirectories(USER_10, CALLING_PACKAGE_1, CALLING_PACKAGE_2); @@ -1390,7 +1390,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { .setIcon(Icon.createWithContentUri("test_uri")) .build() ))); - mService.waitForBitmapSavesForTest(); + mService.waitForBitmapSaves(); assertWith(getCallerShortcuts()) .forShortcutWithId("s1", si -> { assertTrue(si.hasIconUri()); @@ -1402,13 +1402,13 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { .setIcon(Icon.createWithResource(getTestContext(), R.drawable.black_32x32)) .build() ))); - mService.waitForBitmapSavesForTest(); + mService.waitForBitmapSaves(); assertWith(getCallerShortcuts()) .forShortcutWithId("s1", si -> { assertTrue(si.hasIconResource()); assertEquals(R.drawable.black_32x32, si.getIconResourceId()); }); - mService.waitForBitmapSavesForTest(); + mService.waitForBitmapSaves(); mInjectedCurrentTimeMillis += INTERVAL; // reset throttling @@ -1419,7 +1419,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { getTestContext().getResources(), R.drawable.black_64x64))) .build() ))); - mService.waitForBitmapSavesForTest(); + mService.waitForBitmapSaves(); assertWith(getCallerShortcuts()) .forShortcutWithId("s1", si -> { assertTrue(si.hasIconFile()); @@ -1437,7 +1437,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { getTestContext().getResources(), R.drawable.black_64x64))) .build() ))); - mService.waitForBitmapSavesForTest(); + mService.waitForBitmapSaves(); assertWith(getCallerShortcuts()) .forShortcutWithId("s1", si -> { assertTrue(si.hasIconFile()); @@ -1451,7 +1451,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { .setIcon(Icon.createWithResource(getTestContext(), R.drawable.black_32x32)) .build() ))); - mService.waitForBitmapSavesForTest(); + mService.waitForBitmapSaves(); assertWith(getCallerShortcuts()) .forShortcutWithId("s1", si -> { assertTrue(si.hasIconResource()); @@ -1463,7 +1463,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { .setIcon(Icon.createWithContentUri("test_uri")) .build() ))); - mService.waitForBitmapSavesForTest(); + mService.waitForBitmapSaves(); assertWith(getCallerShortcuts()) .forShortcutWithId("s1", si -> { assertTrue(si.hasIconUri());