From 6dd00dbf5affe80a490fceb61aa9087fe2502b22 Mon Sep 17 00:00:00 2001
From: Jorge Gil <jorgegil@google.com>
Date: Tue, 18 Oct 2022 19:46:45 +0000
Subject: [PATCH] Prevent animation update callbacks after it has finished

Remove the update listener once the animation has ended or has been
canceled to prevent further updates from trying to animate a surface
that has already been released.
This scenario can happend when mergeAnimation() manually runs end() on
animations that already finished but had not been removed yet. This may
happen because the removal of the animation is posted from the shell
anim thread to the shell main thread, so mergeAnimation() may win a race
and schedule another end() before the first end() removes the
animations.

Bug: 252872225
Test: boot, no shell.anim crash
Change-Id: I442f1152fba97c30ce425b08f1aa65bb94fb3bb4
---
 .../transition/DefaultTransitionHandler.java  | 21 ++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultTransitionHandler.java b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultTransitionHandler.java
index 91c153e7b8fda..addd0daf0cc75 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultTransitionHandler.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultTransitionHandler.java
@@ -740,12 +740,13 @@ public class DefaultTransitionHandler implements Transitions.TransitionHandler {
         // Animation length is already expected to be scaled.
         va.overrideDurationScale(1.0f);
         va.setDuration(anim.computeDurationHint());
-        va.addUpdateListener(animation -> {
+        final ValueAnimator.AnimatorUpdateListener updateListener = animation -> {
             final long currentPlayTime = Math.min(va.getDuration(), va.getCurrentPlayTime());
 
             applyTransformation(currentPlayTime, transaction, leash, anim, transformation, matrix,
                     position, cornerRadius, clipRect);
-        });
+        };
+        va.addUpdateListener(updateListener);
 
         final Runnable finisher = () -> {
             applyTransformation(va.getDuration(), transaction, leash, anim, transformation, matrix,
@@ -758,20 +759,30 @@ public class DefaultTransitionHandler implements Transitions.TransitionHandler {
             });
         };
         va.addListener(new AnimatorListenerAdapter() {
+            // It is possible for the end/cancel to be called more than once, which may cause
+            // issues if the animating surface has already been released. Track the finished
+            // state here to skip duplicate callbacks. See b/252872225.
             private boolean mFinished = false;
 
             @Override
             public void onAnimationEnd(Animator animation) {
-                if (mFinished) return;
-                mFinished = true;
-                finisher.run();
+                onFinish();
             }
 
             @Override
             public void onAnimationCancel(Animator animation) {
+                onFinish();
+            }
+
+            private void onFinish() {
                 if (mFinished) return;
                 mFinished = true;
                 finisher.run();
+                // The update listener can continue to be called after the animation has ended if
+                // end() is called manually again before the finisher removes the animation.
+                // Remove it manually here to prevent animating a released surface.
+                // See b/252872225.
+                va.removeUpdateListener(updateListener);
             }
         });
         animations.add(va);