commit 6f9ac2b1703c62f53cd5c504e1ac7a3371b4fbe9
author Lucas Dupin <dupin@google.com> Fri Jul 14 11:32:21 2023 -0700
committer Lucas Dupin <dupin@google.com> Mon Jul 17 17:08:39 2023 +0000
tree 7883b60799f3596e6fb17dc836b892f5db87648c
parent ddb03a45b8e06cb84df7568ec95e683f83e47d9c

Avoid sending messages of destroyed Engine

And app can call notifyColorsChanged() on a destroyed engine, which
would cause problems, given it doesn't have a surface anymore.

The same is true to visibility notification, where the engine can be
destroyed before it had the opportuinity of drawing its first frame (if
it had to load large assets for example)

Test: manually change wallpapers
Test: WallpaperServiceTest (both CTS and internal)
Fixes: 291053623
Change-Id: Iff83603695cff491f2045586d5b3f4658f7ae55a
(cherry picked from commit 650a7f8afc00885afd6590e07b3a2e4c9ced404e)

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

diff --git a/core/java/android/service/wallpaper/WallpaperService.java b/core/java/android/service/wallpaper/WallpaperService.java
index dbc1be1..d9ac4850 100644
--- a/core/java/android/service/wallpaper/WallpaperService.java
+++ b/core/java/android/service/wallpaper/WallpaperService.java
@@ -868,6 +868,11 @@
          * This will trigger a {@link #onComputeColors()} call.
          */
         public void notifyColorsChanged() {
+            if (mDestroyed) {
+                Log.i(TAG, "Ignoring notifyColorsChanged(), Engine has already been destroyed.");
+                return;
+            }
+
             final long now = mClockFunction.get();
             if (now - mLastColorInvalidation < NOTIFY_COLORS_RATE_LIMIT_MS) {
                 Log.w(TAG, "This call has been deferred. You should only call "
@@ -2226,7 +2231,11 @@
             }
         }
 
-        void detach() {
+        /**
+         * @hide
+         */
+        @VisibleForTesting
+        public void detach() {
             if (mDestroyed) {
                 return;
             }
@@ -2442,6 +2451,14 @@
         }
 
         public void reportShown() {
+            if (mEngine == null) {
+                Log.i(TAG, "Can't report null engine as shown.");
+                return;
+            }
+            if (mEngine.mDestroyed) {
+                Log.i(TAG, "Engine was destroyed before we could draw.");
+                return;
+            }
             if (!mShownReported) {
                 mShownReported = true;
                 Trace.beginSection("WPMS.mConnection.engineShown");

tests/Internal/src/android/service/wallpaper/OWNERS

diff --git a/tests/Internal/src/android/service/wallpaper/OWNERS b/tests/Internal/src/android/service/wallpaper/OWNERS
new file mode 100644
index 0000000..5a26d0e
--- /dev/null
+++ b/tests/Internal/src/android/service/wallpaper/OWNERS
@@ -0,0 +1,4 @@
+dupin@google.com
+santie@google.com
+pomini@google.com
+poultney@google.com
\ No newline at end of file

tests/Internal/src/android/service/wallpaper/WallpaperServiceTest.java

diff --git a/tests/Internal/src/android/service/wallpaper/WallpaperServiceTest.java b/tests/Internal/src/android/service/wallpaper/WallpaperServiceTest.java
index 153ca79..0c5e8d48 100644
--- a/tests/Internal/src/android/service/wallpaper/WallpaperServiceTest.java
+++ b/tests/Internal/src/android/service/wallpaper/WallpaperServiceTest.java
@@ -85,4 +85,17 @@
         assertEquals("onAmbientModeChanged should have been called", 2, zoomChangedCount[0]);
     }
 
+    @Test
+    public void testNotifyColorsOfDestroyedEngine_doesntCrash() {
+        WallpaperService service = new WallpaperService() {
+            @Override
+            public Engine onCreateEngine() {
+                return new Engine();
+            }
+        };
+        WallpaperService.Engine engine = service.onCreateEngine();
+        engine.detach();
+
+        engine.notifyColorsChanged();
+    }
 }