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>

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);
+        // 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) {
+        // Run on executor as this is a binder call from OverlayClient.
+        mExecutor.execute(() -> {
             if (mCurrentClient != client) {
                 return;
             }
 
             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() {
     }

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,7 +177,6 @@ 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);
@@ -207,7 +207,6 @@ public class DreamOverlayService extends android.service.dreams.DreamOverlayServ
             return;
         }
 
-
         setCurrentStateLocked(Lifecycle.State.RESUMED);
         mStateController.setOverlayActive(true);
         final ComponentName dreamComponent = getDreamComponent();
@@ -217,14 +216,11 @@ public class DreamOverlayService extends android.service.dreams.DreamOverlayServ
 
         mDreamOverlayCallbackController.onStartDream();
         mStarted = true;
-        });
     }
 
     @Override
     public void onEndDream() {
-        mExecutor.execute(() -> {
         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);
         }
-        });
     }
 
     /**

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();
     }
@@ -296,6 +300,48 @@ public class DreamOverlayServiceTest extends SysuiTestCase {
         verify(mStateController).setEntryAnimationsFinished(false);
     }
 
+    @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();

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) {
+        TestDreamOverlayService(Monitor monitor, Executor executor) {
-            super();
+            super(executor);
             mMonitor = monitor;
         }
 
@@ -117,14 +125,64 @@ public class DreamOverlayServiceTest {
         MockitoAnnotations.initMocks(this);
     }
 
+    /**
+     * 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);