From 6f9ac2b1703c62f53cd5c504e1ac7a3371b4fbe9 Mon Sep 17 00:00:00 2001
From: Lucas Dupin <dupin@google.com>
Date: Fri, 14 Jul 2023 11:32:21 -0700
Subject: [PATCH] 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)
---
 .../service/wallpaper/WallpaperService.java   | 19 ++++++++++++++++++-
 .../src/android/service/wallpaper/OWNERS      |  4 ++++
 .../wallpaper/WallpaperServiceTest.java       | 13 +++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 tests/Internal/src/android/service/wallpaper/OWNERS

diff --git a/core/java/android/service/wallpaper/WallpaperService.java b/core/java/android/service/wallpaper/WallpaperService.java
index dbc1be1415715..d9ac4850e9244 100644
--- a/core/java/android/service/wallpaper/WallpaperService.java
+++ b/core/java/android/service/wallpaper/WallpaperService.java
@@ -868,6 +868,11 @@ public abstract class WallpaperService extends Service {
          * 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 @@ public abstract class WallpaperService extends Service {
             }
         }
 
-        void detach() {
+        /**
+         * @hide
+         */
+        @VisibleForTesting
+        public void detach() {
             if (mDestroyed) {
                 return;
             }
@@ -2442,6 +2451,14 @@ public abstract class WallpaperService extends Service {
         }
 
         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");
diff --git a/tests/Internal/src/android/service/wallpaper/OWNERS b/tests/Internal/src/android/service/wallpaper/OWNERS
new file mode 100644
index 0000000000000..5a26d0e1f62b5
--- /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
diff --git a/tests/Internal/src/android/service/wallpaper/WallpaperServiceTest.java b/tests/Internal/src/android/service/wallpaper/WallpaperServiceTest.java
index 153ca79e346bb..0c5e8d4811318 100644
--- a/tests/Internal/src/android/service/wallpaper/WallpaperServiceTest.java
+++ b/tests/Internal/src/android/service/wallpaper/WallpaperServiceTest.java
@@ -85,4 +85,17 @@ public class WallpaperServiceTest {
         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();
+    }
 }