From 06502521d4ad6119a00b81eb69c866c4111df854 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
Merged-In: 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 9c2c2fa8598a1..af79386caf9c4 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
@@ -619,12 +619,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,
@@ -637,20 +638,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);