Merge "Fix unfreeze on unstarted freezer" into sc-v2-dev

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

@@ -57,8 +57,11 @@ class SurfaceAnimator {
     @VisibleForTesting
     SurfaceControl mLeash;
     @VisibleForTesting
+    SurfaceFreezer.Snapshot mSnapshot;
+    @VisibleForTesting
     final Animatable mAnimatable;
-    private final OnAnimationFinishedCallback mInnerAnimationFinishedCallback;
+    @VisibleForTesting
+    final OnAnimationFinishedCallback mInnerAnimationFinishedCallback;
 
     /**
      * Static callback to run on all animations started through this SurfaceAnimator
@@ -151,12 +154,14 @@ class SurfaceAnimator {
      * @param animationFinishedCallback The callback being triggered when the animation finishes.
      * @param animationCancelledCallback The callback is triggered after the SurfaceAnimator sends a
      *                                   cancel call to the underlying AnimationAdapter.
+     * @param snapshotAnim The animation to run for the snapshot. {@code null} if there is no
+     *                     snapshot.
      */
     void startAnimation(Transaction t, AnimationAdapter anim, boolean hidden,
             @AnimationType int type,
             @Nullable OnAnimationFinishedCallback animationFinishedCallback,
             @Nullable Runnable animationCancelledCallback,
-            @Nullable SurfaceFreezer freezer) {
+            @Nullable AnimationAdapter snapshotAnim, @Nullable SurfaceFreezer freezer) {
         cancelAnimation(t, true /* restarting */, true /* forwardCancel */);
         mAnimation = anim;
         mAnimationType = type;
@@ -181,12 +186,16 @@ class SurfaceAnimator {
             return;
         }
         mAnimation.startAnimation(mLeash, t, type, mInnerAnimationFinishedCallback);
+        if (snapshotAnim != null) {
+            mSnapshot = freezer.takeSnapshotForAnimation();
+            mSnapshot.startAnimation(t, snapshotAnim, type);
+        }
     }
 
     void startAnimation(Transaction t, AnimationAdapter anim, boolean hidden,
             @AnimationType int type) {
         startAnimation(t, anim, hidden, type, null /* animationFinishedCallback */,
-                null /* animationCancelledCallback */, null /* freezer */);
+                null /* animationCancelledCallback */, null /* snapshotAnim */, null /* freezer */);
     }
 
     /**
@@ -328,6 +337,7 @@ class SurfaceAnimator {
         final OnAnimationFinishedCallback animationFinishedCallback =
                 mSurfaceAnimationFinishedCallback;
         final Runnable animationCancelledCallback = mAnimationCancelledCallback;
+        final SurfaceFreezer.Snapshot snapshot = mSnapshot;
         reset(t, false);
         if (animation != null) {
             if (!mAnimationStartDelayed && forwardCancel) {
@@ -346,10 +356,15 @@ class SurfaceAnimator {
             }
         }
 
-        if (forwardCancel && leash != null) {
+        if (forwardCancel) {
+            if (snapshot != null) {
+                snapshot.cancelAnimation(t, false /* restarting */);
+            }
+            if (leash != null) {
                 t.remove(leash);
                 mService.scheduleAnimationLocked();
             }
+        }
 
         if (!restarting) {
             mAnimationStartDelayed = false;
@@ -361,6 +376,12 @@ class SurfaceAnimator {
         mAnimation = null;
         mSurfaceAnimationFinishedCallback = null;
         mAnimationType = ANIMATION_TYPE_NONE;
+        final SurfaceFreezer.Snapshot snapshot = mSnapshot;
+        mSnapshot = null;
+        if (snapshot != null) {
+            // Reset the mSnapshot reference before calling the callback to prevent circular reset.
+            snapshot.cancelAnimation(t, !destroyLeash);
+        }
         if (mLeash == null) {
             return;
         }
@@ -377,11 +398,15 @@ class SurfaceAnimator {
         boolean scheduleAnim = false;
         final SurfaceControl surface = animatable.getSurfaceControl();
         final SurfaceControl parent = animatable.getParentSurfaceControl();
+        final SurfaceControl curAnimationLeash = animatable.getAnimationLeash();
 
         // If the surface was destroyed or the leash is invalid, we don't care to reparent it back.
         // Note that we also set this variable to true even if the parent isn't valid anymore, in
         // order to ensure onAnimationLeashLost still gets called in this case.
-        final boolean reparent = surface != null;
+        // If the animation leash is set, and it is different from the removing leash, it means the
+        // surface now has a new animation surface. We don't want to reparent for that.
+        final boolean reparent = surface != null && (curAnimationLeash == null
+                || curAnimationLeash.equals(leash));
         if (reparent) {
             if (DEBUG_ANIM) Slog.i(TAG, "Reparenting to original parent: " + parent);
             // We shouldn't really need these isValid checks but we do
@@ -607,6 +632,14 @@ class SurfaceAnimator {
          */
         void onAnimationLeashLost(Transaction t);
 
+        /**
+         * Gets the last created animation leash that has not lost yet.
+         */
+        @Nullable
+        default SurfaceControl getAnimationLeash() {
+            return null;
+        }
+
         /**
          * @return A new surface to be used for the animation leash, inserted at the correct
          *         position in the hierarchy.

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

@@ -17,7 +17,6 @@
 package com.android.server.wm;
 
 import static com.android.internal.protolog.ProtoLogGroup.WM_SHOW_TRANSACTIONS;
-import static com.android.server.wm.SurfaceAnimator.ANIMATION_TYPE_APP_TRANSITION;
 import static com.android.server.wm.SurfaceAnimator.ANIMATION_TYPE_SCREEN_ROTATION;
 
 import android.annotation.NonNull;
@@ -27,13 +26,11 @@ import android.graphics.PixelFormat;
 import android.graphics.Point;
 import android.graphics.Rect;
 import android.hardware.HardwareBuffer;
-import android.view.Surface;
 import android.view.SurfaceControl;
 
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.protolog.common.ProtoLog;
 
-import java.util.function.Supplier;
-
 /**
  * This class handles "freezing" of an Animatable. The Animatable in question should implement
  * Freezable.
@@ -54,7 +51,8 @@ class SurfaceFreezer {
 
     private final Freezable mAnimatable;
     private final WindowManagerService mWmService;
-    private SurfaceControl mLeash;
+    @VisibleForTesting
+    SurfaceControl mLeash;
     Snapshot mSnapshot = null;
     final Rect mFreezeBounds = new Rect();
 
@@ -94,7 +92,7 @@ class SurfaceFreezer {
             if (buffer == null || buffer.getWidth() <= 1 || buffer.getHeight() <= 1) {
                 return;
             }
-            mSnapshot = new Snapshot(mWmService.mSurfaceFactory, t, screenshotBuffer, mLeash);
+            mSnapshot = new Snapshot(t, screenshotBuffer, mLeash);
         }
     }
 
@@ -108,6 +106,18 @@ class SurfaceFreezer {
         return out;
     }
 
+    /**
+     * Used by {@link SurfaceAnimator}. This "transfers" the snapshot leash to be used for
+     * animation. By transferring the leash, this will no longer try to clean-up the leash when
+     * finished.
+     */
+    @Nullable
+    Snapshot takeSnapshotForAnimation() {
+        final Snapshot out = mSnapshot;
+        mSnapshot = null;
+        return out;
+    }
+
     /**
      * Clean-up the snapshot and remove leash. If the leash was taken, this just cleans-up the
      * snapshot.
@@ -115,6 +125,7 @@ class SurfaceFreezer {
     void unfreeze(SurfaceControl.Transaction t) {
         if (mSnapshot != null) {
             mSnapshot.cancelAnimation(t, false /* restarting */);
+            mSnapshot = null;
         }
         if (mLeash == null) {
             return;
@@ -163,13 +174,12 @@ class SurfaceFreezer {
     class Snapshot {
         private SurfaceControl mSurfaceControl;
         private AnimationAdapter mAnimation;
-        private SurfaceAnimator.OnAnimationFinishedCallback mFinishedCallback;
 
         /**
          * @param t Transaction to create the thumbnail in.
          * @param screenshotBuffer A thumbnail or placeholder for thumbnail to initialize with.
          */
-        Snapshot(Supplier<Surface> surfaceFactory, SurfaceControl.Transaction t,
+        Snapshot(SurfaceControl.Transaction t,
                 SurfaceControl.ScreenshotHardwareBuffer screenshotBuffer, SurfaceControl parent) {
             // We can't use a delegating constructor since we need to
             // reference this::onAnimationFinished
@@ -211,19 +221,15 @@ class SurfaceFreezer {
          *             component responsible for running the animation. It runs the animation with
          *             {@link AnimationAdapter#startAnimation} once the hierarchy with
          *             the Leash has been set up.
-         * @param animationFinishedCallback The callback being triggered when the animation
-         *                                  finishes.
          */
-        void startAnimation(SurfaceControl.Transaction t, AnimationAdapter anim, int type,
+        void startAnimation(SurfaceControl.Transaction t, AnimationAdapter anim, int type) {
-                @Nullable SurfaceAnimator.OnAnimationFinishedCallback animationFinishedCallback) {
             cancelAnimation(t, true /* restarting */);
             mAnimation = anim;
-            mFinishedCallback = animationFinishedCallback;
             if (mSurfaceControl == null) {
                 cancelAnimation(t, false /* restarting */);
                 return;
             }
-            mAnimation.startAnimation(mSurfaceControl, t, type, animationFinishedCallback);
+            mAnimation.startAnimation(mSurfaceControl, t, type, null /* finishCallback */);
         }
 
         /**
@@ -235,18 +241,9 @@ class SurfaceFreezer {
         void cancelAnimation(SurfaceControl.Transaction t, boolean restarting) {
             final SurfaceControl leash = mSurfaceControl;
             final AnimationAdapter animation = mAnimation;
-            final SurfaceAnimator.OnAnimationFinishedCallback animationFinishedCallback =
-                    mFinishedCallback;
             mAnimation = null;
-            mFinishedCallback = null;
             if (animation != null) {
                 animation.onAnimationCancelled(leash);
-                if (!restarting) {
-                    if (animationFinishedCallback != null) {
-                        animationFinishedCallback.onAnimationFinished(
-                                ANIMATION_TYPE_APP_TRANSITION, animation);
-                    }
-                }
             }
             if (!restarting) {
                 destroy(t);

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

@@ -178,6 +178,10 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
      */
     protected final SurfaceAnimator mSurfaceAnimator;
 
+    /** The parent leash added for animation. */
+    @Nullable
+    private SurfaceControl mAnimationLeash;
+
     final SurfaceFreezer mSurfaceFreezer;
     protected final WindowManagerService mWmService;
     final TransitionController mTransitionController;
@@ -2561,11 +2565,14 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
      * @param animationFinishedCallback The callback being triggered when the animation finishes.
      * @param animationCancelledCallback The callback is triggered after the SurfaceAnimator sends a
      *                                   cancel call to the underlying AnimationAdapter.
+     * @param snapshotAnim  The animation to run for the snapshot. {@code null} if there is no
+     *                      snapshot.
      */
     void startAnimation(Transaction t, AnimationAdapter anim, boolean hidden,
             @AnimationType int type,
             @Nullable OnAnimationFinishedCallback animationFinishedCallback,
-            @Nullable Runnable animationCancelledCallback) {
+            @Nullable Runnable animationCancelledCallback,
+            @Nullable AnimationAdapter snapshotAnim) {
         if (DEBUG_ANIM) {
             Slog.v(TAG, "Starting animation on " + this + ": type=" + type + ", anim=" + anim);
         }
@@ -2573,14 +2580,14 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
         // TODO: This should use isVisible() but because isVisible has a really weird meaning at
         // the moment this doesn't work for all animatable window containers.
         mSurfaceAnimator.startAnimation(t, anim, hidden, type, animationFinishedCallback,
-                animationCancelledCallback, mSurfaceFreezer);
+                animationCancelledCallback, snapshotAnim, mSurfaceFreezer);
     }
 
     void startAnimation(Transaction t, AnimationAdapter anim, boolean hidden,
             @AnimationType int type,
             @Nullable OnAnimationFinishedCallback animationFinishedCallback) {
         startAnimation(t, anim, hidden, type, animationFinishedCallback,
-                null /* adapterAnimationCancelledCallback */);
+                null /* adapterAnimationCancelledCallback */, null /* snapshotAnim */);
     }
 
     void startAnimation(Transaction t, AnimationAdapter anim, boolean hidden,
@@ -2832,17 +2839,12 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
                     ? taskDisplayArea::clearBackgroundColor : () -> {};
 
             startAnimation(getPendingTransaction(), adapter, !isVisible(),
-                    ANIMATION_TYPE_APP_TRANSITION,
+                    ANIMATION_TYPE_APP_TRANSITION, (type, anim) -> cleanUpCallback.run(),
-                    (type, anim) -> cleanUpCallback.run(),
+                    cleanUpCallback, thumbnailAdapter);
-                    cleanUpCallback);
 
             if (adapter.getShowWallpaper()) {
                 getDisplayContent().pendingLayoutChanges |= FINISH_LAYOUT_REDO_WALLPAPER;
             }
-            if (thumbnailAdapter != null) {
-                mSurfaceFreezer.mSnapshot.startAnimation(getPendingTransaction(),
-                        thumbnailAdapter, ANIMATION_TYPE_APP_TRANSITION, (type, anim) -> { });
-            }
         }
     }
 
@@ -2972,6 +2974,7 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
     @Override
     public void onAnimationLeashCreated(Transaction t, SurfaceControl leash) {
         mLastLayer = -1;
+        mAnimationLeash = leash;
         reassignLayer(t);
 
         // Leash is now responsible for position, so set our position to 0.
@@ -2981,11 +2984,16 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
     @Override
     public void onAnimationLeashLost(Transaction t) {
         mLastLayer = -1;
-        mSurfaceFreezer.unfreeze(t);
+        mAnimationLeash = null;
         reassignLayer(t);
         updateSurfacePosition(t);
     }
 
+    @Override
+    public SurfaceControl getAnimationLeash() {
+        return mAnimationLeash;
+    }
+
     private void doAnimationFinished(@AnimationType int type, AnimationAdapter anim) {
         for (int i = 0; i < mSurfaceAnimationSources.size(); ++i) {
             mSurfaceAnimationSources.valueAt(i).onAnimationFinished(type, anim);

services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java

@@ -25,6 +25,7 @@ import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED;
 import static android.view.WindowManager.LayoutParams.TYPE_BASE_APPLICATION;
 import static android.view.WindowManager.TRANSIT_CLOSE;
 import static android.view.WindowManager.TRANSIT_OLD_TASK_CLOSE;
+import static android.view.WindowManager.TRANSIT_OLD_TASK_FRAGMENT_CHANGE;
 import static android.view.WindowManager.TRANSIT_OLD_TASK_OPEN;
 import static android.view.WindowManager.TRANSIT_OPEN;
 import static android.window.DisplayAreaOrganizer.FEATURE_DEFAULT_TASK_CONTAINER;
@@ -52,6 +53,7 @@ import static com.android.server.wm.WindowContainer.POSITION_TOP;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -1106,6 +1108,71 @@ public class WindowContainerTests extends WindowTestsBase {
         verify(surfaceAnimator, never()).setRelativeLayer(any(), any(), anyInt());
     }
 
+    @Test
+    public void testStartChangeTransitionWhenPreviousIsNotFinished() {
+        final WindowContainer container = createTaskFragmentWithParentTask(
+                createTask(mDisplayContent), false);
+        container.mSurfaceControl = mock(SurfaceControl.class);
+        final SurfaceAnimator surfaceAnimator = container.mSurfaceAnimator;
+        final SurfaceFreezer surfaceFreezer = container.mSurfaceFreezer;
+        final SurfaceControl.Transaction t = mock(SurfaceControl.Transaction.class);
+        spyOn(container);
+        spyOn(surfaceAnimator);
+        spyOn(surfaceFreezer);
+        doReturn(t).when(container).getPendingTransaction();
+        doReturn(t).when(container).getSyncTransaction();
+
+        // Leash and snapshot created for change transition.
+        container.initializeChangeTransition(new Rect(0, 0, 1000, 2000));
+        // Can't really take a snapshot, manually set one.
+        surfaceFreezer.mSnapshot = mock(SurfaceFreezer.Snapshot.class);
+
+        assertNotNull(surfaceFreezer.mLeash);
+        assertEquals(surfaceFreezer.mLeash, container.getAnimationLeash());
+
+        // Start animation: surfaceAnimator take over the leash and snapshot from surfaceFreezer.
+        container.applyAnimationUnchecked(null /* lp */, true /* enter */,
+                TRANSIT_OLD_TASK_FRAGMENT_CHANGE, false /* isVoiceInteraction */,
+                null /* sources */);
+
+        assertNull(surfaceFreezer.mLeash);
+        assertNull(surfaceFreezer.mSnapshot);
+        assertNotNull(surfaceAnimator.mLeash);
+        assertNotNull(surfaceAnimator.mSnapshot);
+        final SurfaceControl prevLeash = surfaceAnimator.mLeash;
+        final SurfaceFreezer.Snapshot prevSnapshot = surfaceAnimator.mSnapshot;
+
+        // Prepare another change transition.
+        container.initializeChangeTransition(new Rect(0, 0, 1000, 2000));
+        surfaceFreezer.mSnapshot = mock(SurfaceFreezer.Snapshot.class);
+
+        assertNotNull(surfaceFreezer.mLeash);
+        assertEquals(surfaceFreezer.mLeash, container.getAnimationLeash());
+        assertNotEquals(prevLeash, container.getAnimationLeash());
+
+        // Start another animation before the previous one is finished, it should reset the previous
+        // one, but not change the current one.
+        container.applyAnimationUnchecked(null /* lp */, true /* enter */,
+                TRANSIT_OLD_TASK_FRAGMENT_CHANGE, false /* isVoiceInteraction */,
+                null /* sources */);
+
+        verify(container, never()).onAnimationLeashLost(any());
+        verify(surfaceFreezer, never()).unfreeze(any());
+        assertNotNull(surfaceAnimator.mLeash);
+        assertNotNull(surfaceAnimator.mSnapshot);
+        assertEquals(surfaceAnimator.mLeash, container.getAnimationLeash());
+        assertNotEquals(prevLeash, surfaceAnimator.mLeash);
+        assertNotEquals(prevSnapshot, surfaceAnimator.mSnapshot);
+
+        // Clean up after animation finished.
+        surfaceAnimator.mInnerAnimationFinishedCallback.onAnimationFinished(
+                ANIMATION_TYPE_APP_TRANSITION, surfaceAnimator.getAnimation());
+
+        verify(container).onAnimationLeashLost(any());
+        assertNull(surfaceAnimator.mLeash);
+        assertNull(surfaceAnimator.mSnapshot);
+    }
+
     /* Used so we can gain access to some protected members of the {@link WindowContainer} class */
     private static class TestWindowContainer extends WindowContainer<TestWindowContainer> {
         private final int mLayer;