Fix PiP Shell Transitions

PiP Shell Transitions needs to be more robust to pass CTS tests
reliably.

Some changes:

1. DELIVERED_TO_TOP intents don't create a transition since there is
   no change to the WM hierarchy. This way the app-launch transition
   won't conflict with activityRestartAttempt.

2. If PiP started an expand transition while an enter transition is
   ongoing, cancel the enter transition.
   a. Additionally, because PiP doesn't resize task until the enter
      transition finishes, the expand transition will actually be
      a no-op. If we see this pattern, also cancel the enter
      transition.

3. When taskVanished, make sure to call any pending finishCallback
   because it doesn't allow the running animator to properly finish.
   Transition systems require the finish callback to function
   properly.

4. Minimize the uses of finishTransition with a sync callback. We
   probably need to phase this out because it can conflict with
   subsequent startTransition calls (because only 1 sync is supported
   at a time).

Bug: 183993924
Test: atest PinnedStackTests
Change-Id: Ieb573032839a2f31b0bb371410484fc93a1714fa

6 files changed, 94 insertions, 23 deletions

diff --git a/data/etc/services.core.protolog.json b/data/etc/services.core.protolog.json
index 95736074a3ef..6e92755be98e 100644
--- a/data/etc/services.core.protolog.json
+++ b/data/etc/services.core.protolog.json
@@ -1141,6 +1141,12 @@
       "group": "WM_DEBUG_BOOT",
       "at": "com\/android\/server\/wm\/WindowManagerService.java"
     },
+    "-863438038": {
+      "message": "Aborting Transition: %d",
+      "level": "VERBOSE",
+      "group": "WM_DEBUG_WINDOW_TRANSITIONS",
+      "at": "com\/android\/server\/wm\/Transition.java"
+    },
     "-861859917": {
       "message": "Attempted to add window to a display that does not exist: %d. Aborting.",
       "level": "WARN",
diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTaskOrganizer.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTaskOrganizer.java
index 96867761cc7e..291cbb3676dc 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTaskOrganizer.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTaskOrganizer.java
@@ -282,6 +282,7 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
         mMainExecutor.execute(() -> {
             mTaskOrganizer.addListenerForType(this, TASK_LISTENER_TYPE_PIP);
         });
+        mPipTransitionController.setPipOrganizer(this);
         displayController.addDisplayWindowListener(this);
     }
 
@@ -349,6 +350,10 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
         }
     }
 
+    public ActivityManager.RunningTaskInfo getTaskInfo() {
+        return mTaskInfo;
+    }
+
     public SurfaceControl getSurfaceControl() {
         return mLeash;
     }
@@ -716,6 +721,9 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
             mOnDisplayIdChangeCallback.accept(Display.DEFAULT_DISPLAY);
         }
 
+        if (Transitions.ENABLE_SHELL_TRANSITIONS) {
+            mPipTransitionController.forceFinishTransition();
+        }
         final PipAnimationController.PipTransitionAnimator<?> animator =
                 mPipAnimationController.getCurrentAnimator();
         if (animator != null) {
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 6fec1fbda7b0..328f3ed73f2e 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
@@ -32,17 +32,18 @@ import static com.android.wm.shell.pip.PipAnimationController.isOutPipDirection;
 import static com.android.wm.shell.transition.Transitions.TRANSIT_EXIT_PIP;
 import static com.android.wm.shell.transition.Transitions.TRANSIT_REMOVE_PIP;
 
+import android.app.ActivityManager;
 import android.app.TaskInfo;
 import android.content.Context;
 import android.graphics.Matrix;
 import android.graphics.Rect;
 import android.os.IBinder;
+import android.util.Log;
 import android.view.Surface;
 import android.view.SurfaceControl;
 import android.window.TransitionInfo;
 import android.window.TransitionRequestInfo;
 import android.window.WindowContainerTransaction;
-import android.window.WindowContainerTransactionCallback;
 
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
@@ -57,11 +58,14 @@ import com.android.wm.shell.transition.Transitions;
  */
 public class PipTransition extends PipTransitionController {
 
+    private static final String TAG = PipTransition.class.getSimpleName();
+
     private final PipTransitionState mPipTransitionState;
     private final int mEnterExitAnimationDuration;
     private @PipAnimationController.AnimationType int mOneShotAnimationType = ANIM_TYPE_BOUNDS;
     private Transitions.TransitionFinishCallback mFinishCallback;
     private Rect mExitDestinationBounds = new Rect();
+    private IBinder mExitTransition = null;
 
     public PipTransition(Context context,
             PipBoundsState pipBoundsState,
@@ -96,7 +100,7 @@ public class PipTransition extends PipTransitionController {
     public void startTransition(Rect destinationBounds, WindowContainerTransaction out) {
         if (destinationBounds != null) {
             mExitDestinationBounds.set(destinationBounds);
-            mTransitions.startTransition(TRANSIT_EXIT_PIP, out, this);
+            mExitTransition = mTransitions.startTransition(TRANSIT_EXIT_PIP, out, this);
         } else {
             mTransitions.startTransition(TRANSIT_REMOVE_PIP, out, this);
         }
@@ -109,14 +113,19 @@ public class PipTransition extends PipTransitionController {
             @android.annotation.NonNull SurfaceControl.Transaction finishTransaction,
             @android.annotation.NonNull Transitions.TransitionFinishCallback finishCallback) {
 
-        if (info.getType() == TRANSIT_EXIT_PIP && info.getChanges().size() == 1) {
-            final TransitionInfo.Change change = info.getChanges().get(0);
-            mFinishCallback = finishCallback;
-            startTransaction.apply();
-            boolean success = startExpandAnimation(change.getTaskInfo(), change.getLeash(),
-                    new Rect(mExitDestinationBounds));
-            mExitDestinationBounds.setEmpty();
-            return success;
+        if (mExitTransition == transition || info.getType() == TRANSIT_EXIT_PIP) {
+            mExitTransition = null;
+            if (info.getChanges().size() == 1) {
+                final TransitionInfo.Change change = info.getChanges().get(0);
+                mFinishCallback = finishCallback;
+                startTransaction.apply();
+                boolean success = startExpandAnimation(change.getTaskInfo(), change.getLeash(),
+                        new Rect(mExitDestinationBounds));
+                mExitDestinationBounds.setEmpty();
+                return success;
+            } else {
+                Log.e(TAG, "Got an exit-pip transition with unexpected change-list");
+            }
         }
 
         if (info.getType() == TRANSIT_REMOVE_PIP) {
@@ -183,26 +192,58 @@ public class PipTransition extends PipTransitionController {
     }
 
     @Override
+    public void onTransitionMerged(@NonNull IBinder transition) {
+        if (transition != mExitTransition) {
+            return;
+        }
+        // This means an expand happened before enter-pip finished and we are now "merging" a
+        // no-op transition that happens to match our exit-pip.
+        boolean cancelled = false;
+        if (mPipAnimationController.getCurrentAnimator() != null) {
+            mPipAnimationController.getCurrentAnimator().cancel();
+            cancelled = true;
+        }
+        // Unset exitTransition AFTER cancel so that finishResize knows we are merging.
+        mExitTransition = null;
+        if (!cancelled) return;
+        final ActivityManager.RunningTaskInfo taskInfo = mPipOrganizer.getTaskInfo();
+        if (taskInfo != null) {
+            startExpandAnimation(taskInfo, mPipOrganizer.getSurfaceControl(),
+                    new Rect(mExitDestinationBounds));
+        }
+        mExitDestinationBounds.setEmpty();
+    }
+
+    @Override
     public void onFinishResize(TaskInfo taskInfo, Rect destinationBounds,
             @PipAnimationController.TransitionDirection int direction,
-            SurfaceControl.Transaction tx) {
+            @Nullable SurfaceControl.Transaction tx) {
 
         if (isInPipDirection(direction)) {
             mPipTransitionState.setTransitionState(PipTransitionState.ENTERED_PIP);
         }
-        WindowContainerTransaction wct = new WindowContainerTransaction();
-        prepareFinishResizeTransaction(taskInfo, destinationBounds,
-                direction, tx, wct);
-        mFinishCallback.onTransitionFinished(wct, new WindowContainerTransactionCallback() {
-            @Override
-            public void onTransactionReady(int id, @NonNull SurfaceControl.Transaction t) {
-                t.merge(tx);
-                t.apply();
+        // 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) {
+            WindowContainerTransaction wct = new WindowContainerTransaction();
+            prepareFinishResizeTransaction(taskInfo, destinationBounds,
+                    direction, wct);
+            if (tx != null) {
+                wct.setBoundsChangeTransaction(taskInfo.token, tx);
             }
-        });
+            mFinishCallback.onTransitionFinished(wct, null /* wctCallback */);
+            mFinishCallback = null;
+        }
         finishResizeForMenu(destinationBounds);
     }
 
+    @Override
+    public void forceFinishTransition() {
+        if (mFinishCallback == null) return;
+        mFinishCallback.onTransitionFinished(null /* wct */, null /* wctCallback */);
+        mFinishCallback = null;
+    }
+
     private boolean startExpandAnimation(final TaskInfo taskInfo, final SurfaceControl leash,
             final Rect destinationBounds) {
         PipAnimationController.PipTransitionAnimator animator =
@@ -243,7 +284,7 @@ public class PipTransition extends PipTransitionController {
             startTransaction.merge(tx);
             startTransaction.apply();
             mPipBoundsState.setBounds(destinationBounds);
-            onFinishResize(taskInfo, destinationBounds, TRANSITION_DIRECTION_TO_PIP, tx);
+            onFinishResize(taskInfo, destinationBounds, TRANSITION_DIRECTION_TO_PIP, null /* tx */);
             sendOnPipTransitionFinished(TRANSITION_DIRECTION_TO_PIP);
             mFinishCallback = null;
             mPipTransitionState.setInSwipePipToHomeTransition(false);
@@ -292,7 +333,6 @@ public class PipTransition extends PipTransitionController {
 
     private void prepareFinishResizeTransaction(TaskInfo taskInfo, Rect destinationBounds,
             @PipAnimationController.TransitionDirection int direction,
-            SurfaceControl.Transaction tx,
             WindowContainerTransaction wct) {
         Rect taskBounds = null;
         if (isInPipDirection(direction)) {
diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTransitionController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTransitionController.java
index dbf603ca72d9..376f3298a83c 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTransitionController.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTransitionController.java
@@ -49,6 +49,7 @@ public abstract class PipTransitionController implements Transitions.TransitionH
     protected final Transitions mTransitions;
     private final Handler mMainHandler;
     private final List<PipTransitionCallback> mPipTransitionCallbacks = new ArrayList<>();
+    protected PipTaskOrganizer mPipOrganizer;
 
     protected final PipAnimationController.PipAnimationCallback mPipAnimationCallback =
             new PipAnimationController.PipAnimationCallback() {
@@ -103,6 +104,13 @@ public abstract class PipTransitionController implements Transitions.TransitionH
         // Default implementation does nothing.
     }
 
+    /**
+     * Called when the transition animation can't continue (eg. task is removed during
+     * animation)
+     */
+    public void forceFinishTransition() {
+    }
+
     public PipTransitionController(PipBoundsState pipBoundsState,
             PipMenuController pipMenuController, PipBoundsAlgorithm pipBoundsAlgorithm,
             PipAnimationController pipAnimationController, Transitions transitions,
@@ -119,6 +127,10 @@ public abstract class PipTransitionController implements Transitions.TransitionH
         }
     }
 
+    void setPipOrganizer(PipTaskOrganizer pto) {
+        mPipOrganizer = pto;
+    }
+
     /**
      * Registers {@link PipTransitionCallback} to receive transition callbacks.
      */
diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java
index 47467abf459b..edf330ada31d 100644
--- a/services/core/java/com/android/server/wm/ActivityStarter.java
+++ b/services/core/java/com/android/server/wm/ActivityStarter.java
@@ -1565,7 +1565,7 @@ class ActivityStarter {
         // transition based on a sub-action.
         // Only do the create here (and defer requestStart) since startActivityInner might abort.
         final TransitionController transitionController = r.mTransitionController;
-        final Transition newTransition = (!transitionController.isCollecting()
+        Transition newTransition = (!transitionController.isCollecting()
                 && transitionController.getTransitionPlayer() != null)
                 ? transitionController.createTransition(TRANSIT_OPEN) : null;
         RemoteTransition remoteTransition = r.takeRemoteTransition();
@@ -1620,6 +1620,10 @@ class ActivityStarter {
                     // The activity is started new rather than just brought forward, so record
                     // it as an existence change.
                     transitionController.collectExistenceChange(r);
+                } else if (result == START_DELIVERED_TO_TOP && newTransition != null) {
+                    // We just delivered to top, so there isn't an actual transition here
+                    newTransition.abort();
+                    newTransition = null;
                 }
                 if (isTransient) {
                     // `r` isn't guaranteed to be the actual relevant activity, so we must wait
diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java
index e537c0afc147..0cd126bcaab4 100644
--- a/services/core/java/com/android/server/wm/Transition.java
+++ b/services/core/java/com/android/server/wm/Transition.java
@@ -469,6 +469,7 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe
         if (mState != STATE_COLLECTING) {
             throw new IllegalStateException("Too late to abort.");
         }
+        ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "Aborting Transition: %d", mSyncId);
         mController.dispatchLegacyAppTransitionCancelled();
         mState = STATE_ABORT;
         // Syncengine abort will call through to onTransactionReady()