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

Commit f6f34944 authored by lumark's avatar lumark
Browse files

Fix recent task screenshot may not cleanup in some cases

In previous implementation, TaskScreenshotAnimatable's surface
relies on SurfaceAnimator#onAnimationLeashDestroyed callback.

As the definition of this callback method is called when leash is being
destroyed, and the surface was reparented back to the original parent,
which may not be reliable for recent task screenshot use case because task
screenhot surface may fail to reparent back if the parent is null, and
the onAnimationLeashDestroyed callback won't be called.

To fix this, we modify the reparent check to exclude parent surface check.
This is because the screenshot animatable was returning null in
Animatable.getParentSurface(), but we still need to invoke
onAnimationLeashDestroyed.

We also rename the callback to onAnimationLeashLost as it will also be called
when the animation is transferred away to another animation, even though the
leash didn't get destroyed.

Also, modified TaskScreenshotAnimatable#getParentSurfaceControl to
align Task's surface for surface hierachy correctness.

Fix: 130606600
Test: atest RecentsAnimationControllerTest
Test: atest SurfaceAnimatorTest

Change-Id: I918554befe3982a3dc0a9949c6973042697bb24b
parent ccc7759b
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -195,7 +195,7 @@ class AppWindowThumbnail implements Animatable {
    }

    @Override
    public void onAnimationLeashDestroyed(Transaction t) {
    public void onAnimationLeashLost(Transaction t) {

        // TODO: Once attached to app token, we don't need to hide it immediately if thumbnail
        // became visible.
+3 −3
Original line number Diff line number Diff line
@@ -2668,8 +2668,8 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
    }

    @Override
    public void onAnimationLeashDestroyed(Transaction t) {
        super.onAnimationLeashDestroyed(t);
    public void onAnimationLeashLost(Transaction t) {
        super.onAnimationLeashLost(t);
        if (mAnimationBoundsLayer != null) {
            t.remove(mAnimationBoundsLayer);
            mAnimationBoundsLayer = null;
@@ -2855,7 +2855,7 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
        t.reparent(mTransitChangeLeash, null);
        mTransitChangeLeash = null;
        if (cancel) {
            onAnimationLeashDestroyed(t);
            onAnimationLeashLost(t);
        }
    }

+1 −1
Original line number Diff line number Diff line
@@ -62,7 +62,7 @@ class Dimmer {
        }

        @Override
        public void onAnimationLeashDestroyed(SurfaceControl.Transaction t) {
        public void onAnimationLeashLost(SurfaceControl.Transaction t) {
        }

        @Override
+4 −0
Original line number Diff line number Diff line
@@ -331,6 +331,10 @@ class RecentsAnimation implements RecentsAnimationCallbacks,
        }
        final RecentsAnimationController controller =
                mWindowManager.getRecentsAnimationController();
        if (controller == null) {
            return;
        }

        final DisplayContent dc =
                mService.mRootActivityContainer.getDefaultDisplay().mDisplayContent;
        dc.mBoundsAnimationController.setAnimationType(
+16 −12
Original line number Diff line number Diff line
@@ -50,7 +50,8 @@ class SurfaceAnimator {

    @VisibleForTesting
    SurfaceControl mLeash;
    private final Animatable mAnimatable;
    @VisibleForTesting
    final Animatable mAnimatable;
    private final OnAnimationFinishedCallback mInnerAnimationFinishedCallback;
    @VisibleForTesting
    final Runnable mAnimationFinishedCallback;
@@ -282,13 +283,15 @@ class SurfaceAnimator {

        boolean scheduleAnim = false;

        // If the surface was destroyed, we don't care to reparent it back.
        final boolean destroy = mLeash != null && surface != null && parent != null;
        if (destroy) {
            if (DEBUG_ANIM) Slog.i(TAG, "Reparenting to original parent");
        // 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 = mLeash != null && surface != null;
        if (reparent) {
            if (DEBUG_ANIM) Slog.i(TAG, "Reparenting to original parent: " + parent);
            // We shouldn't really need these isValid checks but we do
            // b/130364451
            if (surface.isValid() && parent.isValid()) {
            if (surface.isValid() && parent != null && parent.isValid()) {
                t.reparent(surface, parent);
                scheduleAnim = true;
            }
@@ -301,9 +304,10 @@ class SurfaceAnimator {
        mLeash = null;
        mAnimation = null;

        // Make sure to inform the animatable after the leash was destroyed.
        if (destroy) {
            mAnimatable.onAnimationLeashDestroyed(t);
        if (reparent) {
            // Make sure to inform the animatable after the surface was reparented (or reparent
            // wasn't possible, but we still need to invoke the callback)
            mAnimatable.onAnimationLeashLost(t);
            scheduleAnim = true;
        }

@@ -394,12 +398,12 @@ class SurfaceAnimator {
        void onAnimationLeashCreated(Transaction t, SurfaceControl leash);

        /**
         * Called when the leash is being destroyed, and the surface was reparented back to the
         * original parent.
         * Called when the leash is being destroyed, or when the leash is being transferred to
         * another SurfaceAnimator.
         *
         * @param t The transaction to use to apply any necessary changes.
         */
        void onAnimationLeashDestroyed(Transaction t);
        void onAnimationLeashLost(Transaction t);

        /**
         * @return A new surface to be used for the animation leash, inserted at the correct
Loading