From 4dd07128770768223b0025f4cdc7559cdc71707f Mon Sep 17 00:00:00 2001
From: wilsonshih <wilsonshih@google.com>
Date: Wed, 3 Apr 2019 15:55:02 +0800
Subject: [PATCH] Fix pinned stack destination been overwrite by obsolete
 animator.

There is a race condition when an app leave from landscape mode and enter
pinned stack in portrait mode. First racer is that TaskStack will post
animateBounds to animation thread, the second is #onConfigurationChanged
will ask TaskStack to update new stack bounds if needed.

When the first animation executed, it will call TaskStack#onAnimationEnd
then #setPinnedStackSize, however, this bounds may already obsoleted if
configuration changed happen after first animation is posted.

To fix this issue, we can cancel the existing animator via verifying the
final destination, and prevent animator set obsoleted bounds back if the
animation is canceled.

Fix: 129723312
Test: Try reproduce this issue over 100+ times.
Test: atest ActivityManagerPinnedStackTests
Test: atest BoundsAnimationControllerTests

Change-Id: I5d42cbe576e7683b22679b1e97be07063c2a05d6
---
 .../server/wm/BoundsAnimationController.java        | 12 ++++++++++++
 .../core/java/com/android/server/wm/TaskStack.java  | 13 +++++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/services/core/java/com/android/server/wm/BoundsAnimationController.java b/services/core/java/com/android/server/wm/BoundsAnimationController.java
index c1d872f23f0f1..b5e7067108238 100644
--- a/services/core/java/com/android/server/wm/BoundsAnimationController.java
+++ b/services/core/java/com/android/server/wm/BoundsAnimationController.java
@@ -435,6 +435,18 @@ public class BoundsAnimationController {
                 moveFromFullscreen, moveToFullscreen, animationType);
     }
 
+    /**
+     * Cancel existing animation if the destination was modified.
+     */
+    void cancel(final BoundsAnimationTarget target) {
+        final BoundsAnimator existing = mRunningAnimations.get(target);
+        if (existing != null) {
+            // Cancel animation. Since its already started, send animation end to client.
+            if (DEBUG) Slog.d(TAG, "cancel: mTarget= " + target);
+            existing.cancelAndCallAnimationEnd();
+        }
+    }
+
     @VisibleForTesting
     BoundsAnimator animateBoundsImpl(final BoundsAnimationTarget target, Rect from, Rect to,
             int animationDuration, @SchedulePipModeChangedState int schedulePipModeChangedState,
diff --git a/services/core/java/com/android/server/wm/TaskStack.java b/services/core/java/com/android/server/wm/TaskStack.java
index bdb4d04748655..7515b3fa12499 100644
--- a/services/core/java/com/android/server/wm/TaskStack.java
+++ b/services/core/java/com/android/server/wm/TaskStack.java
@@ -388,7 +388,9 @@ public class TaskStack extends WindowContainer<Task> implements
      * @return true if bounds were updated to some non-empty value.
      */
     boolean calculatePinnedBoundsForConfigChange(Rect inOutBounds) {
+        boolean animating = false;
         if ((mBoundsAnimatingRequested || mBoundsAnimating) && !mBoundsAnimationTarget.isEmpty()) {
+            animating = true;
             getFinalAnimationBounds(mTmpRect2);
         } else {
             mTmpRect2.set(inOutBounds);
@@ -398,6 +400,13 @@ public class TaskStack extends WindowContainer<Task> implements
         if (updated) {
             inOutBounds.set(mTmpRect3);
 
+            // The final boundary is updated while there is an existing boundary animation. Let's
+            // cancel this animation to prevent the obsolete animation overwritten updated bounds.
+            if (animating && !inOutBounds.equals(mBoundsAnimationTarget)) {
+                final DisplayContent displayContent = getDisplayContent();
+                displayContent.mBoundsAnimationController.getHandler().post(() ->
+                        displayContent.mBoundsAnimationController.cancel(this));
+            }
             // Once we've set the bounds based on the rotation of the old bounds in the new
             // orientation, clear the animation target bounds since they are obsolete, and
             // cancel any currently running animations
@@ -1585,7 +1594,6 @@ public class TaskStack extends WindowContainer<Task> implements
 
             mBoundsAnimatingRequested = false;
             mBoundsAnimating = true;
-            mCancelCurrentBoundsAnimation = false;
             mAnimationType = animationType;
 
             // If we are changing UI mode, as in the PiP to fullscreen
@@ -1645,7 +1653,7 @@ public class TaskStack extends WindowContainer<Task> implements
                         mBoundsAnimationTarget, false /* forceUpdate */);
             }
 
-            if (finalStackSize != null) {
+            if (finalStackSize != null && !mCancelCurrentBoundsAnimation) {
                 setPinnedStackSize(finalStackSize, null);
             } else {
                 // We have been canceled, so the final stack size is null, still run the
@@ -1758,6 +1766,7 @@ public class TaskStack extends WindowContainer<Task> implements
         }
 
         final @BoundsAnimationController.AnimationType int animationType = intendedAnimationType;
+        mCancelCurrentBoundsAnimation = false;
         displayContent.mBoundsAnimationController.getHandler().post(() -> {
             displayContent.mBoundsAnimationController.animateBounds(this, fromBounds,
                     finalToBounds, animationDuration, finalSchedulePipModeChangedState,