Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5b00d142 authored by Chris Li's avatar Chris Li
Browse files

Fix unfreeze on unstarted freezer 

Before, when trigger change transition and screen freeze before the
previous transition is finished, it will cancel the animation and call
to unfreeze. However, it will then unfreeze the new snapshot instead of
the old one, which cause a client crash because of missing leash.

The fix contains two changes:
1. Let SurfaceAnimator to take the snapshot from the freezer as it does
   for animation leash.
2. When an animation leash needs to be removed, check if it is the
   current animation leash (parent of the window surface) before
   reparenting the window surface back to the parent window surface.

Bug: 201622511
Bug: 196173550
Test: atest WmTests:WindowContainerTests
      #testStartChangeTransitionWhenPreviousIsNotFinished
Test: click demo app launch spit before the previous one is finished
Change-Id: I58fb05a971b49981d168d00c9fb42014b4d3cd4d
parent d54349db

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

+40 −7
Original line number Diff line number Diff line
@@ -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

+20 −23
Original line number Diff line number Diff line
@@ -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,

                @Nullable SurfaceAnimator.OnAnimationFinishedCallback animationFinishedCallback) {

        void startAnimation(SurfaceControl.Transaction t, AnimationAdapter anim, int type) {

            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

+19 −11
Original line number Diff line number Diff line
@@ -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,

                    (type, anim) -> cleanUpCallback.run(),

                    cleanUpCallback);

                    ANIMATION_TYPE_APP_TRANSITION, (type, anim) -> cleanUpCallback.run(),

                    cleanUpCallback, thumbnailAdapter);



            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

+67 −0
Original line number Diff line number Diff line
@@ -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;