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
This commit is contained in:
Evan Rosky
2021-10-12 18:37:47 -07:00
parent 10e6377fd1
commit c6d5abe414
6 changed files with 94 additions and 23 deletions

View File

@@ -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",

View File

@@ -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) {

View File

@@ -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) {
@@ -182,27 +191,59 @@ 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)) {

View File

@@ -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.
*/

View File

@@ -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

View File

@@ -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()