From e0b68d396e3f72cd99934791080c988bf8f700b5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Aur=C3=A9lien=20Pomini?= <pomini@google.com>
Date: Fri, 28 Oct 2022 14:42:23 +0000
Subject: [PATCH] Don't recycle the bitmap on CanvasEngine.onDestroy

A crash was possible due to race conditions if CanvasEngine.onDestroy is called before drawFrame has enough time to finish drawing.

We don't want to recycle the bitmap when onDestroy is called. The bitmap will be automatically recycled after loading, once the frame is drawn. We simply wait for the automatic recycling. This is similar to what was done in GLEngine.onDestroy: a message is queued to destroy the GL context, so the GLEngine first waits that other operations (i.e. drawFrame) are finished before unloading.

Bug: 255920551
Test: manual
Test: atest ImageWallpaperTest
Change-Id: I45b99f572dd89926ccdc1632abb20442bafcf854
---
 .../systemui/wallpapers/ImageWallpaper.java   | 23 +++++++------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java b/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java
index bdb4d28ddce91..dd964195ae316 100644
--- a/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java
+++ b/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java
@@ -599,7 +599,6 @@ public class ImageWallpaper extends WallpaperService {
             getDisplayContext().getSystemService(DisplayManager.class)
                     .unregisterDisplayListener(this);
             mWallpaperLocalColorExtractor.cleanUp();
-            unloadBitmap();
         }
 
         @Override
@@ -677,9 +676,14 @@ public class ImageWallpaper extends WallpaperService {
         void drawFrameOnCanvas(Bitmap bitmap) {
             Trace.beginSection("ImageWallpaper.CanvasEngine#drawFrame");
             Surface surface = mSurfaceHolder.getSurface();
-            Canvas canvas = mWideColorGamut
-                    ? surface.lockHardwareWideColorGamutCanvas()
-                    : surface.lockHardwareCanvas();
+            Canvas canvas = null;
+            try {
+                canvas = mWideColorGamut
+                        ? surface.lockHardwareWideColorGamutCanvas()
+                        : surface.lockHardwareCanvas();
+            } catch (IllegalStateException e) {
+                Log.w(TAG, "Unable to lock canvas", e);
+            }
             if (canvas != null) {
                 Rect dest = mSurfaceHolder.getSurfaceFrame();
                 try {
@@ -710,17 +714,6 @@ public class ImageWallpaper extends WallpaperService {
             }
         }
 
-        private void unloadBitmap() {
-            mBackgroundExecutor.execute(this::unloadBitmapSynchronized);
-        }
-
-        private void unloadBitmapSynchronized() {
-            synchronized (mLock) {
-                mBitmapUsages = 0;
-                unloadBitmapInternal();
-            }
-        }
-
         private void unloadBitmapInternal() {
             Trace.beginSection("ImageWallpaper.CanvasEngine#unloadBitmap");
             if (mBitmap != null) {