From f71a1b2970c6a2c93201d38d4053814d3f3be392 Mon Sep 17 00:00:00 2001 From: Ben Lin Date: Mon, 18 Oct 2021 17:57:32 -0700 Subject: [PATCH] PiP: Fix more Shell Transition potential issues. If an animator is already running, we usually just update its end bounds - but we also end up over-writing the original callbacks. This used to work well since pre-shell transition, callbacks are always pulled from PipTaskOrganizer, but now we can have two places that supply callbacks (PipTransition). So let's just leave the callback alone. Another potential issue is that sometimes finish callbacks from one transition is not called before starting another one, and starting a new transition overwrites the old finishCallback. We will then finish the earlier callbacks to ensure the transition system does not hang the entire device, but throw an Exception to not gloss over the potential issue. Bug: 183993924 Test: atest PinnedStackTests Change-Id: Ib4c5926beec5745d8a0abccec5c67849488e532b --- .../wm/shell/pip/PipTaskOrganizer.java | 6 ++++- .../android/wm/shell/pip/PipTransition.java | 23 ++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) 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 291cbb3676dcf..b6e5804a64ddf 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 @@ -1292,13 +1292,17 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener, } Rect baseBounds = direction == TRANSITION_DIRECTION_SNAP_AFTER_RESIZE ? mPipBoundsState.getBounds() : currentBounds; + final boolean existingAnimatorRunning = mPipAnimationController.getCurrentAnimator() != null + && mPipAnimationController.getCurrentAnimator().isRunning(); final PipAnimationController.PipTransitionAnimator animator = mPipAnimationController .getAnimator(mTaskInfo, mLeash, baseBounds, currentBounds, destinationBounds, sourceHintRect, direction, startingAngle, rotationDelta); animator.setTransitionDirection(direction) - .setPipAnimationCallback(mPipAnimationCallback) .setPipTransactionHandler(mPipTransactionHandler) .setDuration(durationMs); + if (!existingAnimatorRunning) { + animator.setPipAnimationCallback(mPipAnimationCallback); + } if (isInPipDirection(direction)) { // Similar to auto-enter-pip transition, we use content overlay when there is no // source rect hint to enter PiP use bounds animation. 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 328f3ed73f2eb..b31e6e0750ce1 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 @@ -116,6 +116,12 @@ public class PipTransition extends PipTransitionController { if (mExitTransition == transition || info.getType() == TRANSIT_EXIT_PIP) { mExitTransition = null; if (info.getChanges().size() == 1) { + if (mFinishCallback != null) { + mFinishCallback.onTransitionFinished(null, null); + mFinishCallback = null; + throw new RuntimeException("Previous callback not called, aborting exit PIP."); + } + final TransitionInfo.Change change = info.getChanges().get(0); mFinishCallback = finishCallback; startTransaction.apply(); @@ -129,6 +135,12 @@ public class PipTransition extends PipTransitionController { } if (info.getType() == TRANSIT_REMOVE_PIP) { + if (mFinishCallback != null) { + mFinishCallback.onTransitionFinished(null /* wct */, null /* callback */); + mFinishCallback = null; + throw new RuntimeException("Previous callback not called, aborting remove PIP."); + } + startTransaction.apply(); finishTransaction.setWindowCrop(info.getChanges().get(0).getLeash(), mPipBoundsState.getDisplayBounds()); @@ -159,6 +171,12 @@ public class PipTransition extends PipTransitionController { return false; } + if (mFinishCallback != null) { + mFinishCallback.onTransitionFinished(null /* wct */, null /* callback */); + mFinishCallback = null; + throw new RuntimeException("Previous callback not called, aborting entering PIP."); + } + // Show the wallpaper if there is a wallpaper change. if (wallpaper != null) { startTransaction.show(wallpaper.getLeash()); @@ -231,7 +249,7 @@ public class PipTransition extends PipTransitionController { if (tx != null) { wct.setBoundsChangeTransaction(taskInfo.token, tx); } - mFinishCallback.onTransitionFinished(wct, null /* wctCallback */); + mFinishCallback.onTransitionFinished(wct, null /* callback */); mFinishCallback = null; } finishResizeForMenu(destinationBounds); @@ -240,7 +258,7 @@ public class PipTransition extends PipTransitionController { @Override public void forceFinishTransition() { if (mFinishCallback == null) return; - mFinishCallback.onTransitionFinished(null /* wct */, null /* wctCallback */); + mFinishCallback.onTransitionFinished(null /* wct */, null /* callback */); mFinishCallback = null; } @@ -286,7 +304,6 @@ public class PipTransition extends PipTransitionController { mPipBoundsState.setBounds(destinationBounds); onFinishResize(taskInfo, destinationBounds, TRANSITION_DIRECTION_TO_PIP, null /* tx */); sendOnPipTransitionFinished(TRANSITION_DIRECTION_TO_PIP); - mFinishCallback = null; mPipTransitionState.setInSwipePipToHomeTransition(false); return true; }