Merge "Fix issue with 0 duration animations"

services/core/java/com/android/server/wm/DisplayContent.java

@@ -1923,6 +1923,8 @@ class DisplayContent extends WindowContainer<DisplayContent.DisplayChildWindowCo
                     mService.unregisterPointerEventListener(mService.mMousePositionTracker);
                 }
             }
+            mService.mAnimator.removeDisplayLocked(mDisplayId);
+
             // The pending transaction won't be applied so we should
             // just clean up any surfaces pending destruction.
             onPendingTransactionApplied();

services/core/java/com/android/server/wm/SurfaceAnimator.java

@@ -72,23 +72,29 @@ class SurfaceAnimator {
                     target.mInnerAnimationFinishedCallback.onAnimationFinished(anim);
                     return;
                 }
-                if (anim != mAnimation) {
-                    // Callback was from another animation - ignore.
-                    return;
-                }
 
-                final Transaction t = new Transaction();
-                SurfaceControl.openTransaction();
-                try {
-                    reset(t, true /* destroyLeash */);
-                    animationFinishedCallback.run();
-                } finally {
-                    // TODO: This should use pendingTransaction eventually, but right now things
-                    // happening on the animation finished callback are happening on the global
-                    // transaction.
-                    SurfaceControl.mergeToGlobalTransaction(t);
-                    SurfaceControl.closeTransaction();
-                }
+                // TODO: This should use pendingTransaction eventually, but right now things
+                // happening on the animation finished callback are happening on the global
+                // transaction.
+                // For now we need to run this after it's guaranteed that the transaction that
+                // reparents the surface onto the leash is executed already. Otherwise this may be
+                // executed first, leading to surface loss, as the reparent operations wouldn't
+                // be in order.
+                mService.mAnimator.addAfterPrepareSurfacesRunnable(() -> {
+                    if (anim != mAnimation) {
+                        // Callback was from another animation - ignore.
+                        return;
+                    }
+                    final Transaction t = new Transaction();
+                    SurfaceControl.openTransaction();
+                    try {
+                        reset(t, true /* destroyLeash */);
+                        animationFinishedCallback.run();
+                    } finally {
+                        SurfaceControl.mergeToGlobalTransaction(t);
+                        SurfaceControl.closeTransaction();
+                    }
+                });
             }
         };
     }

services/core/java/com/android/server/wm/WindowAnimator.java

@@ -35,6 +35,7 @@ import com.android.server.AnimationThread;
 import com.android.server.policy.WindowManagerPolicy;
 
 import java.io.PrintWriter;
+import java.util.ArrayList;
 
 /**
  * Singleton class that carries out the animations and Surface operations in a separate task
@@ -87,6 +88,12 @@ public class WindowAnimator {
      */
     private boolean mAnimationFrameCallbackScheduled;
 
+    /**
+     * A list of runnable that need to be run after {@link WindowContainer#prepareSurfaces} is
+     * executed and the corresponding transaction is closed and applied.
+     */
+    private final ArrayList<Runnable> mAfterPrepareSurfacesRunnables = new ArrayList<>();
+
     WindowAnimator(final WindowManagerService service) {
         mService = service;
         mContext = service.mContext;
@@ -262,6 +269,7 @@ public class WindowAnimator {
             mService.destroyPreservedSurfaceLocked();
             mService.mWindowPlacerLocked.destroyPendingSurfaces();
 
+            executeAfterPrepareSurfacesRunnables();
 
             if (DEBUG_WINDOW_TRACE) {
                 Slog.i(TAG, "!!! animate: exit mAnimating=" + mAnimating
@@ -425,4 +433,23 @@ public class WindowAnimator {
     void orAnimating(boolean animating) {
         mAnimating |= animating;
     }
+
+    /**
+     * Adds a runnable to be executed after {@link WindowContainer#prepareSurfaces} is called and
+     * the corresponding transaction is closed and applied.
+     */
+    void addAfterPrepareSurfacesRunnable(Runnable r) {
+        mAfterPrepareSurfacesRunnables.add(r);
+        scheduleAnimation();
+    }
+
+    private void executeAfterPrepareSurfacesRunnables() {
+
+        // Traverse in order they were added.
+        final int size = mAfterPrepareSurfacesRunnables.size();
+        for (int i = 0; i < size; i++) {
+            mAfterPrepareSurfacesRunnables.get(i).run();
+        }
+        mAfterPrepareSurfacesRunnables.clear();
+    }
 }

services/tests/servicestests/src/com/android/server/wm/SurfaceAnimatorTest.java

@@ -45,6 +45,7 @@ import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
 import java.util.ArrayList;
+import java.util.concurrent.CountDownLatch;
 
 /**
  * Test class for {@link SurfaceAnimatorTest}.
@@ -82,6 +83,7 @@ public class SurfaceAnimatorTest extends WindowTestsBase {
         verify(mSpec).startAnimation(any(), any(), callbackCaptor.capture());
 
         callbackCaptor.getValue().onAnimationFinished(mSpec);
+        waitUntilPrepareSurfaces();
         assertNotAnimating(mAnimatable);
         assertTrue(mAnimatable.mFinishedCallbackCalled);
         assertTrue(mAnimatable.mPendingDestroySurfaces.contains(mAnimatable.mLeash));
@@ -104,11 +106,13 @@ public class SurfaceAnimatorTest extends WindowTestsBase {
 
         // First animation was finished, but this shouldn't cancel the second animation
         callbackCaptor.getValue().onAnimationFinished(mSpec);
+        waitUntilPrepareSurfaces();
         assertTrue(mAnimatable.mSurfaceAnimator.isAnimating());
 
         // Second animation was finished
         verify(mSpec2).startAnimation(any(), any(), callbackCaptor.capture());
         callbackCaptor.getValue().onAnimationFinished(mSpec2);
+        waitUntilPrepareSurfaces();
         assertNotAnimating(mAnimatable);
         assertTrue(mAnimatable.mFinishedCallbackCalled);
     }
@@ -160,6 +164,7 @@ public class SurfaceAnimatorTest extends WindowTestsBase {
         assertEquals(leash, mAnimatable2.mSurfaceAnimator.mLeash);
         assertFalse(mAnimatable.mPendingDestroySurfaces.contains(leash));
         callbackCaptor.getValue().onAnimationFinished(mSpec);
+        waitUntilPrepareSurfaces();
         assertNotAnimating(mAnimatable2);
         assertTrue(mAnimatable2.mFinishedCallbackCalled);
         assertTrue(mAnimatable2.mPendingDestroySurfaces.contains(leash));
@@ -175,6 +180,14 @@ public class SurfaceAnimatorTest extends WindowTestsBase {
         assertNull(animatable.mSurfaceAnimator.getAnimation());
     }
 
+    private void waitUntilPrepareSurfaces() throws Exception {
+        final CountDownLatch latch = new CountDownLatch(1);
+        synchronized (sWm.mWindowMap) {
+            sWm.mAnimator.addAfterPrepareSurfacesRunnable(latch::countDown);
+        }
+        latch.await();
+    }
+
     private class MyAnimatable implements Animatable {
 
         final SurfaceControl mParent;

services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java

@@ -103,6 +103,7 @@ class WindowTestsBase {
 
         context.getDisplay().getDisplayInfo(mDisplayInfo);
         mDisplayContent = createNewDisplay();
+        sWm.mAnimator.mInitialized = true;
         sWm.mDisplayEnabled = true;
         sWm.mDisplayReady = true;