Merge "Move bitmap saving into corresponding ShortcutPackage" into tm-dev

This commit is contained in:
Pinyao Ting
2022-05-04 19:33:15 +00:00
committed by Android (Google) Code Review
6 changed files with 100 additions and 74 deletions

View File

@@ -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<String> getUsedBitmapFiles() {
@GuardedBy("mLock")
private ArraySet<String> getUsedBitmapFilesLocked() {
final ArraySet<String> 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<String> 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());

View File

@@ -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();

View File

@@ -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<String> 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));
}
}

View File

@@ -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.

View File

@@ -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();
}

View File

@@ -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());