Improve DreamOverlayService thread safety.

Local states like the lifecycle state should and are now only accessed from the main thread. Also add "locked" to names of local functions that should only be called synchronously.

Test: atest DreamOverlayServiceTest
Bug: 255203719
Change-Id: Iaa7f952e84b5c4c33f48238baeac0a4124233e2c

2 files changed, 44 insertions, 29 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayService.java b/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayService.java
index d1b73685a3f3..1e539098261b 100644
--- a/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayService.java
+++ b/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayService.java
@@ -90,13 +90,15 @@ public class DreamOverlayService extends android.service.dreams.DreamOverlayServ
             new KeyguardUpdateMonitorCallback() {
                 @Override
                 public void onShadeExpandedChanged(boolean expanded) {
-                    if (mLifecycleRegistry.getCurrentState() != Lifecycle.State.RESUMED
-                            && mLifecycleRegistry.getCurrentState() != Lifecycle.State.STARTED) {
-                        return;
-                    }
-
-                    mLifecycleRegistry.setCurrentState(
-                            expanded ? Lifecycle.State.STARTED : Lifecycle.State.RESUMED);
+                    mExecutor.execute(() -> {
+                        if (getCurrentStateLocked() != Lifecycle.State.RESUMED
+                                && getCurrentStateLocked() != Lifecycle.State.STARTED) {
+                            return;
+                        }
+
+                        setCurrentStateLocked(
+                                expanded ? Lifecycle.State.STARTED : Lifecycle.State.RESUMED);
+                    });
                 }
             };
 
@@ -146,29 +148,30 @@ public class DreamOverlayService extends android.service.dreams.DreamOverlayServ
                 () -> mExecutor.execute(DreamOverlayService.this::requestExit);
         mDreamOverlayComponent = dreamOverlayComponentFactory.create(viewModelStore, host);
         mLifecycleRegistry = mDreamOverlayComponent.getLifecycleRegistry();
-        setCurrentState(Lifecycle.State.CREATED);
-    }
 
-    private void setCurrentState(Lifecycle.State state) {
-        mExecutor.execute(() -> mLifecycleRegistry.setCurrentState(state));
+        mExecutor.execute(() -> setCurrentStateLocked(Lifecycle.State.CREATED));
     }
 
     @Override
     public void onDestroy() {
         mKeyguardUpdateMonitor.removeCallback(mKeyguardCallback);
-        setCurrentState(Lifecycle.State.DESTROYED);
 
-        resetCurrentDreamOverlay();
+        mExecutor.execute(() -> {
+            setCurrentStateLocked(Lifecycle.State.DESTROYED);
+
+            resetCurrentDreamOverlayLocked();
+
+            mDestroyed = true;
+        });
 
-        mDestroyed = true;
         super.onDestroy();
     }
 
     @Override
     public void onStartDream(@NonNull WindowManager.LayoutParams layoutParams) {
-        setCurrentState(Lifecycle.State.STARTED);
-
         mExecutor.execute(() -> {
+            setCurrentStateLocked(Lifecycle.State.STARTED);
+
             mUiEventLogger.log(DreamOverlayEvent.DREAM_OVERLAY_ENTER_START);
 
             if (mDestroyed) {
@@ -181,7 +184,7 @@ public class DreamOverlayService extends android.service.dreams.DreamOverlayServ
                 // Reset the current dream overlay before starting a new one. This can happen
                 // when two dreams overlap (briefly, for a smoother dream transition) and both
                 // dreams are bound to the dream overlay service.
-                resetCurrentDreamOverlay();
+                resetCurrentDreamOverlayLocked();
             }
 
             mDreamOverlayContainerViewController =
@@ -191,7 +194,7 @@ public class DreamOverlayService extends android.service.dreams.DreamOverlayServ
 
             mStateController.setShouldShowComplications(shouldShowComplications());
             addOverlayWindowLocked(layoutParams);
-            setCurrentState(Lifecycle.State.RESUMED);
+            setCurrentStateLocked(Lifecycle.State.RESUMED);
             mStateController.setOverlayActive(true);
             final ComponentName dreamComponent = getDreamComponent();
             mStateController.setLowLightActive(
@@ -202,6 +205,14 @@ public class DreamOverlayService extends android.service.dreams.DreamOverlayServ
         });
     }
 
+    private Lifecycle.State getCurrentStateLocked() {
+        return mLifecycleRegistry.getCurrentState();
+    }
+
+    private void setCurrentStateLocked(Lifecycle.State state) {
+        mLifecycleRegistry.setCurrentState(state);
+    }
+
     /**
      * Inserts {@link Window} to host the dream overlay into the dream's parent window. Must be
      * called from the main executing thread. The window attributes closely mirror those that are
@@ -231,13 +242,13 @@ public class DreamOverlayService extends android.service.dreams.DreamOverlayServ
         // Make extra sure the container view has been removed from its old parent (otherwise we
         // risk an IllegalStateException in some cases when setting the container view as the
         // window's content view and the container view hasn't been properly removed previously).
-        removeContainerViewFromParent();
+        removeContainerViewFromParentLocked();
         mWindow.setContentView(mDreamOverlayContainerViewController.getContainerView());
 
         mWindowManager.addView(mWindow.getDecorView(), mWindow.getAttributes());
     }
 
-    private void removeContainerViewFromParent() {
+    private void removeContainerViewFromParentLocked() {
         View containerView = mDreamOverlayContainerViewController.getContainerView();
         if (containerView == null) {
             return;
@@ -250,7 +261,7 @@ public class DreamOverlayService extends android.service.dreams.DreamOverlayServ
         parentView.removeView(containerView);
     }
 
-    private void resetCurrentDreamOverlay() {
+    private void resetCurrentDreamOverlayLocked() {
         if (mStarted && mWindow != null) {
             mWindowManager.removeView(mWindow.getDecorView());
         }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/dreams/DreamOverlayServiceTest.java b/packages/SystemUI/tests/src/com/android/systemui/dreams/DreamOverlayServiceTest.java
index f370be190761..2cdbef7ffd80 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/dreams/DreamOverlayServiceTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/dreams/DreamOverlayServiceTest.java
@@ -273,24 +273,28 @@ public class DreamOverlayServiceTest extends SysuiTestCase {
 
     @Test
     public void testDecorViewNotAddedToWindowAfterDestroy() throws Exception {
-        when(mDreamOverlayContainerView.getParent())
-                .thenReturn(mDreamOverlayContainerViewParent)
-                .thenReturn(null);
-
         final IBinder proxy = mService.onBind(new Intent());
         final IDreamOverlay overlay = IDreamOverlay.Stub.asInterface(proxy);
 
+        // Destroy the service.
+        mService.onDestroy();
+        mMainExecutor.runAllReady();
+
         // Inform the overlay service of dream starting.
         overlay.startDream(mWindowParams, mDreamOverlayCallback, DREAM_COMPONENT,
                 false /*shouldShowComplication*/);
+        mMainExecutor.runAllReady();
 
-        // Destroy the service.
-        mService.onDestroy();
+        verify(mWindowManager, never()).addView(any(), any());
+    }
 
-        // Run executor tasks.
+    @Test
+    public void testNeverRemoveDecorViewIfNotAdded() {
+        // Service destroyed before dream started.
+        mService.onDestroy();
         mMainExecutor.runAllReady();
 
-        verify(mWindowManager, never()).addView(any(), any());
+        verify(mWindowManager, never()).removeView(any());
     }
 
     @Test