From 475c36539da9687d75447adb9054c98ce03c754a Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Mon, 8 May 2017 14:29:03 -0700 Subject: [PATCH] Offload saving bitmaps from binder threads Bug 34691191 Test: adb shell am instrument -e class com.android.server.pm.ShortcutManagerTest1 -w com.android.frameworks.servicestests Test: adb shell am instrument -e class com.android.server.pm.ShortcutManagerTest2 -w com.android.frameworks.servicestests Test: adb shell am instrument -e class com.android.server.pm.ShortcutManagerTest3 -w com.android.frameworks.servicestests Test: adb shell am instrument -e class com.android.server.pm.ShortcutManagerTest4 -w com.android.frameworks.servicestests Test: adb shell am instrument -e class com.android.server.pm.ShortcutManagerTest5 -w com.android.frameworks.servicestests Test: adb shell am instrument -e class com.android.server.pm.ShortcutManagerTest6 -w com.android.frameworks.servicestests Test: adb shell am instrument -e class com.android.server.pm.ShortcutManagerTest7 -w com.android.frameworks.servicestests Test: adb shell am instrument -e class com.android.server.pm.ShortcutManagerTest8 -w com.android.frameworks.servicestests Test: adb shell am instrument -e class com.android.server.pm.ShortcutManagerTest9 -w com.android.frameworks.servicestests Test: adb shell am instrument -e class com.android.server.pm.ShortcutManagerTest10 -w com.android.frameworks.servicestests Test: cts-tradefed run cts-dev --skip-device-info --skip-preconditions --skip-system-status-check com.android.compatibility.common.tradefed.targetprep.NetworkConnectivityChecker -a armeabi-v7a -l INFO -m CtsShortcutManagerTestCases Test: cts-tradefed run cts-dev --skip-device-info --skip-preconditions --skip-system-status-check com.android.compatibility.common.tradefed.targetprep.NetworkConnectivityChecker -a armeabi-v7a -l INFO -m CtsShortcutHostTestCases Test: Manual test with the phone app Change-Id: Ibccf8ca162eae9179ee4cbdf0307ff34a6953aa1 --- .../java/android/content/pm/ShortcutInfo.java | 31 +- .../server/pm/ShortcutBitmapSaver.java | 311 ++++++++++++++++++ .../android/server/pm/ShortcutPackage.java | 15 +- .../android/server/pm/ShortcutService.java | 106 +++--- .../server/pm/BaseShortcutManagerTest.java | 17 + .../server/pm/ShortcutManagerTest2.java | 7 +- 6 files changed, 424 insertions(+), 63 deletions(-) create mode 100644 services/core/java/com/android/server/pm/ShortcutBitmapSaver.java diff --git a/core/java/android/content/pm/ShortcutInfo.java b/core/java/android/content/pm/ShortcutInfo.java index d0c63979d833e..d3a3560c72299 100644 --- a/core/java/android/content/pm/ShortcutInfo.java +++ b/core/java/android/content/pm/ShortcutInfo.java @@ -97,6 +97,9 @@ public final class ShortcutInfo implements Parcelable { /** @hide */ public static final int FLAG_RETURNED_BY_SERVICE = 1 << 10; + /** @hide When this is set, the bitmap icon is waiting to be saved. */ + public static final int FLAG_ICON_FILE_PENDING_SAVE = 1 << 11; + /** @hide */ @IntDef(flag = true, value = { @@ -110,7 +113,8 @@ public final class ShortcutInfo implements Parcelable { FLAG_STRINGS_RESOLVED, FLAG_IMMUTABLE, FLAG_ADAPTIVE_BITMAP, - FLAG_RETURNED_BY_SERVICE + FLAG_RETURNED_BY_SERVICE, + FLAG_ICON_FILE_PENDING_SAVE, }) @Retention(RetentionPolicy.SOURCE) public @interface ShortcutFlags {} @@ -1471,6 +1475,21 @@ public final class ShortcutInfo implements Parcelable { return hasFlags(FLAG_ADAPTIVE_BITMAP); } + /** @hide */ + public boolean isIconPendingSave() { + return hasFlags(FLAG_ICON_FILE_PENDING_SAVE); + } + + /** @hide */ + public void setIconPendingSave() { + addFlags(FLAG_ICON_FILE_PENDING_SAVE); + } + + /** @hide */ + public void clearIconPendingSave() { + clearFlags(FLAG_ICON_FILE_PENDING_SAVE); + } + /** * Return whether a shortcut only contains "key" information only or not. If true, only the * following fields are available. @@ -1534,7 +1553,12 @@ public final class ShortcutInfo implements Parcelable { return mIconResId; } - /** @hide */ + /** + * Bitmap path. Note this will be null even if {@link #hasIconFile()} is set when the save + * is pending. Use {@link #isIconPendingSave()} to check it. + * + * @hide + */ public String getBitmapPath() { return mBitmapPath; } @@ -1780,6 +1804,9 @@ public final class ShortcutInfo implements Parcelable { if (hasIconFile()) { sb.append("If"); } + if (isIconPendingSave()) { + sb.append("^"); + } if (hasIconResource()) { sb.append("Ir"); } diff --git a/services/core/java/com/android/server/pm/ShortcutBitmapSaver.java b/services/core/java/com/android/server/pm/ShortcutBitmapSaver.java new file mode 100644 index 0000000000000..4f5d1560bfb10 --- /dev/null +++ b/services/core/java/com/android/server/pm/ShortcutBitmapSaver.java @@ -0,0 +1,311 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.server.pm; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.content.pm.ShortcutInfo; +import android.graphics.Bitmap; +import android.graphics.Bitmap.CompressFormat; +import android.graphics.drawable.Icon; +import android.os.SystemClock; +import android.util.Log; +import android.util.Slog; + +import com.android.internal.annotations.GuardedBy; +import com.android.internal.util.Preconditions; +import com.android.server.pm.ShortcutService.FileOutputStreamWithPath; + +import libcore.io.IoUtils; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.PrintWriter; +import java.util.Deque; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executor; +import java.util.concurrent.LinkedBlockingDeque; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; + +/** + * Class to save shortcut bitmaps on a worker thread. + * + * The methods with the "Locked" prefix must be called with the service lock held. + */ +public class ShortcutBitmapSaver { + private static final String TAG = ShortcutService.TAG; + private static final boolean DEBUG = ShortcutService.DEBUG; + + private static final boolean ADD_DELAY_BEFORE_SAVE_FOR_TEST = false; // DO NOT submit with true. + private static final long SAVE_DELAY_MS_FOR_TEST = 1000; // DO NOT submit with true. + + /** + * Before saving shortcuts.xml, and returning icons to the launcher, we wait for all pending + * saves to finish. However if it takes more than this long, we just give up and proceed. + */ + private final long SAVE_WAIT_TIMEOUT_MS = 30 * 1000; + + private final ShortcutService mService; + + /** + * Bitmaps are saved on this thread. + * + * Note: Just before saving shortcuts into the XML, we need to wait on all pending saves to + * finish, and we need to do it with the service lock held, which would still block incoming + * binder calls, meaning saving bitmaps *will* still actually block API calls too, which is + * not ideal but fixing it would be tricky, so this is still a known issue on the current + * version. + * + * In order to reduce the conflict, we use an own thread for this purpose, rather than + * reusing existing background threads, and also to avoid possible deadlocks. + */ + private final Executor mExecutor = new ThreadPoolExecutor(0, 1, 60L, TimeUnit.SECONDS, + new LinkedBlockingQueue<>()); + + /** Represents a bitmap to save. */ + private static class PendingItem { + /** Hosting shortcut. */ + public final ShortcutInfo shortcut; + + /** Compressed bitmap data. */ + public final byte[] bytes; + + /** Instantiated time, only for dogfooding. */ + private final long mInstantiatedUptimeMillis; // Only for dumpsys. + + private PendingItem(ShortcutInfo shortcut, byte[] bytes) { + this.shortcut = shortcut; + this.bytes = bytes; + mInstantiatedUptimeMillis = SystemClock.uptimeMillis(); + } + + @Override + public String toString() { + return "PendingItem{size=" + bytes.length + + " age=" + (SystemClock.uptimeMillis() - mInstantiatedUptimeMillis) + "ms" + + " shortcut=" + shortcut.toInsecureString() + + "}"; + } + } + + @GuardedBy("mPendingItems") + private final Deque mPendingItems = new LinkedBlockingDeque<>(); + + public ShortcutBitmapSaver(ShortcutService service) { + mService = service; + // mLock = lock; + } + + public boolean waitForAllSavesLocked() { + final CountDownLatch latch = new CountDownLatch(1); + + mExecutor.execute(() -> latch.countDown()); + + try { + if (latch.await(SAVE_WAIT_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { + return true; + } + mService.wtf("Timed out waiting on saving bitmaps."); + } catch (InterruptedException e) { + Slog.w(TAG, "interrupted"); + } + return false; + } + + /** + * Wait for all pending saves to finish, and then return the given shortcut's bitmap path. + */ + @Nullable + public String getBitmapPathMayWaitLocked(ShortcutInfo shortcut) { + final boolean success = waitForAllSavesLocked(); + if (success && shortcut.hasIconFile()) { + return shortcut.getBitmapPath(); + } else { + return null; + } + } + + public void removeIcon(ShortcutInfo shortcut) { + // Do not remove the actual bitmap file yet, because if the device crashes before saving + // the XML we'd lose the icon. We just remove all dangling files after saving the XML. + shortcut.setIconResourceId(0); + shortcut.setIconResName(null); + shortcut.setBitmapPath(null); + shortcut.clearFlags(ShortcutInfo.FLAG_HAS_ICON_FILE | + ShortcutInfo.FLAG_ADAPTIVE_BITMAP | ShortcutInfo.FLAG_HAS_ICON_RES | + ShortcutInfo.FLAG_ICON_FILE_PENDING_SAVE); + } + + public void saveBitmapLocked(ShortcutInfo shortcut, + int maxDimension, CompressFormat format, int quality) { + final Icon icon = shortcut.getIcon(); + Preconditions.checkNotNull(icon); + + final Bitmap original = icon.getBitmap(); + if (original == null) { + Log.e(TAG, "Missing icon: " + shortcut); + return; + } + + // Compress it and enqueue to the requests. + final byte[] bytes; + try { + final Bitmap shrunk = mService.shrinkBitmap(original, maxDimension); + try { + try (final ByteArrayOutputStream out = new ByteArrayOutputStream(64 * 1024)) { + if (!shrunk.compress(format, quality, out)) { + Slog.wtf(ShortcutService.TAG, "Unable to compress bitmap"); + } + out.flush(); + bytes = out.toByteArray(); + out.close(); + } + } finally { + if (shrunk != original) { + shrunk.recycle(); + } + } + } catch (IOException | RuntimeException | OutOfMemoryError e) { + Slog.wtf(ShortcutService.TAG, "Unable to write bitmap to file", e); + return; + } + + shortcut.addFlags( + ShortcutInfo.FLAG_HAS_ICON_FILE | ShortcutInfo.FLAG_ICON_FILE_PENDING_SAVE); + + if (icon.getType() == Icon.TYPE_ADAPTIVE_BITMAP) { + shortcut.addFlags(ShortcutInfo.FLAG_ADAPTIVE_BITMAP); + } + + // Enqueue a pending save. + final PendingItem item = new PendingItem(shortcut, bytes); + synchronized (mPendingItems) { + mPendingItems.add(item); + } + + if (DEBUG) { + Slog.d(TAG, "Scheduling to save: " + item); + } + + mExecutor.execute(mRunnable); + } + + private final Runnable mRunnable = () -> { + // Process all pending items. + while (processPendingItems()) { + } + }; + + /** + * Takes a {@link PendingItem} from {@link #mPendingItems} and process it. + * + * Must be called {@link #mExecutor}. + * + * @return true if it processed an item, false if the queue is empty. + */ + private boolean processPendingItems() { + if (ADD_DELAY_BEFORE_SAVE_FOR_TEST) { + Slog.w(TAG, "*** ARTIFICIAL SLEEP ***"); + try { + Thread.sleep(SAVE_DELAY_MS_FOR_TEST); + } catch (InterruptedException e) { + } + } + + // NOTE: + // Ideally we should be holding the service lock when accessing shortcut instances, + // but that could cause a deadlock so we don't do it. + // + // Instead, waitForAllSavesLocked() uses a latch to make sure changes made on this + // thread is visible on the caller thread. + + ShortcutInfo shortcut = null; + try { + final PendingItem item; + + synchronized (mPendingItems) { + if (mPendingItems.size() == 0) { + return false; + } + item = mPendingItems.pop(); + } + + shortcut = item.shortcut; + + // See if the shortcut is still relevant. (It might have been removed already.) + if (!shortcut.isIconPendingSave()) { + return true; + } + + if (DEBUG) { + Slog.d(TAG, "Saving bitmap: " + item); + } + + File file = null; + try { + final FileOutputStreamWithPath out = mService.openIconFileForWrite( + shortcut.getUserId(), shortcut); + file = out.getFile(); + + try { + out.write(item.bytes); + } finally { + IoUtils.closeQuietly(out); + } + + shortcut.setBitmapPath(file.getAbsolutePath()); + + } catch (IOException | RuntimeException e) { + Slog.e(ShortcutService.TAG, "Unable to write bitmap to file", e); + + if (file != null && file.exists()) { + file.delete(); + } + return true; + } + } finally { + if (DEBUG) { + Slog.d(TAG, "Saved bitmap."); + } + if (shortcut != null) { + if (shortcut.getBitmapPath() == null) { + removeIcon(shortcut); + } + + // Whatever happened, remove this flag. + shortcut.clearFlags(ShortcutInfo.FLAG_ICON_FILE_PENDING_SAVE); + } + } + return true; + } + + public void dumpLocked(@NonNull PrintWriter pw, @NonNull String prefix) { + synchronized (mPendingItems) { + final int N = mPendingItems.size(); + pw.print(prefix); + pw.println("Pending saves: Num=" + N + " Executor=" + mExecutor); + + for (PendingItem item : mPendingItems) { + pw.print(prefix); + pw.print(" "); + pw.println(item); + } + } + } +} diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java index 5035e6820be83..103b25d4cea03 100644 --- a/services/core/java/com/android/server/pm/ShortcutPackage.java +++ b/services/core/java/com/android/server/pm/ShortcutPackage.java @@ -198,7 +198,7 @@ class ShortcutPackage extends ShortcutPackageItem { private ShortcutInfo deleteShortcutInner(@NonNull String id) { final ShortcutInfo shortcut = mShortcuts.remove(id); if (shortcut != null) { - mShortcutUser.mService.removeIcon(getPackageUserId(), shortcut); + mShortcutUser.mService.removeIconLocked(shortcut); shortcut.clearFlags(ShortcutInfo.FLAG_DYNAMIC | ShortcutInfo.FLAG_PINNED | ShortcutInfo.FLAG_MANIFEST); } @@ -211,7 +211,7 @@ class ShortcutPackage extends ShortcutPackageItem { deleteShortcutInner(newShortcut.getId()); // Extract Icon and update the icon res ID and the bitmap path. - s.saveIconAndFixUpShortcut(getPackageUserId(), newShortcut); + s.saveIconAndFixUpShortcutLocked(newShortcut); s.fixUpShortcutResourceNamesAndValues(newShortcut); mShortcuts.put(newShortcut.getId(), newShortcut); } @@ -1263,13 +1263,21 @@ class ShortcutPackage extends ShortcutPackageItem { out.endTag(null, TAG_ROOT); } - private static void saveShortcut(XmlSerializer out, ShortcutInfo si, boolean forBackup) + private void saveShortcut(XmlSerializer out, ShortcutInfo si, boolean forBackup) throws IOException, XmlPullParserException { + + final ShortcutService s = mShortcutUser.mService; + if (forBackup) { if (!(si.isPinned() && si.isEnabled())) { return; // We only backup pinned shortcuts that are enabled. } } + // 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); + } out.startTag(null, TAG_SHORTCUT); ShortcutService.writeAttr(out, ATTR_ID, si.getId()); // writeAttr(out, "package", si.getPackageName()); // not needed @@ -1293,6 +1301,7 @@ class ShortcutPackage extends ShortcutPackageItem { ShortcutService.writeAttr(out, ATTR_FLAGS, si.getFlags() & ~(ShortcutInfo.FLAG_HAS_ICON_FILE | ShortcutInfo.FLAG_HAS_ICON_RES + | ShortcutInfo.FLAG_ICON_FILE_PENDING_SAVE | ShortcutInfo.FLAG_DYNAMIC)); } else { // When writing for backup, ranks shouldn't be saved, since shortcuts won't be restored diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java index 7f0528a49098a..ac4b828ef099a 100644 --- a/services/core/java/com/android/server/pm/ShortcutService.java +++ b/services/core/java/com/android/server/pm/ShortcutService.java @@ -306,6 +306,7 @@ public class ShortcutService extends IShortcutService.Stub { private final ActivityManagerInternal mActivityManagerInternal; private final ShortcutRequestPinProcessor mShortcutRequestPinProcessor; + private final ShortcutBitmapSaver mShortcutBitmapSaver; @GuardedBy("mLock") final SparseIntArray mUidState = new SparseIntArray(); @@ -426,6 +427,7 @@ public class ShortcutService extends IShortcutService.Stub { LocalServices.getService(ActivityManagerInternal.class)); mShortcutRequestPinProcessor = new ShortcutRequestPinProcessor(this, mLock); + mShortcutBitmapSaver = new ShortcutBitmapSaver(this); if (onlyForPackageManagerApis) { return; // Don't do anything further. For unit tests only. @@ -926,6 +928,9 @@ public class ShortcutService extends IShortcutService.Stub { if (DEBUG) { Slog.d(TAG, "Saving to " + path); } + + mShortcutBitmapSaver.waitForAllSavesLocked(); + path.getParentFile().mkdirs(); final AtomicFile file = new AtomicFile(path); FileOutputStream os = null; @@ -1213,13 +1218,8 @@ public class ShortcutService extends IShortcutService.Stub { // === Caller validation === - void removeIcon(@UserIdInt int userId, ShortcutInfo shortcut) { - // Do not remove the actual bitmap file yet, because if the device crashes before saving - // he XML we'd lose the icon. We just remove all dangling files after saving the XML. - shortcut.setIconResourceId(0); - shortcut.setIconResName(null); - shortcut.clearFlags(ShortcutInfo.FLAG_HAS_ICON_FILE | - ShortcutInfo.FLAG_ADAPTIVE_BITMAP | ShortcutInfo.FLAG_HAS_ICON_RES); + void removeIconLocked(ShortcutInfo shortcut) { + mShortcutBitmapSaver.removeIcon(shortcut); } public void cleanupBitmapsForPackage(@UserIdInt int userId, String packageName) { @@ -1232,6 +1232,13 @@ public class ShortcutService extends IShortcutService.Stub { } } + /** + * Remove dangling bitmap files for a user. + * + * 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 cleanupDanglingBitmapDirectoriesLocked(@UserIdInt int userId) { if (DEBUG) { Slog.d(TAG, "cleanupDanglingBitmaps: userId=" + userId); @@ -1265,6 +1272,13 @@ public class ShortcutService extends IShortcutService.Stub { 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 = @@ -1303,7 +1317,6 @@ public class ShortcutService extends IShortcutService.Stub { * * The filename will be based on the ID, except certain characters will be escaped. */ - @VisibleForTesting FileOutputStreamWithPath openIconFileForWrite(@UserIdInt int userId, ShortcutInfo shortcut) throws IOException { final File packagePath = new File(getUserBitmapFilePath(userId), @@ -1329,7 +1342,7 @@ public class ShortcutService extends IShortcutService.Stub { } } - void saveIconAndFixUpShortcut(@UserIdInt int userId, ShortcutInfo shortcut) { + void saveIconAndFixUpShortcutLocked(ShortcutInfo shortcut) { if (shortcut.hasIconFile() || shortcut.hasIconResource()) { return; } @@ -1337,7 +1350,7 @@ public class ShortcutService extends IShortcutService.Stub { final long token = injectClearCallingIdentity(); try { // Clear icon info on the shortcut. - removeIcon(userId, shortcut); + removeIconLocked(shortcut); final Icon icon = shortcut.getIcon(); if (icon == null) { @@ -1364,41 +1377,8 @@ public class ShortcutService extends IShortcutService.Stub { // just in case. throw ShortcutInfo.getInvalidIconException(); } - if (bitmap == null) { - Slog.e(TAG, "Null bitmap detected"); - return; - } - // Shrink and write to the file. - File path = null; - try { - final FileOutputStreamWithPath out = openIconFileForWrite(userId, shortcut); - try { - path = out.getFile(); - - Bitmap shrunk = shrinkBitmap(bitmap, mMaxIconDimension); - try { - shrunk.compress(mIconPersistFormat, mIconPersistQuality, out); - } finally { - if (bitmap != shrunk) { - shrunk.recycle(); - } - } - - shortcut.setBitmapPath(out.getFile().getAbsolutePath()); - shortcut.addFlags(ShortcutInfo.FLAG_HAS_ICON_FILE); - if (icon.getType() == Icon.TYPE_ADAPTIVE_BITMAP) { - shortcut.addFlags(ShortcutInfo.FLAG_ADAPTIVE_BITMAP); - } - } finally { - IoUtils.closeQuietly(out); - } - } catch (IOException | RuntimeException e) { - // STOPSHIP Change wtf to e - Slog.wtf(ShortcutService.TAG, "Unable to write bitmap to file", e); - if (path != null && path.exists()) { - path.delete(); - } - } + mShortcutBitmapSaver.saveBitmapLocked(shortcut, + mMaxIconDimension, mIconPersistFormat, mIconPersistQuality); } finally { // Once saved, we won't use the original icon information, so null it out. shortcut.clearIcon(); @@ -1418,7 +1398,6 @@ public class ShortcutService extends IShortcutService.Stub { } } - @VisibleForTesting static Bitmap shrinkBitmap(Bitmap in, int maxSize) { // Original width/height. final int ow = in.getWidth(); @@ -1787,7 +1766,7 @@ public class ShortcutService extends IShortcutService.Stub { final boolean replacingIcon = (source.getIcon() != null); if (replacingIcon) { - removeIcon(userId, target); + removeIconLocked(target); } // Note copyNonNullFieldsFrom() does the "updatable with?" check too. @@ -1795,7 +1774,7 @@ public class ShortcutService extends IShortcutService.Stub { target.setTimestamp(injectCurrentTimeMillis()); if (replacingIcon) { - saveIconAndFixUpShortcut(userId, target); + saveIconAndFixUpShortcutLocked(target); } // When we're updating any resource related fields, re-extract the res names and @@ -2613,16 +2592,17 @@ public class ShortcutService extends IShortcutService.Stub { if (shortcutInfo == null || !shortcutInfo.hasIconFile()) { return null; } + final String path = mShortcutBitmapSaver.getBitmapPathMayWaitLocked(shortcutInfo); + if (path == null) { + Slog.w(TAG, "null bitmap detected in getShortcutIconFd()"); + return null; + } try { - if (shortcutInfo.getBitmapPath() == null) { - Slog.w(TAG, "null bitmap detected in getShortcutIconFd()"); - return null; - } return ParcelFileDescriptor.open( - new File(shortcutInfo.getBitmapPath()), + new File(path), ParcelFileDescriptor.MODE_READ_ONLY); } catch (FileNotFoundException e) { - Slog.e(TAG, "Icon file not found: " + shortcutInfo.getBitmapPath()); + Slog.e(TAG, "Icon file not found: " + path); return null; } } @@ -3384,6 +3364,9 @@ public class ShortcutService extends IShortcutService.Stub { scheduleSaveUser(userId); saveDirtyInfo(); + // Note, in case of backup, we don't have to wait on bitmap saving, because we don't + // back up bitmaps anyway. + // Then create the backup payload. final ByteArrayOutputStream os = new ByteArrayOutputStream(32 * 1024); try { @@ -3516,6 +3499,9 @@ public class ShortcutService extends IShortcutService.Stub { pw.println(Log.getStackTraceString(mLastWtfStacktrace)); } + pw.println(); + mShortcutBitmapSaver.dumpLocked(pw, " "); + for (int i = 0; i < mUsers.size(); i++) { pw.println(); mUsers.valueAt(i).dump(pw, " "); @@ -3827,6 +3813,11 @@ public class ShortcutService extends IShortcutService.Stub { return SystemClock.elapsedRealtime(); } + @VisibleForTesting + long injectUptimeMillis() { + return SystemClock.uptimeMillis(); + } + // Injection point. @VisibleForTesting int injectBinderCallingUid() { @@ -3997,4 +3988,11 @@ public class ShortcutService extends IShortcutService.Stub { forEachLoadedUserLocked(u -> u.forAllPackageItems(ShortcutPackageItem::verifyStates)); } } + + @VisibleForTesting + void waitForBitmapSavesForTest() { + synchronized (mLock) { + mShortcutBitmapSaver.waitForAllSavesLocked(); + } + } } 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 d281e5aed3cc1..f1d592789c410 100644 --- a/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java @@ -278,6 +278,11 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase { return mInjectedCurrentTimeMillis - START_TIME; } + @Override + long injectUptimeMillis() { + return mInjectedCurrentTimeMillis - START_TIME - mDeepSleepTime; + } + @Override int injectBinderCallingUid() { return mInjectedCallingUid; @@ -557,6 +562,7 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase { protected boolean mSafeMode; protected long mInjectedCurrentTimeMillis; + protected long mDeepSleepTime; // Used to calculate "uptimeMillis". protected boolean mInjectedIsLowRamDevice; @@ -1707,9 +1713,19 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase { if (si == null) { return null; } + mService.waitForBitmapSavesForTest(); return new File(si.getBitmapPath()).getName(); } + protected String getBitmapAbsPath(int userId, String packageName, String shortcutId) { + final ShortcutInfo si = mService.getPackageShortcutForTest(packageName, shortcutId, userId); + if (si == null) { + return null; + } + mService.waitForBitmapSavesForTest(); + return new File(si.getBitmapPath()).getAbsolutePath(); + } + /** * @return all shortcuts stored internally for the caller. This reflects the *internal* view * of shortcuts, which may be different from what {@link #getCallerVisibleShortcuts} would @@ -1826,6 +1842,7 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase { } protected boolean bitmapDirectoryExists(String packageName, int userId) { + 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/ShortcutManagerTest2.java b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java index 2b40c511ef14c..3220ea960f5df 100644 --- a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java +++ b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java @@ -2026,12 +2026,11 @@ public class ShortcutManagerTest2 extends BaseShortcutManagerTest { makeShortcutWithIcon("bmp32x32", bmp32x32), makeShortcutWithIcon("bmp64x64", bmp64x64)))); }); + // We can't predict the compressed bitmap sizes, so get the real sizes here. final long bitmapTotal = - new File(getPackageShortcut(CALLING_PACKAGE_2, "bmp32x32", USER_0) - .getBitmapPath()).length() + - new File(getPackageShortcut(CALLING_PACKAGE_2, "bmp64x64", USER_0) - .getBitmapPath()).length(); + new File(getBitmapAbsPath(USER_0, CALLING_PACKAGE_2, "bmp32x32")).length() + + new File(getBitmapAbsPath(USER_0, CALLING_PACKAGE_2, "bmp64x64")).length(); // Read the expected output and inject the bitmap size. final String expected = readTestAsset("shortcut/dumpsys_expected.txt")