From f717b9350926205f85063ce2d178f88c97215f7b Mon Sep 17 00:00:00 2001
From: Christopher Tate <ctate@google.com>
Date: Mon, 11 Sep 2017 15:52:54 -0700
Subject: [PATCH] Make sure that updated wallpaper id is immediate

As soon as setBitmap() or equivalent returns, the new wallpaper id needs
to be immediately observable.  This was being subverted by inconsistent
and racy "initialize from persisted state" handling in the set-wallpaper
case: a load triggered by rebinding the static image display service
could in some cases wind up overriding the new state calculated while
new wallpaper imagery was being processed.

The fix is to clarify the semantics of when load-from-persisted happens:
it is now done *only* when the user is first spun up (or at boot, for
the system user), and a firm guarantee provided about the up-front
availability of the associated bookkeeping.  That, in turn, means not
having to futz with lazy init when some client wants to read the current
wallpaper imagery, and eliminating that gets rid of the races.

And in a strictly-cosmetic fix, corrected the descriptive text for one
of the permission enforcement calls.  Copypasta strikes again!

Bug: 65016846
Test: cts-tradefed run cts-dev -m CtsAppTestCases -t android.app.cts.WallpaperManagerTest\#setBitmapTest
Change-Id: I73da48a58cca1849f073b8aea72019916dc2272b
---
 .../wallpaper/WallpaperManagerService.java    | 26 +++++++++++--------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/services/core/java/com/android/server/wallpaper/WallpaperManagerService.java b/services/core/java/com/android/server/wallpaper/WallpaperManagerService.java
index b963561dfd3c7..0536ff155513c 100644
--- a/services/core/java/com/android/server/wallpaper/WallpaperManagerService.java
+++ b/services/core/java/com/android/server/wallpaper/WallpaperManagerService.java
@@ -1138,7 +1138,13 @@ public class WallpaperManagerService extends IWallpaperManager.Stub {
         mMonitor = new MyPackageMonitor();
         mMonitor.register(context, null, UserHandle.ALL, true);
         getWallpaperDir(UserHandle.USER_SYSTEM).mkdirs();
+
+        // Initialize state from the persistent store, then guarantee that the
+        // WallpaperData for the system imagery is instantiated & active, creating
+        // it from defaults if necessary.
         loadSettingsLocked(UserHandle.USER_SYSTEM, false);
+        getWallpaperSafeLocked(UserHandle.USER_SYSTEM, FLAG_SYSTEM);
+
         mColorsChangedListeners = new SparseArray<>();
     }
 
@@ -1627,13 +1633,10 @@ public class WallpaperManagerService extends IWallpaperManager.Stub {
                     (which == FLAG_LOCK) ? mLockWallpaperMap : mWallpaperMap;
             WallpaperData wallpaper = whichSet.get(wallpaperUserId);
             if (wallpaper == null) {
-                // common case, this is the first lookup post-boot of the system or
-                // unified lock, so we bring up the saved state lazily now and recheck.
-                loadSettingsLocked(wallpaperUserId, false);
-                wallpaper = whichSet.get(wallpaperUserId);
-                if (wallpaper == null) {
-                    return null;
-                }
+                // There is no established wallpaper imagery of this type (expected
+                // only for lock wallpapers; a system WallpaperData is established at
+                // user switch)
+                return null;
             }
             try {
                 if (outParams != null) {
@@ -1658,7 +1661,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub {
     @Override
     public WallpaperInfo getWallpaperInfo(int userId) {
         userId = ActivityManager.handleIncomingUser(Binder.getCallingPid(),
-                Binder.getCallingUid(), userId, false, true, "getWallpaperIdForUser", null);
+                Binder.getCallingUid(), userId, false, true, "getWallpaperInfo", null);
         synchronized (mLock) {
             WallpaperData wallpaper = mWallpaperMap.get(userId);
             if (wallpaper != null && wallpaper.connection != null) {
@@ -2260,7 +2263,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub {
     private void writeWallpaperAttributes(XmlSerializer out, String tag, WallpaperData wallpaper)
             throws IllegalArgumentException, IllegalStateException, IOException {
         if (DEBUG) {
-            Slog.v(TAG, "writeWallpaperAttributes");
+            Slog.v(TAG, "writeWallpaperAttributes id=" + wallpaper.wallpaperId);
         }
         out.startTag(null, tag);
         out.attribute(null, "id", Integer.toString(wallpaper.wallpaperId));
@@ -2359,6 +2362,9 @@ public class WallpaperManagerService extends IWallpaperManager.Stub {
      * the event yet.  We use this safe method when we don't care about this ordering and just
      * want to update the data.  The data is going to be applied when the user switch observer
      * is eventually executed.
+     *
+     * Important: this method loads settings to initialize the given user's wallpaper data if
+     * there is no current in-memory state.
      */
     private WallpaperData getWallpaperSafeLocked(int userId, int which) {
         // We're setting either just system (work with the system wallpaper),
@@ -2397,8 +2403,6 @@ public class WallpaperManagerService extends IWallpaperManager.Stub {
     }
 
     private void loadSettingsLocked(int userId, boolean keepDimensionHints) {
-        if (DEBUG) Slog.v(TAG, "loadSettingsLocked");
-
         JournaledFile journal = makeJournaledFile(userId);
         FileInputStream stream = null;
         File file = journal.chooseForRead();