diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java index fef6ce1f67b39..4df54b74bb1d2 100644 --- a/services/core/java/com/android/server/pm/ShortcutPackage.java +++ b/services/core/java/com/android/server/pm/ShortcutPackage.java @@ -349,7 +349,7 @@ class ShortcutPackage extends ShortcutPackageItem { private ShortcutInfo forceDeleteShortcutInner(@NonNull String id) { final ShortcutInfo shortcut = mShortcuts.remove(id); if (shortcut != null) { - mShortcutUser.mService.removeIconLocked(shortcut); + removeIcon(shortcut); shortcut.clearFlags(ShortcutInfo.FLAG_DYNAMIC | ShortcutInfo.FLAG_PINNED | ShortcutInfo.FLAG_MANIFEST | ShortcutInfo.FLAG_CACHED_ALL); } @@ -366,7 +366,7 @@ class ShortcutPackage extends ShortcutPackageItem { forceDeleteShortcutInner(newShortcut.getId()); // Extract Icon and update the icon res ID and the bitmap path. - s.saveIconAndFixUpShortcutLocked(newShortcut); + s.saveIconAndFixUpShortcutLocked(this, newShortcut); s.fixUpShortcutResourceNamesAndValues(newShortcut); saveShortcut(newShortcut); } @@ -972,7 +972,8 @@ class ShortcutPackage extends ShortcutPackageItem { /** * Return the filenames (excluding path names) of icon bitmap files from this package. */ - public ArraySet getUsedBitmapFiles() { + @GuardedBy("mLock") + private ArraySet getUsedBitmapFilesLocked() { final ArraySet usedFiles = new ArraySet<>(1); forEachShortcut(si -> { if (si.getBitmapPath() != null) { @@ -982,6 +983,26 @@ class ShortcutPackage extends ShortcutPackageItem { return usedFiles; } + public void cleanupDanglingBitmapFiles(@NonNull File path) { + synchronized (mLock) { + mShortcutBitmapSaver.waitForAllSavesLocked(); + final ArraySet usedFiles = getUsedBitmapFilesLocked(); + + for (File child : path.listFiles()) { + if (!child.isFile()) { + continue; + } + final String name = child.getName(); + if (!usedFiles.contains(name)) { + if (ShortcutService.DEBUG) { + Slog.d(TAG, "Removing dangling bitmap file: " + child.getAbsolutePath()); + } + child.delete(); + } + } + } + } + private static String getFileName(@NonNull String path) { final int sep = path.lastIndexOf(File.separatorChar); if (sep == -1) { @@ -1608,6 +1629,11 @@ class ShortcutPackage extends ShortcutPackageItem { pw.print(" ("); pw.print(Formatter.formatFileSize(mShortcutUser.mService.mContext, totalBitmapSize[0])); pw.println(")"); + + pw.println(); + synchronized (mLock) { + mShortcutBitmapSaver.dumpLocked(pw, " "); + } } public void dumpShortcuts(@NonNull PrintWriter pw, int matchFlags) { @@ -1729,7 +1755,7 @@ class ShortcutPackage extends ShortcutPackageItem { // Note: at this point no shortcuts should have bitmaps pending save, but if they do, // just remove the bitmap. if (si.isIconPendingSave()) { - s.removeIconLocked(si); + removeIcon(si); } out.startTag(null, TAG_SHORTCUT); ShortcutService.writeAttr(out, ATTR_ID, si.getId()); diff --git a/services/core/java/com/android/server/pm/ShortcutPackageItem.java b/services/core/java/com/android/server/pm/ShortcutPackageItem.java index 6e0436f208e33..7800183692ca6 100644 --- a/services/core/java/com/android/server/pm/ShortcutPackageItem.java +++ b/services/core/java/com/android/server/pm/ShortcutPackageItem.java @@ -16,8 +16,10 @@ package com.android.server.pm; import android.annotation.NonNull; +import android.annotation.Nullable; import android.content.pm.PackageInfo; import android.content.pm.ShortcutInfo; +import android.graphics.Bitmap; import android.util.AtomicFile; import android.util.Slog; import android.util.TypedXmlSerializer; @@ -50,6 +52,9 @@ abstract class ShortcutPackageItem { protected ShortcutUser mShortcutUser; + @GuardedBy("mLock") + protected ShortcutBitmapSaver mShortcutBitmapSaver; + protected final Object mLock = new Object(); protected ShortcutPackageItem(@NonNull ShortcutUser shortcutUser, @@ -59,6 +64,7 @@ abstract class ShortcutPackageItem { mPackageUserId = packageUserId; mPackageName = Preconditions.checkStringNotEmpty(packageName); mPackageInfo = Objects.requireNonNull(packageInfo); + mShortcutBitmapSaver = new ShortcutBitmapSaver(shortcutUser.mService); } /** @@ -206,7 +212,7 @@ abstract class ShortcutPackageItem { void saveShortcutPackageItem() { // Wait for bitmap saves to conclude before proceeding to saving shortcuts. - mShortcutUser.mService.waitForBitmapSaves(); + waitForBitmapSaves(); // Save each ShortcutPackageItem in a separate Xml file. final File path = getShortcutPackageItemFile(); if (ShortcutService.DEBUG || ShortcutService.DEBUG_REBOOT) { @@ -221,6 +227,35 @@ abstract class ShortcutPackageItem { } } + public boolean waitForBitmapSaves() { + synchronized (mLock) { + return mShortcutBitmapSaver.waitForAllSavesLocked(); + } + } + + public void saveBitmap(ShortcutInfo shortcut, + int maxDimension, Bitmap.CompressFormat format, int quality) { + synchronized (mLock) { + mShortcutBitmapSaver.saveBitmapLocked(shortcut, maxDimension, format, quality); + } + } + + /** + * Wait for all pending saves to finish, and then return the given shortcut's bitmap path. + */ + @Nullable + public String getBitmapPathMayWait(ShortcutInfo shortcut) { + synchronized (mLock) { + return mShortcutBitmapSaver.getBitmapPathMayWaitLocked(shortcut); + } + } + + public void removeIcon(ShortcutInfo shortcut) { + synchronized (mLock) { + mShortcutBitmapSaver.removeIcon(shortcut); + } + } + void removeShortcutPackageItem() { synchronized (mLock) { getShortcutPackageItemFile().delete(); diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java index 780f976d2a400..f2bcf5e461a77 100644 --- a/services/core/java/com/android/server/pm/ShortcutService.java +++ b/services/core/java/com/android/server/pm/ShortcutService.java @@ -363,7 +363,6 @@ public class ShortcutService extends IShortcutService.Stub { private final RoleManager mRoleManager; private final ShortcutRequestPinProcessor mShortcutRequestPinProcessor; - private final ShortcutBitmapSaver mShortcutBitmapSaver; private final ShortcutDumpFiles mShortcutDumpFiles; @GuardedBy("mLock") @@ -490,7 +489,6 @@ public class ShortcutService extends IShortcutService.Stub { mRoleManager = Objects.requireNonNull(mContext.getSystemService(RoleManager.class)); mShortcutRequestPinProcessor = new ShortcutRequestPinProcessor(this, mLock); - mShortcutBitmapSaver = new ShortcutBitmapSaver(this); mShortcutDumpFiles = new ShortcutDumpFiles(this); mIsAppSearchEnabled = DeviceConfig.getBoolean(NAMESPACE_SYSTEMUI, SystemUiDeviceConfigFlags.SHORTCUT_APPSEARCH_INTEGRATION, true) @@ -1063,8 +1061,6 @@ public class ShortcutService extends IShortcutService.Stub { Slog.d(TAG, "Saving to " + path); } - mShortcutBitmapSaver.waitForAllSavesLocked(); - path.getParentFile().mkdirs(); final AtomicFile file = new AtomicFile(path); FileOutputStream os = null; @@ -1388,15 +1384,12 @@ public class ShortcutService extends IShortcutService.Stub { // === Caller validation === - void removeIconLocked(ShortcutInfo shortcut) { - mShortcutBitmapSaver.removeIcon(shortcut); - } - public void cleanupBitmapsForPackage(@UserIdInt int userId, String packageName) { final File packagePath = new File(getUserBitmapFilePath(userId), packageName); if (!packagePath.isDirectory()) { return; } + // ShortcutPackage is already removed at this point, we can safely remove the folder. if (!(FileUtils.deleteContents(packagePath) && packagePath.delete())) { Slog.w(TAG, "Unable to remove directory " + packagePath); } @@ -1437,38 +1430,12 @@ public class ShortcutService extends IShortcutService.Stub { } cleanupBitmapsForPackage(userId, packageName); } else { - cleanupDanglingBitmapFilesLocked(userId, user, packageName, child); + user.getPackageShortcuts(packageName).cleanupDanglingBitmapFiles(child); } } logDurationStat(Stats.CLEANUP_DANGLING_BITMAPS, start); } - /** - * Remove dangling bitmap files for a package. - * - * Note this method must be called with the lock held after calling - * {@link ShortcutBitmapSaver#waitForAllSavesLocked()} to make sure there's no pending bitmap - * saves are going on. - */ - private void cleanupDanglingBitmapFilesLocked(@UserIdInt int userId, @NonNull ShortcutUser user, - @NonNull String packageName, @NonNull File path) { - final ArraySet usedFiles = - user.getPackageShortcuts(packageName).getUsedBitmapFiles(); - - for (File child : path.listFiles()) { - if (!child.isFile()) { - continue; - } - final String name = child.getName(); - if (!usedFiles.contains(name)) { - if (DEBUG) { - Slog.d(TAG, "Removing dangling bitmap file: " + child.getAbsolutePath()); - } - child.delete(); - } - } - } - @VisibleForTesting static class FileOutputStreamWithPath extends FileOutputStream { private final File mFile; @@ -1513,7 +1480,7 @@ public class ShortcutService extends IShortcutService.Stub { } } - void saveIconAndFixUpShortcutLocked(ShortcutInfo shortcut) { + void saveIconAndFixUpShortcutLocked(ShortcutPackage p, ShortcutInfo shortcut) { if (shortcut.hasIconFile() || shortcut.hasIconResource() || shortcut.hasIconUri()) { return; } @@ -1521,7 +1488,7 @@ public class ShortcutService extends IShortcutService.Stub { final long token = injectClearCallingIdentity(); try { // Clear icon info on the shortcut. - removeIconLocked(shortcut); + p.removeIcon(shortcut); final Icon icon = shortcut.getIcon(); if (icon == null) { @@ -1560,8 +1527,7 @@ public class ShortcutService extends IShortcutService.Stub { // just in case. throw ShortcutInfo.getInvalidIconException(); } - mShortcutBitmapSaver.saveBitmapLocked(shortcut, - maxIconDimension, mIconPersistFormat, mIconPersistQuality); + p.saveBitmap(shortcut, maxIconDimension, mIconPersistFormat, mIconPersistQuality); } finally { // Once saved, we won't use the original icon information, so null it out. shortcut.clearIcon(); @@ -2110,7 +2076,7 @@ public class ShortcutService extends IShortcutService.Stub { final boolean replacingIcon = (source.getIcon() != null); if (replacingIcon) { - removeIconLocked(target); + ps.removeIcon(target); } // Note copyNonNullFieldsFrom() does the "updatable with?" check too. @@ -2118,7 +2084,7 @@ public class ShortcutService extends IShortcutService.Stub { target.setTimestamp(injectCurrentTimeMillis()); if (replacingIcon) { - saveIconAndFixUpShortcutLocked(target); + saveIconAndFixUpShortcutLocked(ps, target); } // When we're updating any resource related fields, re-extract the res @@ -3463,7 +3429,7 @@ public class ShortcutService extends IShortcutService.Stub { if (shortcutInfo == null) { return null; } - return getShortcutIconParcelFileDescriptor(shortcutInfo); + return getShortcutIconParcelFileDescriptor(p, shortcutInfo); } } @@ -3476,6 +3442,7 @@ public class ShortcutService extends IShortcutService.Stub { Objects.requireNonNull(shortcutId, "shortcutId"); // Checks shortcuts in memory first + final ShortcutPackage p; synchronized (mLock) { throwIfUserLockedL(userId); throwIfUserLockedL(launcherUserId); @@ -3483,8 +3450,7 @@ public class ShortcutService extends IShortcutService.Stub { getLauncherShortcutsLocked(callingPackage, userId, launcherUserId) .attemptToRestoreIfNeededAndSave(); - final ShortcutPackage p = getUserShortcutsLocked(userId) - .getPackageShortcutsIfExists(packageName); + p = getUserShortcutsLocked(userId).getPackageShortcutsIfExists(packageName); if (p == null) { cb.complete(null); return; @@ -3492,24 +3458,23 @@ public class ShortcutService extends IShortcutService.Stub { final ShortcutInfo shortcutInfo = p.findShortcutById(shortcutId); if (shortcutInfo != null) { - cb.complete(getShortcutIconParcelFileDescriptor(shortcutInfo)); + cb.complete(getShortcutIconParcelFileDescriptor(p, shortcutInfo)); return; } } // Otherwise check persisted shortcuts - getShortcutInfoAsync(launcherUserId, packageName, shortcutId, userId, si -> { - cb.complete(getShortcutIconParcelFileDescriptor(si)); - }); + getShortcutInfoAsync(launcherUserId, packageName, shortcutId, userId, si -> + cb.complete(getShortcutIconParcelFileDescriptor(p, si))); } @Nullable private ParcelFileDescriptor getShortcutIconParcelFileDescriptor( - @Nullable final ShortcutInfo shortcutInfo) { - if (shortcutInfo == null || !shortcutInfo.hasIconFile()) { + @Nullable final ShortcutPackage p, @Nullable final ShortcutInfo shortcutInfo) { + if (p == null || shortcutInfo == null || !shortcutInfo.hasIconFile()) { return null; } - final String path = mShortcutBitmapSaver.getBitmapPathMayWaitLocked(shortcutInfo); + final String path = p.getBitmapPathMayWait(shortcutInfo); if (path == null) { Slog.w(TAG, "null bitmap detected in getShortcutIconFd()"); return null; @@ -4771,9 +4736,6 @@ public class ShortcutService extends IShortcutService.Stub { pw.println(Log.getStackTraceString(mLastWtfStacktrace)); } - pw.println(); - mShortcutBitmapSaver.dumpLocked(pw, " "); - pw.println(); } @@ -5347,9 +5309,11 @@ public class ShortcutService extends IShortcutService.Stub { } } - void waitForBitmapSaves() { + @VisibleForTesting + void waitForBitmapSavesForTest() { synchronized (mLock) { - mShortcutBitmapSaver.waitForAllSavesLocked(); + forEachLoadedUserLocked(u -> + u.forAllPackageItems(ShortcutPackageItem::waitForBitmapSaves)); } } diff --git a/services/core/java/com/android/server/pm/ShortcutUser.java b/services/core/java/com/android/server/pm/ShortcutUser.java index 75e18b547c553..b9fd2fdb96304 100644 --- a/services/core/java/com/android/server/pm/ShortcutUser.java +++ b/services/core/java/com/android/server/pm/ShortcutUser.java @@ -401,6 +401,7 @@ class ShortcutUser { private void saveShortcutPackageItem(TypedXmlSerializer out, ShortcutPackageItem spi, boolean forBackup) throws IOException, XmlPullParserException { + spi.waitForBitmapSaves(); if (forBackup) { if (spi.getPackageUserId() != spi.getOwnerUserId()) { return; // Don't save cross-user information. 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 40943774c0afc..fdf9354747a08 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.waitForBitmapSaves(); + mService.waitForBitmapSavesForTest(); return new File(si.getBitmapPath()).getName(); } @@ -1984,7 +1984,7 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase { if (si == null) { return null; } - mService.waitForBitmapSaves(); + mService.waitForBitmapSavesForTest(); return new File(si.getBitmapPath()).getAbsolutePath(); } @@ -2139,7 +2139,7 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase { } protected boolean bitmapDirectoryExists(String packageName, int userId) { - mService.waitForBitmapSaves(); + mService.waitForBitmapSavesForTest(); 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 411b52155abb2..867890f938ba6 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.waitForBitmapSaves(); + mService.waitForBitmapSavesForTest(); // 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.waitForBitmapSaves(); + mService.waitForBitmapSavesForTest(); 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.waitForBitmapSaves(); + mService.waitForBitmapSavesForTest(); 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.waitForBitmapSaves(); + mService.waitForBitmapSavesForTest(); 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.waitForBitmapSaves(); + mService.waitForBitmapSavesForTest(); assertWith(getCallerShortcuts()) .forShortcutWithId("s1", si -> { assertTrue(si.hasIconResource()); assertEquals(R.drawable.black_32x32, si.getIconResourceId()); }); - mService.waitForBitmapSaves(); + mService.waitForBitmapSavesForTest(); mInjectedCurrentTimeMillis += INTERVAL; // reset throttling @@ -1419,7 +1419,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { getTestContext().getResources(), R.drawable.black_64x64))) .build() ))); - mService.waitForBitmapSaves(); + mService.waitForBitmapSavesForTest(); 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.waitForBitmapSaves(); + mService.waitForBitmapSavesForTest(); 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.waitForBitmapSaves(); + mService.waitForBitmapSavesForTest(); assertWith(getCallerShortcuts()) .forShortcutWithId("s1", si -> { assertTrue(si.hasIconResource()); @@ -1463,7 +1463,7 @@ public class ShortcutManagerTest1 extends BaseShortcutManagerTest { .setIcon(Icon.createWithContentUri("test_uri")) .build() ))); - mService.waitForBitmapSaves(); + mService.waitForBitmapSavesForTest(); assertWith(getCallerShortcuts()) .forShortcutWithId("s1", si -> { assertTrue(si.hasIconUri());