Merge "Offload saving bitmaps from binder threads" into oc-dev

am: 31ef139466

Change-Id: I2980b05a8a997e003792fe995973b54eaf29d5e1

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");
         }

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<PendingItem> 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);
+            }
+        }
+    }
+}

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

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) {
+    void removeIconLocked(ShortcutInfo shortcut) {
-        // Do not remove the actual bitmap file yet, because if the device crashes before saving
+        mShortcutBitmapSaver.removeIcon(shortcut);
-        // 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);
     }
 
     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<String> 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) {
+                mShortcutBitmapSaver.saveBitmapLocked(shortcut,
-                    Slog.e(TAG, "Null bitmap detected");
+                        mMaxIconDimension, mIconPersistFormat, mIconPersistQuality);
-                    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();
-                    }
-                }
             } 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();
+        }
+    }
 }

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

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)
+                new File(getBitmapAbsPath(USER_0, CALLING_PACKAGE_2, "bmp32x32")).length() +
-                        .getBitmapPath()).length() +
+                new File(getBitmapAbsPath(USER_0, CALLING_PACKAGE_2, "bmp64x64")).length();
-                new File(getPackageShortcut(CALLING_PACKAGE_2, "bmp64x64", USER_0)
-                        .getBitmapPath()).length();
 
         // Read the expected output and inject the bitmap size.
         final String expected = readTestAsset("shortcut/dumpsys_expected.txt")