Merge "Ensure ordering of overlay callbacks" into tm-qpr-dev am: ced571cb63

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/21896080

Change-Id: Ic53541c3cffe6c137d4d2fdd2a0d1576bd17fc08
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

4 files changed, 199 insertions, 55 deletions

diff --git a/core/java/android/service/dreams/DreamOverlayService.java b/core/java/android/service/dreams/DreamOverlayService.java
index 6e4535b7218a..5469916bea4e 100644
--- a/core/java/android/service/dreams/DreamOverlayService.java
+++ b/core/java/android/service/dreams/DreamOverlayService.java
@@ -27,6 +27,8 @@ import android.os.RemoteException;
 import android.util.Log;
 import android.view.WindowManager;
 
+import java.util.concurrent.Executor;
+
 
 /**
  * Basic implementation of for {@link IDreamOverlay} for testing.
@@ -40,6 +42,12 @@ public abstract class DreamOverlayService extends Service {
     // The last client that started dreaming and hasn't ended
     private OverlayClient mCurrentClient;
 
+    /**
+     * Executor used to run callbacks that subclasses will implement. Any calls coming over Binder
+     * from {@link OverlayClient} should perform the work they need to do on this executor.
+     */
+    private Executor mExecutor;
+
     // An {@link IDreamOverlayClient} implementation that identifies itself when forwarding
     // requests to the {@link DreamOverlayService}
     private static class OverlayClient extends IDreamOverlayClient.Stub {
@@ -61,8 +69,6 @@ public abstract class DreamOverlayService extends Service {
             mService.startDream(this, params);
         }
 
-
-
         @Override
         public void wakeUp() {
             mService.wakeUp(this, () -> {
@@ -97,12 +103,20 @@ public abstract class DreamOverlayService extends Service {
     }
 
     private void startDream(OverlayClient client, WindowManager.LayoutParams params) {
-        endDream(mCurrentClient);
-        mCurrentClient = client;
-        onStartDream(params);
+        // Run on executor as this is a binder call from OverlayClient.
+        mExecutor.execute(() -> {
+            endDreamInternal(mCurrentClient);
+            mCurrentClient = client;
+            onStartDream(params);
+        });
     }
 
     private void endDream(OverlayClient client) {
+        // Run on executor as this is a binder call from OverlayClient.
+        mExecutor.execute(() -> endDreamInternal(client));
+    }
+
+    private void endDreamInternal(OverlayClient client) {
         if (client == null || client != mCurrentClient) {
             return;
         }
@@ -112,11 +126,14 @@ public abstract class DreamOverlayService extends Service {
     }
 
     private void wakeUp(OverlayClient client, Runnable callback) {
-        if (mCurrentClient != client) {
-            return;
-        }
+        // Run on executor as this is a binder call from OverlayClient.
+        mExecutor.execute(() -> {
+            if (mCurrentClient != client) {
+                return;
+            }
 
-        onWakeUp(callback);
+            onWakeUp(callback);
+        });
     }
 
     private IDreamOverlay mDreamOverlay = new IDreamOverlay.Stub() {
@@ -134,6 +151,25 @@ public abstract class DreamOverlayService extends Service {
     public DreamOverlayService() {
     }
 
+    /**
+     * This constructor allows providing an executor to run callbacks on.
+     *
+     * @hide
+     */
+    public DreamOverlayService(@NonNull Executor executor) {
+        mExecutor = executor;
+    }
+
+    @Override
+    public void onCreate() {
+        super.onCreate();
+        if (mExecutor == null) {
+            // If no executor was provided, use the main executor. onCreate is the earliest time
+            // getMainExecutor is available.
+            mExecutor = getMainExecutor();
+        }
+    }
+
     @Nullable
     @Override
     public final IBinder onBind(@NonNull Intent intent) {
@@ -143,6 +179,10 @@ public abstract class DreamOverlayService extends Service {
     /**
      * This method is overridden by implementations to handle when the dream has started and the
      * window is ready to be interacted with.
+     *
+     * This callback will be run on the {@link Executor} provided in the constructor if provided, or
+     * on the main executor if none was provided.
+     *
      * @param layoutParams The {@link android.view.WindowManager.LayoutParams} associated with the
      *                     dream window.
      */
@@ -153,6 +193,9 @@ public abstract class DreamOverlayService extends Service {
      * to wakeup. This allows any overlay animations to run. By default, the method will invoke
      * the callback immediately.
      *
+     * This callback will be run on the {@link Executor} provided in the constructor if provided, or
+     * on the main executor if none was provided.
+     *
      * @param onCompleteCallback The callback to trigger to notify the dream service that the
      *                           overlay has completed waking up.
      * @hide
@@ -164,6 +207,9 @@ public abstract class DreamOverlayService extends Service {
     /**
      * This method is overridden by implementations to handle when the dream has ended. There may
      * be earlier signals leading up to this step, such as @{@link #onWakeUp(Runnable)}.
+     *
+     * This callback will be run on the {@link Executor} provided in the constructor if provided, or
+     * on the main executor if none was provided.
      */
     public void onEndDream() {
     }
diff --git a/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayService.java b/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayService.java
index 854323f51555..a0fef758da51 100644
--- a/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayService.java
+++ b/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayService.java
@@ -139,6 +139,7 @@ public class DreamOverlayService extends android.service.dreams.DreamOverlayServ
                     ComponentName lowLightDreamComponent,
             DreamOverlayCallbackController dreamOverlayCallbackController,
             @Named(DREAM_OVERLAY_WINDOW_TITLE) String windowTitle) {
+        super(executor);
         mContext = context;
         mExecutor = executor;
         mWindowManager = windowManager;
@@ -176,55 +177,50 @@ public class DreamOverlayService extends android.service.dreams.DreamOverlayServ
 
     @Override
     public void onStartDream(@NonNull WindowManager.LayoutParams layoutParams) {
-        mExecutor.execute(() -> {
-            setCurrentStateLocked(Lifecycle.State.STARTED);
-
-            mUiEventLogger.log(DreamOverlayEvent.DREAM_OVERLAY_ENTER_START);
+        setCurrentStateLocked(Lifecycle.State.STARTED);
 
-            if (mDestroyed) {
-                // The task could still be executed after the service has been destroyed. Bail if
-                // that is the case.
-                return;
-            }
+        mUiEventLogger.log(DreamOverlayEvent.DREAM_OVERLAY_ENTER_START);
 
-            if (mStarted) {
-                // 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.
-                resetCurrentDreamOverlayLocked();
-            }
+        if (mDestroyed) {
+            // The task could still be executed after the service has been destroyed. Bail if
+            // that is the case.
+            return;
+        }
 
-            mDreamOverlayContainerViewController =
-                    mDreamOverlayComponent.getDreamOverlayContainerViewController();
-            mDreamOverlayTouchMonitor = mDreamOverlayComponent.getDreamOverlayTouchMonitor();
-            mDreamOverlayTouchMonitor.init();
+        if (mStarted) {
+            // 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.
+            resetCurrentDreamOverlayLocked();
+        }
 
-            mStateController.setShouldShowComplications(shouldShowComplications());
+        mDreamOverlayContainerViewController =
+                mDreamOverlayComponent.getDreamOverlayContainerViewController();
+        mDreamOverlayTouchMonitor = mDreamOverlayComponent.getDreamOverlayTouchMonitor();
+        mDreamOverlayTouchMonitor.init();
 
-            // If we are not able to add the overlay window, reset the overlay.
-            if (!addOverlayWindowLocked(layoutParams)) {
-                resetCurrentDreamOverlayLocked();
-                return;
-            }
+        mStateController.setShouldShowComplications(shouldShowComplications());
 
+        // If we are not able to add the overlay window, reset the overlay.
+        if (!addOverlayWindowLocked(layoutParams)) {
+            resetCurrentDreamOverlayLocked();
+            return;
+        }
 
-            setCurrentStateLocked(Lifecycle.State.RESUMED);
-            mStateController.setOverlayActive(true);
-            final ComponentName dreamComponent = getDreamComponent();
-            mStateController.setLowLightActive(
-                    dreamComponent != null && dreamComponent.equals(mLowLightDreamComponent));
-            mUiEventLogger.log(DreamOverlayEvent.DREAM_OVERLAY_COMPLETE_START);
+        setCurrentStateLocked(Lifecycle.State.RESUMED);
+        mStateController.setOverlayActive(true);
+        final ComponentName dreamComponent = getDreamComponent();
+        mStateController.setLowLightActive(
+                dreamComponent != null && dreamComponent.equals(mLowLightDreamComponent));
+        mUiEventLogger.log(DreamOverlayEvent.DREAM_OVERLAY_COMPLETE_START);
 
-            mDreamOverlayCallbackController.onStartDream();
-            mStarted = true;
-        });
+        mDreamOverlayCallbackController.onStartDream();
+        mStarted = true;
     }
 
     @Override
     public void onEndDream() {
-        mExecutor.execute(() -> {
-            resetCurrentDreamOverlayLocked();
-        });
+        resetCurrentDreamOverlayLocked();
     }
 
     private Lifecycle.State getCurrentStateLocked() {
@@ -237,12 +233,10 @@ public class DreamOverlayService extends android.service.dreams.DreamOverlayServ
 
     @Override
     public void onWakeUp(@NonNull Runnable onCompletedCallback) {
-        mExecutor.execute(() -> {
-            if (mDreamOverlayContainerViewController != null) {
-                mDreamOverlayCallbackController.onWakeUp();
-                mDreamOverlayContainerViewController.wakeUp(onCompletedCallback, mExecutor);
-            }
-        });
+        if (mDreamOverlayContainerViewController != null) {
+            mDreamOverlayCallbackController.onWakeUp();
+            mDreamOverlayContainerViewController.wakeUp(onCompletedCallback, mExecutor);
+        }
     }
 
     /**
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 de7003366469..6443621a67f4 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/dreams/DreamOverlayServiceTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/dreams/DreamOverlayServiceTest.java
@@ -20,7 +20,9 @@ import static com.google.common.truth.Truth.assertThat;
 
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.clearInvocations;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -60,6 +62,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Captor;
+import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
@@ -254,6 +257,7 @@ public class DreamOverlayServiceTest extends SysuiTestCase {
         // Inform the overlay service of dream starting.
         client.startDream(mWindowParams, mDreamOverlayCallback, DREAM_COMPONENT,
                 true /*shouldShowComplication*/);
+        mMainExecutor.runAllReady();
 
         assertThat(mService.shouldShowComplications()).isTrue();
     }
@@ -297,6 +301,48 @@ public class DreamOverlayServiceTest extends SysuiTestCase {
     }
 
     @Test
+    public void testImmediateEndDream() throws Exception {
+        final IDreamOverlayClient client = getClient();
+
+        // Start the dream, but don't execute any Runnables put on the executor yet. We delay
+        // executing Runnables as the timing isn't guaranteed and we want to verify that the overlay
+        // starts and finishes in the proper order even if Runnables are delayed.
+        client.startDream(mWindowParams, mDreamOverlayCallback, DREAM_COMPONENT,
+                false /*shouldShowComplication*/);
+        // Immediately end the dream.
+        client.endDream();
+        // Run any scheduled Runnables.
+        mMainExecutor.runAllReady();
+
+        // The overlay starts then finishes.
+        InOrder inOrder = inOrder(mWindowManager);
+        inOrder.verify(mWindowManager).addView(mViewCaptor.capture(), any());
+        inOrder.verify(mWindowManager).removeView(mViewCaptor.getValue());
+    }
+
+    @Test
+    public void testEndDreamDuringStartDream() throws Exception {
+        final IDreamOverlayClient client = getClient();
+
+        // Schedule the endDream call in the middle of the startDream implementation, as any
+        // ordering is possible.
+        doAnswer(invocation -> {
+            client.endDream();
+            return null;
+        }).when(mStateController).setOverlayActive(true);
+
+        // Start the dream.
+        client.startDream(mWindowParams, mDreamOverlayCallback, DREAM_COMPONENT,
+                false /*shouldShowComplication*/);
+        mMainExecutor.runAllReady();
+
+        // The overlay starts then finishes.
+        InOrder inOrder = inOrder(mWindowManager);
+        inOrder.verify(mWindowManager).addView(mViewCaptor.capture(), any());
+        inOrder.verify(mWindowManager).removeView(mViewCaptor.getValue());
+    }
+
+    @Test
     public void testDestroy() throws RemoteException {
         final IDreamOverlayClient client = getClient();
 
diff --git a/services/tests/servicestests/src/com/android/server/dreams/DreamOverlayServiceTest.java b/services/tests/servicestests/src/com/android/server/dreams/DreamOverlayServiceTest.java
index 6c73f716493c..851d8f94d2c0 100644
--- a/services/tests/servicestests/src/com/android/server/dreams/DreamOverlayServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/dreams/DreamOverlayServiceTest.java
@@ -18,6 +18,8 @@ package com.android.server.dreams;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 
@@ -39,10 +41,13 @@ import androidx.test.runner.AndroidJUnit4;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 
+import java.util.concurrent.Executor;
+
 /**
  * A collection of tests to exercise {@link DreamOverlayService}.
  */
@@ -60,6 +65,9 @@ public class DreamOverlayServiceTest {
     @Mock
     IDreamOverlayCallback mOverlayCallback;
 
+    @Mock
+    Executor mExecutor;
+
     /**
      * {@link TestDreamOverlayService} is a simple {@link DreamOverlayService} implementation for
      * tracking interactions across {@link IDreamOverlay} binder interface. The service reports
@@ -78,8 +86,8 @@ public class DreamOverlayServiceTest {
 
         private final Monitor mMonitor;
 
-        TestDreamOverlayService(Monitor monitor) {
-            super();
+        TestDreamOverlayService(Monitor monitor, Executor executor) {
+            super(executor);
             mMonitor = monitor;
         }
 
@@ -118,13 +126,63 @@ public class DreamOverlayServiceTest {
     }
 
     /**
+     * Verifies that callbacks for subclasses are run on the provided executor.
+     */
+    @Test
+    public void testCallbacksRunOnExecutor() throws RemoteException {
+        final TestDreamOverlayService.Monitor monitor = Mockito.mock(
+                TestDreamOverlayService.Monitor.class);
+        final TestDreamOverlayService service = new TestDreamOverlayService(monitor, mExecutor);
+        final IBinder binder = service.onBind(new Intent());
+        final IDreamOverlay overlay = IDreamOverlay.Stub.asInterface(binder);
+
+        final IDreamOverlayClient client = getClient(overlay);
+
+        // Start the dream.
+        client.startDream(mLayoutParams, mOverlayCallback,
+                FIRST_DREAM_COMPONENT.flattenToString(), false);
+
+        // The callback should not have run yet.
+        verify(monitor, never()).onStartDream();
+
+        // Run the Runnable sent to the executor.
+        ArgumentCaptor<Runnable> mRunnableCaptor = ArgumentCaptor.forClass(Runnable.class);
+        verify(mExecutor).execute(mRunnableCaptor.capture());
+        mRunnableCaptor.getValue().run();
+
+        // Callback is run.
+        verify(monitor).onStartDream();
+
+        // Verify onWakeUp is run on the executor.
+        client.wakeUp();
+        verify(monitor, never()).onWakeUp();
+        mRunnableCaptor = ArgumentCaptor.forClass(Runnable.class);
+        verify(mExecutor).execute(mRunnableCaptor.capture());
+        mRunnableCaptor.getValue().run();
+        verify(monitor).onWakeUp();
+
+        // Verify onEndDream is run on the executor.
+        client.endDream();
+        verify(monitor, never()).onEndDream();
+        mRunnableCaptor = ArgumentCaptor.forClass(Runnable.class);
+        verify(mExecutor).execute(mRunnableCaptor.capture());
+        mRunnableCaptor.getValue().run();
+        verify(monitor).onEndDream();
+    }
+
+    /**
      * Verifies that only the currently started dream is able to affect the overlay.
      */
     @Test
     public void testOverlayClientInteraction() throws RemoteException {
+        doAnswer(invocation -> {
+            ((Runnable) invocation.getArgument(0)).run();
+            return null;
+        }).when(mExecutor).execute(any());
+
         final TestDreamOverlayService.Monitor monitor = Mockito.mock(
                 TestDreamOverlayService.Monitor.class);
-        final TestDreamOverlayService service = new TestDreamOverlayService(monitor);
+        final TestDreamOverlayService service = new TestDreamOverlayService(monitor, mExecutor);
         final IBinder binder = service.onBind(new Intent());
         final IDreamOverlay overlay = IDreamOverlay.Stub.asInterface(binder);