diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java index 151f61e391925..f3ce79cf755fa 100644 --- a/services/core/java/com/android/server/pm/ShortcutPackage.java +++ b/services/core/java/com/android/server/pm/ShortcutPackage.java @@ -451,6 +451,30 @@ class ShortcutPackage extends ShortcutPackageItem { mApiCallCount = 0; } + /** + * Return the filenames (excluding path names) of icon bitmap files from this package. + */ + public ArraySet getUsedBitmapFiles() { + final ArraySet usedFiles = new ArraySet<>(mShortcuts.size()); + + for (int i = mShortcuts.size() - 1; i >= 0; i--) { + final ShortcutInfo si = mShortcuts.valueAt(i); + if (si.getBitmapPath() != null) { + usedFiles.add(getFileName(si.getBitmapPath())); + } + } + return usedFiles; + } + + private static String getFileName(@NonNull String path) { + final int sep = path.lastIndexOf(File.separatorChar); + if (sep == -1) { + return path; + } else { + return path.substring(sep + 1); + } + } + /** * Called when the package is updated. If there are shortcuts with resource icons, update * their timestamps. diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java index 0f17804673352..1d8923c975ccd 100644 --- a/services/core/java/com/android/server/pm/ShortcutService.java +++ b/services/core/java/com/android/server/pm/ShortcutService.java @@ -291,8 +291,9 @@ public class ShortcutService extends IShortcutService.Stub { int GET_PACKAGE_INFO_WITH_SIG = 2; int GET_APPLICATION_INFO = 3; int LAUNCHER_PERMISSION_CHECK = 4; + int CLEANUP_DANGLING_BITMAPS = 5; - int COUNT = LAUNCHER_PERMISSION_CHECK + 1; + int COUNT = CLEANUP_DANGLING_BITMAPS + 1; } final Object mStatLock = new Object(); @@ -328,7 +329,7 @@ public class ShortcutService extends IShortcutService.Stub { void logDurationStat(int statId, long start) { synchronized (mStatLock) { mCountStats[statId]++; - mDurationStats[statId] += (System.currentTimeMillis() - start); + mDurationStats[statId] += (injectElapsedRealtime() - start); } } @@ -804,7 +805,9 @@ public class ShortcutService extends IShortcutService.Stub { return null; } try { - return loadUserInternal(userId, in, /* forBackup= */ false); + final ShortcutUser ret = loadUserInternal(userId, in, /* forBackup= */ false); + cleanupDanglingBitmapDirectoriesLocked(userId, ret); + return ret; } catch (IOException|XmlPullParserException e) { Slog.e(TAG, "Failed to read file " + file.getBaseFile(), e); return null; @@ -1001,6 +1004,57 @@ public class ShortcutService extends IShortcutService.Stub { } } + private void cleanupDanglingBitmapDirectoriesLocked( + @UserIdInt int userId, @NonNull ShortcutUser user) { + if (DEBUG) { + Slog.d(TAG, "cleanupDanglingBitmaps: userId=" + userId); + } + final long start = injectElapsedRealtime(); + + final File bitmapDir = getUserBitmapFilePath(userId); + final File[] children = bitmapDir.listFiles(); + if (children == null) { + return; + } + for (File child : children) { + if (!child.isDirectory()) { + continue; + } + final String packageName = child.getName(); + if (DEBUG) { + Slog.d(TAG, "cleanupDanglingBitmaps: Found directory=" + packageName); + } + if (!user.hasPackage(packageName)) { + if (DEBUG) { + Slog.d(TAG, "Removing dangling bitmap directory: " + packageName); + } + cleanupBitmapsForPackage(userId, packageName); + } else { + cleanupDanglingBitmapFilesLocked(userId, user, packageName, child); + } + } + logDurationStat(Stats.CLEANUP_DANGLING_BITMAPS, start); + } + + private void cleanupDanglingBitmapFilesLocked(@UserIdInt int userId, @NonNull ShortcutUser user, + @NonNull String packageName, @NonNull File path) { + final ArraySet usedFiles = + user.getPackageShortcuts(this, 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; @@ -1596,14 +1650,14 @@ public class ShortcutService extends IShortcutService.Stub { @VisibleForTesting boolean hasShortcutHostPermissionInner(@NonNull String callingPackage, int userId) { synchronized (mLock) { - final long start = System.currentTimeMillis(); + final long start = injectElapsedRealtime(); final ShortcutUser user = getUserShortcutsLocked(userId); final List allHomeCandidates = new ArrayList<>(); // Default launcher from package manager. - final long startGetHomeActivitiesAsUser = System.currentTimeMillis(); + final long startGetHomeActivitiesAsUser = injectElapsedRealtime(); final ComponentName defaultLauncher = injectPackageManagerInternal() .getHomeActivitiesAsUser(allHomeCandidates, userId); logDurationStat(Stats.GET_DEFAULT_HOME, startGetHomeActivitiesAsUser); @@ -2075,7 +2129,7 @@ public class ShortcutService extends IShortcutService.Stub { @VisibleForTesting PackageInfo injectPackageInfo(String packageName, @UserIdInt int userId, boolean getSignatures) { - final long start = System.currentTimeMillis(); + final long start = injectElapsedRealtime(); final long token = injectClearCallingIdentity(); try { return mIPackageManager.getPackageInfo(packageName, PACKAGE_MATCH_FLAGS @@ -2096,7 +2150,7 @@ public class ShortcutService extends IShortcutService.Stub { @VisibleForTesting ApplicationInfo injectApplicationInfo(String packageName, @UserIdInt int userId) { - final long start = System.currentTimeMillis(); + final long start = injectElapsedRealtime(); final long token = injectClearCallingIdentity(); try { return mIPackageManager.getApplicationInfo(packageName, PACKAGE_MATCH_FLAGS, userId); @@ -2269,6 +2323,8 @@ public class ShortcutService extends IShortcutService.Stub { dumpStatLS(pw, p, Stats.GET_PACKAGE_INFO, "getPackageInfo()"); dumpStatLS(pw, p, Stats.GET_PACKAGE_INFO_WITH_SIG, "getPackageInfo(SIG)"); dumpStatLS(pw, p, Stats.GET_APPLICATION_INFO, "getApplicationInfo"); + + dumpStatLS(pw, p, Stats.CLEANUP_DANGLING_BITMAPS, "cleanupDanglingBitmaps"); } for (int i = 0; i < mUsers.size(); i++) { diff --git a/services/core/java/com/android/server/pm/ShortcutUser.java b/services/core/java/com/android/server/pm/ShortcutUser.java index 7d19a78dda6b4..2efae8675b084 100644 --- a/services/core/java/com/android/server/pm/ShortcutUser.java +++ b/services/core/java/com/android/server/pm/ShortcutUser.java @@ -116,6 +116,10 @@ class ShortcutUser { return mPackages; } + public boolean hasPackage(@NonNull String packageName) { + return mPackages.containsKey(packageName); + } + public ShortcutPackage removePackage(@NonNull ShortcutService s, @NonNull String packageName) { final ShortcutPackage removed = mPackages.remove(packageName); diff --git a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest.java b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest.java index ced7cf03e9385..928d420f6dd88 100644 --- a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest.java @@ -156,6 +156,8 @@ public class ShortcutManagerTest extends InstrumentationTestCase { private static final boolean DUMP_IN_TEARDOWN = false; // DO NOT SUBMIT WITH true + private static final String[] EMPTY_STRINGS = new String[0]; // Just for readability. + // public for mockito public class BaseContext extends MockContext { @Override @@ -1047,6 +1049,49 @@ public class ShortcutManagerTest extends InstrumentationTestCase { } } + private void assertBitmapDirectories(int userId, String... expectedDirectories) { + final Set expected = hashSet(set(expectedDirectories)); + + final Set actual = new HashSet<>(); + + final File[] files = mService.getUserBitmapFilePath(userId).listFiles(); + if (files != null) { + for (File child : files) { + if (child.isDirectory()) { + actual.add(child.getName()); + } + } + } + + assertEquals(expected, actual); + } + + private void assertBitmapFiles(int userId, String packageName, String... expectedFiles) { + final Set expected = hashSet(set(expectedFiles)); + + final Set actual = new HashSet<>(); + + final File[] files = new File(mService.getUserBitmapFilePath(userId), packageName) + .listFiles(); + if (files != null) { + for (File child : files) { + if (child.isFile()) { + actual.add(child.getName()); + } + } + } + + assertEquals(expected, actual); + } + + private String getBitmapFilename(int userId, String packageName, String shortcutId) { + final ShortcutInfo si = mService.getPackageShortcutForTest(packageName, shortcutId, userId); + if (si == null) { + return null; + } + return new File(si.getBitmapPath()).getName(); + } + private ShortcutInfo getPackageShortcut(String packageName, String shortcutId) { return getPackageShortcut(packageName, shortcutId, getCallingUserId()); } @@ -1732,8 +1777,173 @@ public class ShortcutManagerTest extends InstrumentationTestCase { bmp = pfdToBitmap( mLauncherApps.getShortcutIconFd(CALLING_PACKAGE_1, "bmp32x32", HANDLE_USER_P0)); assertBitmapSize(128, 128, bmp); + } - // TODO Test the content URI case too. + private File makeFile(File baseDirectory, String... paths) { + File ret = baseDirectory; + + for (String path : paths) { + ret = new File(ret, path); + } + + return ret; + } + + public void testCleanupDanglingBitmaps() throws Exception { + assertBitmapDirectories(USER_0, EMPTY_STRINGS); + assertBitmapDirectories(USER_10, EMPTY_STRINGS); + + // Make some shortcuts with bitmap icons. + final Icon bmp32x32 = Icon.createWithBitmap(BitmapFactory.decodeResource( + getTestContext().getResources(), R.drawable.black_32x32)); + + runWithCaller(CALLING_PACKAGE_1, USER_0, () -> { + mManager.setDynamicShortcuts(list( + makeShortcutWithIcon("s1", bmp32x32), + makeShortcutWithIcon("s2", bmp32x32), + makeShortcutWithIcon("s3", bmp32x32) + )); + }); + + // Increment the time (which actually we don't have to), which is used for filenames. + mInjectedCurrentTimeLillis++; + + runWithCaller(CALLING_PACKAGE_2, USER_0, () -> { + mManager.setDynamicShortcuts(list( + makeShortcutWithIcon("s4", bmp32x32), + makeShortcutWithIcon("s5", bmp32x32), + makeShortcutWithIcon("s6", bmp32x32) + )); + }); + + // Increment the time, which is used for filenames. + mInjectedCurrentTimeLillis++; + + runWithCaller(CALLING_PACKAGE_3, USER_0, () -> { + mManager.setDynamicShortcuts(list( + )); + }); + + // For USER-10, let's try without updating the times. + runWithCaller(CALLING_PACKAGE_1, USER_10, () -> { + mManager.setDynamicShortcuts(list( + makeShortcutWithIcon("10s1", bmp32x32), + makeShortcutWithIcon("10s2", bmp32x32), + makeShortcutWithIcon("10s3", bmp32x32) + )); + }); + runWithCaller(CALLING_PACKAGE_2, USER_10, () -> { + mManager.setDynamicShortcuts(list( + makeShortcutWithIcon("10s4", bmp32x32), + makeShortcutWithIcon("10s5", bmp32x32), + makeShortcutWithIcon("10s6", bmp32x32) + )); + }); + runWithCaller(CALLING_PACKAGE_3, USER_10, () -> { + mManager.setDynamicShortcuts(list( + )); + }); + + dumpsysOnLogcat(); + + // 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); + assertBitmapDirectories(USER_10, CALLING_PACKAGE_1, CALLING_PACKAGE_2); + + assertBitmapFiles(USER_0, CALLING_PACKAGE_1, + getBitmapFilename(USER_0, CALLING_PACKAGE_1, "s1"), + getBitmapFilename(USER_0, CALLING_PACKAGE_1, "s2"), + getBitmapFilename(USER_0, CALLING_PACKAGE_1, "s3") + ); + assertBitmapFiles(USER_0, CALLING_PACKAGE_2, + getBitmapFilename(USER_0, CALLING_PACKAGE_2, "s4"), + getBitmapFilename(USER_0, CALLING_PACKAGE_2, "s5"), + getBitmapFilename(USER_0, CALLING_PACKAGE_2, "s6") + ); + assertBitmapFiles(USER_0, CALLING_PACKAGE_3, + EMPTY_STRINGS + ); + assertBitmapFiles(USER_10, CALLING_PACKAGE_1, + getBitmapFilename(USER_10, CALLING_PACKAGE_1, "10s1"), + getBitmapFilename(USER_10, CALLING_PACKAGE_1, "10s2"), + getBitmapFilename(USER_10, CALLING_PACKAGE_1, "10s3") + ); + assertBitmapFiles(USER_10, CALLING_PACKAGE_2, + getBitmapFilename(USER_10, CALLING_PACKAGE_2, "10s4"), + getBitmapFilename(USER_10, CALLING_PACKAGE_2, "10s5"), + getBitmapFilename(USER_10, CALLING_PACKAGE_2, "10s6") + ); + assertBitmapFiles(USER_10, CALLING_PACKAGE_3, + EMPTY_STRINGS + ); + + // Then create random directories and files. + makeFile(mService.getUserBitmapFilePath(USER_0), "a.b.c").mkdir(); + makeFile(mService.getUserBitmapFilePath(USER_0), "d.e.f").mkdir(); + makeFile(mService.getUserBitmapFilePath(USER_0), "d.e.f", "123").createNewFile(); + makeFile(mService.getUserBitmapFilePath(USER_0), "d.e.f", "456").createNewFile(); + + makeFile(mService.getUserBitmapFilePath(USER_0), CALLING_PACKAGE_3).mkdir(); + + makeFile(mService.getUserBitmapFilePath(USER_0), CALLING_PACKAGE_1, "1").createNewFile(); + makeFile(mService.getUserBitmapFilePath(USER_0), CALLING_PACKAGE_1, "2").createNewFile(); + makeFile(mService.getUserBitmapFilePath(USER_0), CALLING_PACKAGE_1, "3").createNewFile(); + makeFile(mService.getUserBitmapFilePath(USER_0), CALLING_PACKAGE_1, "4").createNewFile(); + + makeFile(mService.getUserBitmapFilePath(USER_10), "10a.b.c").mkdir(); + makeFile(mService.getUserBitmapFilePath(USER_10), "10d.e.f").mkdir(); + makeFile(mService.getUserBitmapFilePath(USER_10), "10d.e.f", "123").createNewFile(); + makeFile(mService.getUserBitmapFilePath(USER_10), "10d.e.f", "456").createNewFile(); + + makeFile(mService.getUserBitmapFilePath(USER_10), CALLING_PACKAGE_2, "1").createNewFile(); + makeFile(mService.getUserBitmapFilePath(USER_10), CALLING_PACKAGE_2, "2").createNewFile(); + makeFile(mService.getUserBitmapFilePath(USER_10), CALLING_PACKAGE_2, "3").createNewFile(); + makeFile(mService.getUserBitmapFilePath(USER_10), CALLING_PACKAGE_2, "4").createNewFile(); + + assertBitmapDirectories(USER_0, CALLING_PACKAGE_1, CALLING_PACKAGE_2, CALLING_PACKAGE_3, + "a.b.c", "d.e.f"); + + // Save and load. When a user is loaded, we do the cleanup. + mService.saveDirtyInfo(); + initService(); + + mService.handleUnlockUser(USER_0); + mService.handleUnlockUser(USER_10); + mService.handleUnlockUser(20); // Make sure the logic will still work for nonexistent user. + + // The below check is the same as above, except this time USER_0 use the CALLING_PACKAGE_3 + // directory. + + assertBitmapDirectories(USER_0, CALLING_PACKAGE_1, CALLING_PACKAGE_2, CALLING_PACKAGE_3); + assertBitmapDirectories(USER_10, CALLING_PACKAGE_1, CALLING_PACKAGE_2); + + assertBitmapFiles(USER_0, CALLING_PACKAGE_1, + getBitmapFilename(USER_0, CALLING_PACKAGE_1, "s1"), + getBitmapFilename(USER_0, CALLING_PACKAGE_1, "s2"), + getBitmapFilename(USER_0, CALLING_PACKAGE_1, "s3") + ); + assertBitmapFiles(USER_0, CALLING_PACKAGE_2, + getBitmapFilename(USER_0, CALLING_PACKAGE_2, "s4"), + getBitmapFilename(USER_0, CALLING_PACKAGE_2, "s5"), + getBitmapFilename(USER_0, CALLING_PACKAGE_2, "s6") + ); + assertBitmapFiles(USER_0, CALLING_PACKAGE_3, + EMPTY_STRINGS + ); + assertBitmapFiles(USER_10, CALLING_PACKAGE_1, + getBitmapFilename(USER_10, CALLING_PACKAGE_1, "10s1"), + getBitmapFilename(USER_10, CALLING_PACKAGE_1, "10s2"), + getBitmapFilename(USER_10, CALLING_PACKAGE_1, "10s3") + ); + assertBitmapFiles(USER_10, CALLING_PACKAGE_2, + getBitmapFilename(USER_10, CALLING_PACKAGE_2, "10s4"), + getBitmapFilename(USER_10, CALLING_PACKAGE_2, "10s5"), + getBitmapFilename(USER_10, CALLING_PACKAGE_2, "10s6") + ); + assertBitmapFiles(USER_10, CALLING_PACKAGE_3, + EMPTY_STRINGS + ); } private void checkShrinkBitmap(int expectedWidth, int expectedHeight, int resId, int maxSize) {