From 44e56a8190cc8744c4008014408bfe037c6de41b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Pomini?= Date: Thu, 27 Oct 2022 10:36:09 +0000 Subject: [PATCH 1/3] Load wallpaper with correct user id The CanvasEngine was not using the correct userId to load the wallpaper. This caused the system wallpaper to be displayed to all the users. Bug: 255702022 Test: manual Test: atest ImageWallpaperTest Change-Id: Id09bf12de3ce491cd8973050dc79d2f52eb6f762 --- .../com/android/systemui/wallpapers/ImageWallpaper.java | 5 +++-- .../android/systemui/wallpapers/ImageWallpaperTest.java | 9 ++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java b/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java index 44f6d03207b16..bdb4d28ddce91 100644 --- a/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java +++ b/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java @@ -31,6 +31,7 @@ import android.os.Handler; import android.os.HandlerThread; import android.os.SystemClock; import android.os.Trace; +import android.os.UserHandle; import android.service.wallpaper.WallpaperService; import android.util.ArraySet; import android.util.Log; @@ -738,7 +739,7 @@ public class ImageWallpaper extends WallpaperService { boolean loadSuccess = false; Bitmap bitmap; try { - bitmap = mWallpaperManager.getBitmap(false); + bitmap = mWallpaperManager.getBitmapAsUser(UserHandle.USER_CURRENT, false); if (bitmap != null && bitmap.getByteCount() > RecordingCanvas.MAX_BITMAP_SIZE) { throw new RuntimeException("Wallpaper is too large to draw!"); @@ -757,7 +758,7 @@ public class ImageWallpaper extends WallpaperService { } try { - bitmap = mWallpaperManager.getBitmap(false); + bitmap = mWallpaperManager.getBitmapAsUser(UserHandle.USER_CURRENT, false); } catch (RuntimeException | OutOfMemoryError e) { Log.w(TAG, "Unable to load default wallpaper!", e); bitmap = null; diff --git a/packages/SystemUI/tests/src/com/android/systemui/wallpapers/ImageWallpaperTest.java b/packages/SystemUI/tests/src/com/android/systemui/wallpapers/ImageWallpaperTest.java index c2543589bfb82..a0ce697fbccad 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/wallpapers/ImageWallpaperTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/wallpapers/ImageWallpaperTest.java @@ -26,6 +26,7 @@ import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; @@ -44,6 +45,7 @@ import android.graphics.Rect; import android.hardware.display.DisplayManager; import android.hardware.display.DisplayManagerGlobal; import android.os.Handler; +import android.os.UserHandle; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; import android.view.Display; @@ -135,9 +137,10 @@ public class ImageWallpaperTest extends SysuiTestCase { when(mWallpaperBitmap.getHeight()).thenReturn(mBitmapHeight); // set up wallpaper manager - when(mWallpaperManager.peekBitmapDimensions()).thenReturn( - new Rect(0, 0, mBitmapWidth, mBitmapHeight)); - when(mWallpaperManager.getBitmap(false)).thenReturn(mWallpaperBitmap); + when(mWallpaperManager.peekBitmapDimensions()) + .thenReturn(new Rect(0, 0, mBitmapWidth, mBitmapHeight)); + when(mWallpaperManager.getBitmapAsUser(eq(UserHandle.USER_CURRENT), anyBoolean())) + .thenReturn(mWallpaperBitmap); when(mMockContext.getSystemService(WallpaperManager.class)).thenReturn(mWallpaperManager); // set up surface From e0b68d396e3f72cd99934791080c988bf8f700b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Pomini?= Date: Fri, 28 Oct 2022 14:42:23 +0000 Subject: [PATCH 2/3] 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) { From 0580dc49b1d4a70a3a35f4d5d245220b5910754b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Pomini?= Date: Fri, 28 Oct 2022 17:10:59 +0000 Subject: [PATCH 3/3] Remove 0-length bitmap checks It is not possible to have a wallpaper bitmap of width or height zero. Remove the unnecessary checks. Bug: 243402530 Test: manual Test: atest ImageWallpaperTest Change-Id: I7279c169798ed11441c935192234884c48eaf614 --- .../systemui/wallpapers/ImageWallpaper.java | 3 --- .../wallpapers/ImageWallpaperTest.java | 26 ------------------- 2 files changed, 29 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java b/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java index dd964195ae316..ad97ef4a79bc1 100644 --- a/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java +++ b/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java @@ -764,9 +764,6 @@ public class ImageWallpaper extends WallpaperService { Log.e(TAG, "Attempt to load a recycled bitmap"); } else if (mBitmap == bitmap) { Log.e(TAG, "Loaded a bitmap that was already loaded"); - } else if (bitmap.getWidth() < 1 || bitmap.getHeight() < 1) { - Log.e(TAG, "Attempt to load an invalid wallpaper of length " - + bitmap.getWidth() + "x" + bitmap.getHeight()); } else { // at this point, loading is done correctly. loadSuccess = true; diff --git a/packages/SystemUI/tests/src/com/android/systemui/wallpapers/ImageWallpaperTest.java b/packages/SystemUI/tests/src/com/android/systemui/wallpapers/ImageWallpaperTest.java index a0ce697fbccad..379bb28ae0322 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/wallpapers/ImageWallpaperTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/wallpapers/ImageWallpaperTest.java @@ -28,7 +28,6 @@ import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -289,9 +288,6 @@ public class ImageWallpaperTest extends SysuiTestCase { testMinSurfaceHelper(8, 8); testMinSurfaceHelper(100, 2000); testMinSurfaceHelper(200, 1); - testMinSurfaceHelper(0, 1); - testMinSurfaceHelper(1, 0); - testMinSurfaceHelper(0, 0); } private void testMinSurfaceHelper(int bitmapWidth, int bitmapHeight) { @@ -309,28 +305,6 @@ public class ImageWallpaperTest extends SysuiTestCase { intThat(greaterThanOrEqualTo(ImageWallpaper.CanvasEngine.MIN_SURFACE_HEIGHT))); } - @Test - public void testZeroBitmap() { - // test that a frame is never drawn with a 0 bitmap - testZeroBitmapHelper(0, 1); - testZeroBitmapHelper(1, 0); - testZeroBitmapHelper(0, 0); - } - - private void testZeroBitmapHelper(int bitmapWidth, int bitmapHeight) { - - clearInvocations(mSurfaceHolder); - setBitmapDimensions(bitmapWidth, bitmapHeight); - - ImageWallpaper imageWallpaper = createImageWallpaperCanvas(); - ImageWallpaper.CanvasEngine engine = - (ImageWallpaper.CanvasEngine) imageWallpaper.onCreateEngine(); - ImageWallpaper.CanvasEngine spyEngine = spy(engine); - spyEngine.onCreate(mSurfaceHolder); - spyEngine.onSurfaceRedrawNeeded(mSurfaceHolder); - verify(spyEngine, never()).drawFrameOnCanvas(any()); - } - @Test public void testLoadDrawAndUnloadBitmap() { setBitmapDimensions(LOW_BMP_WIDTH, LOW_BMP_HEIGHT);