From 9c464cc063c78e164db0d84c2b7c189d97e3dd88 Mon Sep 17 00:00:00 2001
From: Evan Rosky <erosky@google.com>
Date: Wed, 16 Mar 2022 09:44:06 -0700
Subject: [PATCH] Fix pip finish-callback when enter animation runs in shell

PiP animations happen in both launcher and in shell. On top of that,
the implementations are different!? The launcher impl will actually
merge expand-pip into an enter animation, but the shell impl waits
for the enter animation to finish. This means we need to alter the
finish-handling logic depending on whether the animation is running
"locally" in shell or not.

Additionally, fixed an ordering-problem: This was calling the
finish-callback before unsetting the mFinishCallback member. The
problem is that if there are pending animations queued-up, those
will start in sync -- if those end up calling-back into PipTransition,
then they will see mFinishCallback as non-null.

Bug: 224622288
Test: atest PinnedStackTests
Change-Id: Ie3696b101ecca69fae93b347ff358863cdb4e6c3
---
 .../android/wm/shell/pip/PipTransition.java   | 30 +++++++++++++------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTransition.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTransition.java
index be713a59a816e..e5a755c35add6 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTransition.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTransition.java
@@ -166,8 +166,7 @@ public class PipTransition extends PipTransitionController {
             mExitTransition = null;
             mHasFadeOut = false;
             if (mFinishCallback != null) {
-                mFinishCallback.onTransitionFinished(null, null);
-                mFinishCallback = null;
+                callFinishCallback(null /* wct */);
                 mFinishTransaction = null;
                 throw new RuntimeException("Previous callback not called, aborting exit PIP.");
             }
@@ -232,6 +231,11 @@ public class PipTransition extends PipTransitionController {
         return false;
     }
 
+    /** Helper to identify whether this handler is currently the one playing an animation */
+    private boolean isAnimatingLocally() {
+        return mFinishTransaction != null;
+    }
+
     @Nullable
     @Override
     public WindowContainerTransaction handleRequest(@NonNull IBinder transition,
@@ -282,9 +286,11 @@ public class PipTransition extends PipTransitionController {
         if (enteringPip) {
             mPipTransitionState.setTransitionState(ENTERED_PIP);
         }
-        // If there is an expected exit transition, then the exit will be "merged" into this
-        // transition so don't fire the finish-callback in that case.
-        if (mExitTransition == null && mFinishCallback != null) {
+        // If we have an exit transition, but aren't playing a transition locally, it
+        // means we're expecting the exit transition will be "merged" into another transition
+        // (likely a remote like launcher), so don't fire the finish-callback here -- wait until
+        // the exit transition is merged.
+        if ((mExitTransition == null || isAnimatingLocally()) && mFinishCallback != null) {
             WindowContainerTransaction wct = new WindowContainerTransaction();
             prepareFinishResizeTransaction(taskInfo, destinationBounds,
                     direction, wct);
@@ -305,12 +311,19 @@ public class PipTransition extends PipTransitionController {
                 mSurfaceTransactionHelper.crop(mFinishTransaction, leash, finishBounds);
             }
             mFinishTransaction = null;
-            mFinishCallback.onTransitionFinished(wct, null /* callback */);
-            mFinishCallback = null;
+            callFinishCallback(wct);
         }
         finishResizeForMenu(destinationBounds);
     }
 
+    private void callFinishCallback(WindowContainerTransaction wct) {
+        // Need to unset mFinishCallback first because onTransitionFinished can re-enter this
+        // handler if there is a pending PiP animation.
+        final Transitions.TransitionFinishCallback finishCallback = mFinishCallback;
+        mFinishCallback = null;
+        finishCallback.onTransitionFinished(wct, null /* callback */);
+    }
+
     @Override
     public void forceFinishTransition() {
         if (mFinishCallback == null) return;
@@ -572,8 +585,7 @@ public class PipTransition extends PipTransitionController {
         mHasFadeOut = false;
 
         if (mFinishCallback != null) {
-            mFinishCallback.onTransitionFinished(null /* wct */, null /* callback */);
-            mFinishCallback = null;
+            callFinishCallback(null /* wct */);
             mFinishTransaction = null;
             throw new RuntimeException("Previous callback not called, aborting entering PIP.");
         }