DO NOT MERGE: Fix temporary black after setting a new image wallpaper

Sometimes see temporary black wallpaper after setting a new image
wallpaper because of:
- decode bitmap twice, results in long latency until drawing.
- the set wallpaper apis didn't sync with wallpaper rendering well.
Solutions:
- Only decode bitmap when necessary.
- Make set wallpaper apis wait for image wallpaper rendering finished.

Bug: 194080642
Test: see b/194080642#comment3
Test: atest WallpaperManagerServiceTests --iterations 50
Test: atest SystemUITests
Change-Id: I369eff5195571161cf286b3ec97175481b02eed7

core/java/android/app/WallpaperManager.java

@@ -559,6 +559,53 @@ public class WallpaperManager {
             return null;
         }
 
+        public Rect peekWallpaperDimensions(Context context, boolean returnDefault, int userId) {
+            if (mService != null) {
+                try {
+                    if (!mService.isWallpaperSupported(context.getOpPackageName())) {
+                        return new Rect();
+                    }
+                } catch (RemoteException e) {
+                    throw e.rethrowFromSystemServer();
+                }
+            }
+
+            Rect dimensions = null;
+            synchronized (this) {
+                try {
+                    Bundle params = new Bundle();
+                    // Let's peek user wallpaper first.
+                    ParcelFileDescriptor pfd = mService.getWallpaperWithFeature(
+                            context.getOpPackageName(), context.getAttributionTag(), this,
+                            FLAG_SYSTEM, params, userId);
+                    if (pfd != null) {
+                        BitmapFactory.Options options = new BitmapFactory.Options();
+                        options.inJustDecodeBounds = true;
+                        BitmapFactory.decodeFileDescriptor(pfd.getFileDescriptor(), null, options);
+                        dimensions = new Rect(0, 0, options.outWidth, options.outHeight);
+                    }
+                } catch (RemoteException ex) {
+                    Log.w(TAG, "peek wallpaper dimensions failed", ex);
+                }
+            }
+            // If user wallpaper is unavailable, may be the default one instead.
+            if ((dimensions == null || dimensions.width() == 0 || dimensions.height() == 0)
+                    && returnDefault) {
+                InputStream is = openDefaultWallpaper(context, FLAG_SYSTEM);
+                if (is != null) {
+                    try {
+                        BitmapFactory.Options options = new BitmapFactory.Options();
+                        options.inJustDecodeBounds = true;
+                        BitmapFactory.decodeStream(is, null, options);
+                        dimensions = new Rect(0, 0, options.outWidth, options.outHeight);
+                    } finally {
+                        IoUtils.closeQuietly(is);
+                    }
+                }
+            }
+            return dimensions;
+        }
+
         void forgetLoadedWallpaper() {
             synchronized (this) {
                 mCachedWallpaper = null;
@@ -1038,6 +1085,17 @@ public class WallpaperManager {
         return sGlobals.peekWallpaperBitmap(mContext, true, FLAG_SYSTEM, userId, hardware, cmProxy);
     }
 
+    /**
+     * Peek the dimensions of system wallpaper of the user without decoding it.
+     *
+     * @return the dimensions of system wallpaper
+     * @hide
+     */
+    public Rect peekBitmapDimensions() {
+        return sGlobals.peekWallpaperDimensions(
+                mContext, true /* returnDefault */, mContext.getUserId());
+    }
+
     /**
      * Get an open, readable file descriptor to the given wallpaper image file.
      * The caller is responsible for closing the file descriptor when done ingesting the file.

core/java/android/service/wallpaper/WallpaperService.java

@@ -96,6 +96,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Supplier;
 
@@ -152,6 +153,7 @@ public abstract class WallpaperService extends Service {
     private static final int MSG_REQUEST_WALLPAPER_COLORS = 10050;
     private static final int MSG_ZOOM = 10100;
     private static final int MSG_SCALE_PREVIEW = 10110;
+    private static final int MSG_REPORT_SHOWN = 10150;
     private static final List<Float> PROHIBITED_STEPS = Arrays.asList(0f, Float.POSITIVE_INFINITY,
             Float.NEGATIVE_INFINITY);
 
@@ -526,6 +528,35 @@ public abstract class WallpaperService extends Service {
             return false;
         }
 
+        /**
+         * This will be called in the end of {@link #updateSurface(boolean, boolean, boolean)}.
+         * If true is returned, the engine will not report shown until rendering finished is
+         * reported. Otherwise, the engine will report shown immediately right after redraw phase
+         * in {@link #updateSurface(boolean, boolean, boolean)}.
+         *
+         * @hide
+         */
+        public boolean shouldWaitForEngineShown() {
+            return false;
+        }
+
+        /**
+         * Reports the rendering is finished, stops waiting, then invokes
+         * {@link IWallpaperEngineWrapper#reportShown()}.
+         *
+         * @hide
+         */
+        public void reportEngineShown(boolean waitForEngineShown) {
+            if (mIWallpaperEngine.mShownReported) return;
+            Message message = mCaller.obtainMessage(MSG_REPORT_SHOWN);
+            if (!waitForEngineShown) {
+                mCaller.removeMessages(MSG_REPORT_SHOWN);
+                mCaller.sendMessage(message);
+            } else {
+                mCaller.sendMessageDelayed(message, TimeUnit.SECONDS.toMillis(1));
+            }
+        }
+
         /**
          * Control whether this wallpaper will receive raw touch events
          * from the window manager as the user interacts with the window
@@ -930,7 +961,7 @@ public abstract class WallpaperService extends Service {
 
         void updateSurface(boolean forceRelayout, boolean forceReport, boolean redrawNeeded) {
             if (mDestroyed) {
-                Log.w(TAG, "Ignoring updateSurface: destroyed");
+                Log.w(TAG, "Ignoring updateSurface due to destroyed");
             }
 
             boolean fixedSize = false;
@@ -1197,7 +1228,6 @@ public abstract class WallpaperService extends Service {
                                         + this);
                             onVisibilityChanged(false);
                         }
-
                     } finally {
                         mIsCreating = false;
                         mSurfaceCreated = true;
@@ -1207,7 +1237,7 @@ public abstract class WallpaperService extends Service {
                             processLocalColors(mPendingXOffset, mPendingXOffsetStep);
                         }
                         reposition();
-                        mIWallpaperEngine.reportShown();
+                        reportEngineShown(shouldWaitForEngineShown());
                     }
                 } catch (RemoteException ex) {
                 }
@@ -2201,6 +2231,9 @@ public abstract class WallpaperService extends Service {
                         // Connection went away, nothing to do in here.
                     }
                 } break;
+                case MSG_REPORT_SHOWN: {
+                    reportShown();
+                } break;
                 default :
                     Log.w(TAG, "Unknown message type " + message.what);
             }

packages/SystemUI/src/com/android/systemui/ImageWallpaper.java

@@ -120,15 +120,16 @@ public class ImageWallpaper extends WallpaperService {
 
         @Override
         public void onCreate(SurfaceHolder surfaceHolder) {
+            Trace.beginSection("ImageWallpaper.Engine#onCreate");
             mEglHelper = getEglHelperInstance();
             // Deferred init renderer because we need to get wallpaper by display context.
             mRenderer = getRendererInstance();
             setFixedSizeAllowed(true);
             updateSurfaceSize();
-
             mRenderer.setOnBitmapChanged(this::updateMiniBitmap);
             getDisplayContext().getSystemService(DisplayManager.class)
                     .registerDisplayListener(this, mWorker.getThreadHandler());
+            Trace.endSection();
         }
 
         @Override
@@ -197,16 +198,23 @@ public class ImageWallpaper extends WallpaperService {
             return true;
         }
 
+        @Override
+        public boolean shouldWaitForEngineShown() {
+            return true;
+        }
+
         @Override
         public void onDestroy() {
             getDisplayContext().getSystemService(DisplayManager.class)
                     .unregisterDisplayListener(this);
             mMiniBitmap = null;
             mWorker.getThreadHandler().post(() -> {
+                Trace.beginSection("ImageWallpaper.Engine#onDestroy");
                 mRenderer.finish();
                 mRenderer = null;
                 mEglHelper.finish();
                 mEglHelper = null;
+                Trace.endSection();
             });
         }
 
@@ -340,8 +348,10 @@ public class ImageWallpaper extends WallpaperService {
         public void onSurfaceCreated(SurfaceHolder holder) {
             if (mWorker == null) return;
             mWorker.getThreadHandler().post(() -> {
+                Trace.beginSection("ImageWallpaper#onSurfaceCreated");
                 mEglHelper.init(holder, needSupportWideColorGamut());
                 mRenderer.onSurfaceCreated();
+                Trace.endSection();
             });
         }
 
@@ -358,9 +368,11 @@ public class ImageWallpaper extends WallpaperService {
         }
 
         private void drawFrame() {
+            Trace.beginSection("ImageWallpaper#drawFrame");
             preRender();
             requestRender();
             postRender();
+            Trace.endSection();
         }
 
         public void preRender() {
@@ -427,6 +439,7 @@ public class ImageWallpaper extends WallpaperService {
             // This method should only be invoked from worker thread.
             Trace.beginSection("ImageWallpaper#postRender");
             scheduleFinishRendering();
+            reportEngineShown(false /* waitForEngineShown */);
             Trace.endSection();
         }
 

packages/SystemUI/src/com/android/systemui/glwallpaper/ImageWallpaperRenderer.java

@@ -102,7 +102,6 @@ public class ImageWallpaperRenderer implements GLWallpaperRenderer {
 
     @Override
     public Size reportSurfaceSize() {
-        mTexture.use(null /* consumer */);
         mSurfaceSize.set(mTexture.getTextureDimensions());
         return new Size(mSurfaceSize.width(), mSurfaceSize.height());
     }
@@ -124,6 +123,7 @@ public class ImageWallpaperRenderer implements GLWallpaperRenderer {
         private final WallpaperManager mWallpaperManager;
         private Bitmap mBitmap;
         private boolean mWcgContent;
+        private boolean mTextureUsed;
 
         private WallpaperTexture(WallpaperManager wallpaperManager) {
             mWallpaperManager = wallpaperManager;
@@ -141,6 +141,7 @@ public class ImageWallpaperRenderer implements GLWallpaperRenderer {
                     mWallpaperManager.forgetLoadedWallpaper();
                     if (mBitmap != null) {
                         mDimensions.set(0, 0, mBitmap.getWidth(), mBitmap.getHeight());
+                        mTextureUsed = true;
                     } else {
                         Log.w(TAG, "Can't get bitmap");
                     }
@@ -171,6 +172,9 @@ public class ImageWallpaperRenderer implements GLWallpaperRenderer {
         }
 
         private Rect getTextureDimensions() {
+            if (!mTextureUsed) {
+                mDimensions.set(mWallpaperManager.peekBitmapDimensions());
+            }
             return mDimensions;
         }
 

services/core/java/com/android/server/wallpaper/WallpaperManagerService.java

@@ -208,7 +208,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
      * wallpaper set and is created for the first time. The CLOSE_WRITE is triggered
      * every time the wallpaper is changed.
      */
-    private class WallpaperObserver extends FileObserver {
+    class WallpaperObserver extends FileObserver {
 
         final int mUserId;
         final WallpaperData mWallpaper;
@@ -226,7 +226,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
             mWallpaperLockFile = new File(mWallpaperDir, WALLPAPER_LOCK_ORIG);
         }
 
-        private WallpaperData dataForEvent(boolean sysChanged, boolean lockChanged) {
+        WallpaperData dataForEvent(boolean sysChanged, boolean lockChanged) {
             WallpaperData wallpaper = null;
             synchronized (mLock) {
                 if (lockChanged) {
@@ -309,9 +309,18 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
                             }
                             wallpaper.imageWallpaperPending = false;
                             if (sysWallpaperChanged) {
+                                IRemoteCallback.Stub callback = new IRemoteCallback.Stub() {
+                                    @Override
+                                    public void sendResult(Bundle data) throws RemoteException {
+                                        if (DEBUG) {
+                                            Slog.d(TAG, "publish system wallpaper changed!");
+                                        }
+                                        notifyWallpaperChanged(wallpaper);
+                                    }
+                                };
                                 // If this was the system wallpaper, rebind...
                                 bindWallpaperComponentLocked(mImageWallpaper, true,
-                                        false, wallpaper, null);
+                                        false, wallpaper, callback);
                                 notifyColorsWhich |= FLAG_SYSTEM;
                             }
                             if (lockWallpaperChanged
@@ -331,15 +340,9 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
                             }
 
                             saveSettingsLocked(wallpaper.userId);
-
+                            // Notify the client immediately if only lockscreen wallpaper changed.
-                            // Publish completion *after* we've persisted the changes
+                            if (lockWallpaperChanged && !sysWallpaperChanged) {
-                            if (wallpaper.setComplete != null) {
+                                notifyWallpaperChanged(wallpaper);
-                                try {
-                                    wallpaper.setComplete.onWallpaperChanged();
-                                } catch (RemoteException e) {
-                                    // if this fails we don't really care; the setting app may just
-                                    // have crashed and that sort of thing is a fact of life.
-                                }
                             }
                         }
                     }
@@ -353,6 +356,18 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
         }
     }
 
+    private void notifyWallpaperChanged(WallpaperData wallpaper) {
+        // Publish completion *after* we've persisted the changes
+        if (wallpaper.setComplete != null) {
+            try {
+                wallpaper.setComplete.onWallpaperChanged();
+            } catch (RemoteException e) {
+                // if this fails we don't really care; the setting app may just
+                // have crashed and that sort of thing is a fact of life.
+            }
+        }
+    }
+
     private void notifyLockWallpaperChanged() {
         final IWallpaperManagerCallback cb = mKeyguardListener;
         if (cb != null) {
@@ -364,7 +379,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
         }
     }
 
-    private void notifyWallpaperColorsChanged(@NonNull WallpaperData wallpaper, int which) {
+    void notifyWallpaperColorsChanged(@NonNull WallpaperData wallpaper, int which) {
         if (wallpaper.connection != null) {
             wallpaper.connection.forEachDisplayConnector(connector -> {
                 notifyWallpaperColorsChangedOnDisplay(wallpaper, which, connector.mDisplayId);
@@ -568,7 +583,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
      * Once a new wallpaper has been written via setWallpaper(...), it needs to be cropped
      * for display.
      */
-    private void generateCrop(WallpaperData wallpaper) {
+    void generateCrop(WallpaperData wallpaper) {
         boolean success = false;
 
         // Only generate crop for default display.
@@ -2834,7 +2849,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
         return false;
     }
 
-    private boolean bindWallpaperComponentLocked(ComponentName componentName, boolean force,
+    boolean bindWallpaperComponentLocked(ComponentName componentName, boolean force,
             boolean fromUser, WallpaperData wallpaper, IRemoteCallback reply) {
         if (DEBUG_LIVE) {
             Slog.v(TAG, "bindWallpaperComponentLocked: componentName=" + componentName);
@@ -3121,7 +3136,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
         return new JournaledFile(new File(base), new File(base + ".tmp"));
     }
 
-    private void saveSettingsLocked(int userId) {
+    void saveSettingsLocked(int userId) {
         JournaledFile journal = makeJournaledFile(userId);
         FileOutputStream fstream = null;
         try {
@@ -3270,7 +3285,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub
      * 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) {
+    WallpaperData getWallpaperSafeLocked(int userId, int which) {
         // We're setting either just system (work with the system wallpaper),
         // both (also work with the system wallpaper), or just the lock
         // wallpaper (update against the existing lock wallpaper if any).

services/tests/mockingservicestests/src/com/android/server/wallpaper/WallpaperManagerServiceTests.java

@@ -18,13 +18,18 @@ package com.android.server.wallpaper;
 
 import static android.app.WallpaperManager.COMMAND_REAPPLY;
 import static android.app.WallpaperManager.FLAG_SYSTEM;
+import static android.os.FileObserver.CLOSE_WRITE;
+import static android.os.UserHandle.USER_SYSTEM;
 import static android.view.Display.DEFAULT_DISPLAY;
 
+import static com.android.dx.mockito.inline.extended.ExtendedMockito.doAnswer;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.mock;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;
+import static com.android.server.wallpaper.WallpaperManagerService.WALLPAPER;
+import static com.android.server.wallpaper.WallpaperManagerService.WALLPAPER_CROP;
 
 import static org.hamcrest.core.IsNot.not;
 import static org.junit.Assert.assertEquals;
@@ -34,6 +39,7 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.junit.Assume.assumeThat;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.reset;
@@ -41,6 +47,7 @@ import static org.mockito.Mockito.verify;
 
 import android.app.AppGlobals;
 import android.app.AppOpsManager;
+import android.app.WallpaperColors;
 import android.app.WallpaperManager;
 import android.content.ComponentName;
 import android.content.Context;
@@ -49,8 +56,10 @@ import android.content.pm.IPackageManager;
 import android.content.pm.PackageManager;
 import android.content.pm.ParceledListSlice;
 import android.content.pm.ServiceInfo;
+import android.graphics.Color;
 import android.hardware.display.DisplayManager;
-import android.os.UserHandle;
+import android.os.RemoteException;
+import android.os.SystemClock;
 import android.platform.test.annotations.Presubmit;
 import android.service.wallpaper.IWallpaperConnection;
 import android.service.wallpaper.IWallpaperEngine;
@@ -143,7 +152,6 @@ public class WallpaperManagerServiceTests {
         sContext.getTestablePermissions().setPermission(
                 android.Manifest.permission.SET_WALLPAPER,
                 PackageManager.PERMISSION_GRANTED);
-        doNothing().when(sContext).sendBroadcastAsUser(any(), any());
 
         //Wallpaper components
         sWallpaperService = mock(IWallpaperConnection.Stub.class);
@@ -180,6 +188,7 @@ public class WallpaperManagerServiceTests {
         MockitoAnnotations.initMocks(this);
 
         sContext.addMockSystemService(DisplayManager.class, mDisplayManager);
+        doNothing().when(sContext).sendBroadcastAsUser(any(), any());
 
         final Display mockDisplay = mock(Display.class);
         doReturn(DISPLAY_SIZE_DIMENSION).when(mockDisplay).getMaximumSizeDimension();
@@ -242,13 +251,13 @@ public class WallpaperManagerServiceTests {
      */
     @Test
     public void testDataCorrectAfterBoot() {
-        mService.switchUser(UserHandle.USER_SYSTEM, null);
+        mService.switchUser(USER_SYSTEM, null);
 
         final WallpaperData fallbackData = mService.mFallbackWallpaper;
         assertEquals("Fallback wallpaper component should be ImageWallpaper.",
                 sImageWallpaperComponentName, fallbackData.wallpaperComponent);
 
-        verifyLastWallpaperData(UserHandle.USER_SYSTEM, sDefaultWallpaperComponent);
+        verifyLastWallpaperData(USER_SYSTEM, sDefaultWallpaperComponent);
         verifyDisplayData();
     }
 
@@ -261,7 +270,7 @@ public class WallpaperManagerServiceTests {
         assumeThat(sDefaultWallpaperComponent,
                 not(CoreMatchers.equalTo(sImageWallpaperComponentName)));
 
-        final int testUserId = UserHandle.USER_SYSTEM;
+        final int testUserId = USER_SYSTEM;
         mService.switchUser(testUserId, null);
         verifyLastWallpaperData(testUserId, sDefaultWallpaperComponent);
         verifyCurrentSystemData(testUserId);
@@ -281,7 +290,7 @@ public class WallpaperManagerServiceTests {
      */
     @Test
     public void testSetCurrentComponent() throws Exception {
-        final int testUserId = UserHandle.USER_SYSTEM;
+        final int testUserId = USER_SYSTEM;
         mService.switchUser(testUserId, null);
         verifyLastWallpaperData(testUserId, sDefaultWallpaperComponent);
         verifyCurrentSystemData(testUserId);
@@ -387,6 +396,42 @@ public class WallpaperManagerServiceTests {
         assertEquals(systemWallpaperData.primaryColors, shouldMatchSystem.primaryColors);
     }
 
+    @Test
+    public void testWallpaperManagerCallbackInRightOrder() throws RemoteException {
+        WallpaperData wallpaper = new WallpaperData(
+                USER_SYSTEM, mService.getWallpaperDir(USER_SYSTEM), WALLPAPER, WALLPAPER_CROP);
+        wallpaper.primaryColors = new WallpaperColors(Color.valueOf(Color.RED),
+                Color.valueOf(Color.BLUE), null);
+
+        spyOn(wallpaper);
+        doReturn(wallpaper).when(mService).getWallpaperSafeLocked(wallpaper.userId, FLAG_SYSTEM);
+        doNothing().when(mService).switchWallpaper(any(), any());
+        doReturn(true).when(mService)
+                .bindWallpaperComponentLocked(any(), anyBoolean(), anyBoolean(), any(), any());
+        doNothing().when(mService).saveSettingsLocked(wallpaper.userId);
+        doNothing().when(mService).generateCrop(wallpaper);
+
+        // timestamps of {ACTION_WALLPAPER_CHANGED, onWallpaperColorsChanged}
+        final long[] timestamps = new long[2];
+        doAnswer(invocation -> timestamps[0] = SystemClock.elapsedRealtime())
+                .when(sContext).sendBroadcastAsUser(any(), any());
+        doAnswer(invocation -> timestamps[1] = SystemClock.elapsedRealtime())
+                .when(mService).notifyWallpaperColorsChanged(wallpaper, FLAG_SYSTEM);
+
+        assertNull(wallpaper.wallpaperObserver);
+        mService.switchUser(wallpaper.userId, null);
+        assertNotNull(wallpaper.wallpaperObserver);
+        // We will call onEvent directly, so stop watching the file.
+        wallpaper.wallpaperObserver.stopWatching();
+
+        spyOn(wallpaper.wallpaperObserver);
+        doReturn(wallpaper).when(wallpaper.wallpaperObserver).dataForEvent(true, false);
+        wallpaper.wallpaperObserver.onEvent(CLOSE_WRITE, WALLPAPER);
+
+        // ACTION_WALLPAPER_CHANGED should be invoked before onWallpaperColorsChanged.
+        assertTrue(timestamps[1] > timestamps[0]);
+    }
+
     // Verify that after continue switch user from userId 0 to lastUserId, the wallpaper data for
     // non-current user must not bind to wallpaper service.
     private void verifyNoConnectionBeforeLastUser(int lastUserId) {