From e13200ed35fd55b6c2c0e6dcc49baff52189188a Mon Sep 17 00:00:00 2001 From: Jerry Chang Date: Wed, 29 Mar 2023 03:38:58 +0000 Subject: [PATCH] Fix transition when leaving a split pair by home key Pressing home key while in 3-button nav mode won't have an explicit remote transition and thus need to lean on RemoteTransitionHandler to fetch the proper remote transition from registered. This makes sure to cover the flow for split screen so split screen components won't need to deal with the recent transition by itself. Fix: 273685456 Test: atest WMShellUnitTests WMShellFlickerTests Test: http://recall/-/fLARJNt42LVxc3tt86SneW/c2pLS6FwyEMweiLiWqqzPa Change-Id: Icf2739600601bb591dd865ba785151234de4fbd0 --- .../splitscreen/SplitScreenTransitions.java | 106 +++++++----------- .../shell/splitscreen/StageCoordinator.java | 24 ++-- .../shell/transition/DefaultMixedHandler.java | 17 +-- .../wm/shell/transition/Transitions.java | 4 + .../splitscreen/DismissSplitScreenByGoHome.kt | 2 +- .../splitscreen/SplitTransitionTests.java | 4 +- 6 files changed, 66 insertions(+), 91 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/splitscreen/SplitScreenTransitions.java b/libs/WindowManager/Shell/src/com/android/wm/shell/splitscreen/SplitScreenTransitions.java index ebdaaa9aa4a96..616dccaf3ae15 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/splitscreen/SplitScreenTransitions.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/splitscreen/SplitScreenTransitions.java @@ -66,13 +66,11 @@ class SplitScreenTransitions { private final Transitions mTransitions; private final Runnable mOnFinish; - DismissTransition mPendingDismiss = null; + DismissSession mPendingDismiss = null; TransitSession mPendingEnter = null; - TransitSession mPendingRecent = null; TransitSession mPendingResize = null; private IBinder mAnimatingTransition = null; - OneShotRemoteHandler mPendingRemoteHandler = null; private OneShotRemoteHandler mActiveRemoteHandler = null; private final Transitions.TransitionFinishCallback mRemoteFinishCB = this::onFinish; @@ -101,27 +99,30 @@ class SplitScreenTransitions { mFinishCallback = finishCallback; mAnimatingTransition = transition; mFinishTransaction = finishTransaction; - if (mPendingRemoteHandler != null) { - mPendingRemoteHandler.startAnimation(transition, info, startTransaction, - finishTransaction, mRemoteFinishCB); - mActiveRemoteHandler = mPendingRemoteHandler; - mPendingRemoteHandler = null; - return; + + final TransitSession pendingTransition = getPendingTransition(transition); + if (pendingTransition != null) { + if (pendingTransition.mCanceled) { + // The pending transition was canceled, so skip playing animation. + startTransaction.apply(); + onFinish(null /* wct */, null /* wctCB */); + return; + } + + if (pendingTransition.mRemoteHandler != null) { + pendingTransition.mRemoteHandler.startAnimation(transition, info, startTransaction, + finishTransaction, mRemoteFinishCB); + mActiveRemoteHandler = pendingTransition.mRemoteHandler; + return; + } } + playInternalAnimation(transition, info, startTransaction, mainRoot, sideRoot, topRoot); } private void playInternalAnimation(@NonNull IBinder transition, @NonNull TransitionInfo info, @NonNull SurfaceControl.Transaction t, @NonNull WindowContainerToken mainRoot, @NonNull WindowContainerToken sideRoot, @NonNull WindowContainerToken topRoot) { - final TransitSession pendingTransition = getPendingTransition(transition); - if (pendingTransition != null && pendingTransition.mCanceled) { - // The pending transition was canceled, so skip playing animation. - t.apply(); - onFinish(null /* wct */, null /* wctCB */); - return; - } - // Play some place-holder fade animations for (int i = info.getChanges().size() - 1; i >= 0; --i) { final TransitionInfo.Change change = info.getChanges().get(i); @@ -260,10 +261,6 @@ class SplitScreenTransitions { return mPendingEnter != null && mPendingEnter.mTransition == transition; } - boolean isPendingRecent(IBinder transition) { - return mPendingRecent != null && mPendingRecent.mTransition == transition; - } - boolean isPendingDismiss(IBinder transition) { return mPendingDismiss != null && mPendingDismiss.mTransition == transition; } @@ -276,8 +273,6 @@ class SplitScreenTransitions { private TransitSession getPendingTransition(IBinder transition) { if (isPendingEnter(transition)) { return mPendingEnter; - } else if (isPendingRecent(transition)) { - return mPendingRecent; } else if (isPendingDismiss(transition)) { return mPendingDismiss; } else if (isPendingResize(transition)) { @@ -311,14 +306,8 @@ class SplitScreenTransitions { @Nullable RemoteTransition remoteTransition, @Nullable TransitionConsumedCallback consumedCallback, @Nullable TransitionFinishedCallback finishedCallback) { - mPendingEnter = new TransitSession(transition, consumedCallback, finishedCallback); - - if (remoteTransition != null) { - // Wrapping it for ease-of-use (OneShot handles all the binder linking/death stuff) - mPendingRemoteHandler = new OneShotRemoteHandler( - mTransitions.getMainExecutor(), remoteTransition); - mPendingRemoteHandler.setTransition(transition); - } + mPendingEnter = new TransitSession( + transition, consumedCallback, finishedCallback, remoteTransition); ProtoLog.v(ShellProtoLogGroup.WM_SHELL_TRANSITIONS, " splitTransition " + " deduced Enter split screen"); @@ -344,7 +333,7 @@ class SplitScreenTransitions { /** Sets a transition to dismiss split. */ void setDismissTransition(@NonNull IBinder transition, @SplitScreen.StageType int dismissTop, @SplitScreenController.ExitReason int reason) { - mPendingDismiss = new DismissTransition(transition, reason, dismissTop); + mPendingDismiss = new DismissSession(transition, reason, dismissTop); ProtoLog.v(ShellProtoLogGroup.WM_SHELL_TRANSITIONS, " splitTransition " + " deduced Dismiss due to %s. toTop=%s", @@ -372,32 +361,10 @@ class SplitScreenTransitions { + " deduced Resize split screen"); } - void setRecentTransition(@NonNull IBinder transition, - @Nullable RemoteTransition remoteTransition, - @Nullable TransitionFinishedCallback finishCallback) { - mPendingRecent = new TransitSession(transition, null /* consumedCb */, finishCallback); - - if (remoteTransition != null) { - // Wrapping it for ease-of-use (OneShot handles all the binder linking/death stuff) - mPendingRemoteHandler = new OneShotRemoteHandler( - mTransitions.getMainExecutor(), remoteTransition); - mPendingRemoteHandler.setTransition(transition); - } - - ProtoLog.v(ShellProtoLogGroup.WM_SHELL_TRANSITIONS, " splitTransition " - + " deduced Enter recent panel"); - } - void mergeAnimation(IBinder transition, TransitionInfo info, SurfaceControl.Transaction t, IBinder mergeTarget, Transitions.TransitionFinishCallback finishCallback) { if (mergeTarget != mAnimatingTransition) return; - if (isPendingEnter(transition) && isPendingRecent(mergeTarget)) { - // Since there's an entering transition merged, recent transition no longer - // need to handle entering split screen after the transition finished. - mPendingRecent.setFinishedCallback(null); - } - if (mActiveRemoteHandler != null) { mActiveRemoteHandler.mergeAnimation(transition, info, t, mergeTarget, finishCallback); } else { @@ -425,19 +392,13 @@ class SplitScreenTransitions { // An entering transition got merged, appends the rest operations to finish entering // split screen. mStageCoordinator.finishEnterSplitScreen(finishT); - mPendingRemoteHandler = null; } mPendingEnter.onConsumed(aborted); mPendingEnter = null; - mPendingRemoteHandler = null; } else if (isPendingDismiss(transition)) { mPendingDismiss.onConsumed(aborted); mPendingDismiss = null; - } else if (isPendingRecent(transition)) { - mPendingRecent.onConsumed(aborted); - mPendingRecent = null; - mPendingRemoteHandler = null; } else if (isPendingResize(transition)) { mPendingResize.onConsumed(aborted); mPendingResize = null; @@ -451,9 +412,6 @@ class SplitScreenTransitions { if (isPendingEnter(mAnimatingTransition)) { mPendingEnter.onFinished(wct, mFinishTransaction); mPendingEnter = null; - } else if (isPendingRecent(mAnimatingTransition)) { - mPendingRecent.onFinished(wct, mFinishTransaction); - mPendingRecent = null; } else if (isPendingDismiss(mAnimatingTransition)) { mPendingDismiss.onFinished(wct, mFinishTransaction); mPendingDismiss = null; @@ -462,7 +420,6 @@ class SplitScreenTransitions { mPendingResize = null; } - mPendingRemoteHandler = null; mActiveRemoteHandler = null; mAnimatingTransition = null; @@ -568,10 +525,11 @@ class SplitScreenTransitions { } /** Session for a transition and its clean-up callback. */ - static class TransitSession { + class TransitSession { final IBinder mTransition; TransitionConsumedCallback mConsumedCallback; TransitionFinishedCallback mFinishedCallback; + OneShotRemoteHandler mRemoteHandler; /** Whether the transition was canceled. */ boolean mCanceled; @@ -579,10 +537,24 @@ class SplitScreenTransitions { TransitSession(IBinder transition, @Nullable TransitionConsumedCallback consumedCallback, @Nullable TransitionFinishedCallback finishedCallback) { + this(transition, consumedCallback, finishedCallback, null /* remoteTransition */); + } + + TransitSession(IBinder transition, + @Nullable TransitionConsumedCallback consumedCallback, + @Nullable TransitionFinishedCallback finishedCallback, + @Nullable RemoteTransition remoteTransition) { mTransition = transition; mConsumedCallback = consumedCallback; mFinishedCallback = finishedCallback; + if (remoteTransition != null) { + // Wrapping the remote transition for ease-of-use. (OneShot handles all the binder + // linking/death stuff) + mRemoteHandler = new OneShotRemoteHandler( + mTransitions.getMainExecutor(), remoteTransition); + mRemoteHandler.setTransition(transition); + } } /** Sets transition consumed callback. */ @@ -621,11 +593,11 @@ class SplitScreenTransitions { } /** Bundled information of dismiss transition. */ - static class DismissTransition extends TransitSession { + class DismissSession extends TransitSession { final int mReason; final @SplitScreen.StageType int mDismissTop; - DismissTransition(IBinder transition, int reason, int dismissTop) { + DismissSession(IBinder transition, int reason, int dismissTop) { super(transition, null /* consumedCallback */, null /* finishedCallback */); this.mReason = reason; this.mDismissTop = dismissTop; diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/splitscreen/StageCoordinator.java b/libs/WindowManager/Shell/src/com/android/wm/shell/splitscreen/StageCoordinator.java index 33cbdac670612..a5e2a4f22caab 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/splitscreen/StageCoordinator.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/splitscreen/StageCoordinator.java @@ -2226,15 +2226,9 @@ public class StageCoordinator implements SplitLayout.SplitLayoutHandler, } else if (isOpening && inFullscreen) { final int activityType = triggerTask.getActivityType(); if (activityType == ACTIVITY_TYPE_HOME || activityType == ACTIVITY_TYPE_RECENTS) { - if (request.getRemoteTransition() != null) { - // starting recents/home, so don't handle this and let it fall-through to - // the remote handler. - return null; - } - // Need to use the old stuff for non-remote animations, otherwise we don't - // exit split-screen. - mSplitTransitions.setRecentTransition(transition, null /* remote */, - this::onRecentsInSplitAnimationFinish); + // starting recents/home, so don't handle this and let it fall-through to + // the remote handler. + return null; } } } else { @@ -2363,8 +2357,6 @@ public class StageCoordinator implements SplitLayout.SplitLayoutHandler, if (mSplitTransitions.isPendingEnter(transition)) { shouldAnimate = startPendingEnterAnimation( transition, info, startTransaction, finishTransaction); - } else if (mSplitTransitions.isPendingRecent(transition)) { - onRecentsInSplitAnimationStart(startTransaction); } else if (mSplitTransitions.isPendingDismiss(transition)) { shouldAnimate = startPendingDismissAnimation( mSplitTransitions.mPendingDismiss, info, startTransaction, finishTransaction); @@ -2589,7 +2581,7 @@ public class StageCoordinator implements SplitLayout.SplitLayoutHandler, } private boolean startPendingDismissAnimation( - @NonNull SplitScreenTransitions.DismissTransition dismissTransition, + @NonNull SplitScreenTransitions.DismissSession dismissTransition, @NonNull TransitionInfo info, @NonNull SurfaceControl.Transaction t, @NonNull SurfaceControl.Transaction finishT) { prepareDismissAnimation(dismissTransition.mDismissTop, dismissTransition.mReason, info, @@ -2626,7 +2618,7 @@ public class StageCoordinator implements SplitLayout.SplitLayoutHandler, /** Call this when the recents animation during split-screen finishes. */ public void onRecentsInSplitAnimationFinish(WindowContainerTransaction finishWct, - SurfaceControl.Transaction finishT) { + SurfaceControl.Transaction finishT, TransitionInfo info) { // Check if the recent transition is finished by returning to the current // split, so we can restore the divider bar. for (int i = 0; i < finishWct.getHierarchyOps().size(); ++i) { @@ -2643,8 +2635,14 @@ public class StageCoordinator implements SplitLayout.SplitLayoutHandler, } } + // TODO(b/275664132): Remove dismissing split screen here to fit in back-to-split support. // Dismiss the split screen if it's not returning to split. prepareExitSplitScreen(STAGE_TYPE_UNDEFINED, finishWct); + for (TransitionInfo.Change change : info.getChanges()) { + if (change.getTaskInfo() != null && TransitionUtil.isClosingType(change.getMode())) { + finishT.setCrop(change.getLeash(), null).hide(change.getLeash()); + } + } setSplitsVisible(false); setDividerVisibility(false, finishT); logExit(EXIT_REASON_UNKNOWN); diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultMixedHandler.java b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultMixedHandler.java index 586cab07c5088..d52abf7951525 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultMixedHandler.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultMixedHandler.java @@ -187,17 +187,18 @@ public class DefaultMixedHandler implements Transitions.TransitionHandler, && isOpeningType(request.getType()) && request.getTriggerTask() != null && request.getTriggerTask().getWindowingMode() == WINDOWING_MODE_FULLSCREEN - && (request.getTriggerTask().getActivityType() == ACTIVITY_TYPE_HOME - || request.getTriggerTask().getActivityType() == ACTIVITY_TYPE_RECENTS) - && request.getRemoteTransition() != null) { - ProtoLog.v(ShellProtoLogGroup.WM_SHELL_TRANSITIONS, " Got a recents request while " + && request.getTriggerTask().getActivityType() == ACTIVITY_TYPE_HOME) { + ProtoLog.v(ShellProtoLogGroup.WM_SHELL_TRANSITIONS, " Got a going-home request while " + "Split-Screen is active, so treat it as Mixed."); Pair handler = mPlayer.dispatchRequest(transition, request, this); if (handler == null) { - android.util.Log.e(Transitions.TAG, " No handler for remote? This is unexpected" - + ", there should at-least be RemoteHandler."); - return null; + ProtoLog.v(ShellProtoLogGroup.WM_SHELL_TRANSITIONS, + " Lean on the remote transition handler to fetch a proper remote via" + + " TransitionFilter"); + handler = new Pair<>( + mPlayer.getRemoteTransitionHandler(), + new WindowContainerTransaction()); } final MixedTransition mixed = new MixedTransition( MixedTransition.TYPE_RECENTS_DURING_SPLIT, transition); @@ -515,7 +516,7 @@ public class DefaultMixedHandler implements Transitions.TransitionHandler, // If pair-to-pair switching, the post-recents clean-up isn't needed. if (mixed.mAnimType != MixedTransition.ANIM_TYPE_PAIR_TO_PAIR) { wct = wct != null ? wct : new WindowContainerTransaction(); - mSplitHandler.onRecentsInSplitAnimationFinish(wct, finishTransaction); + mSplitHandler.onRecentsInSplitAnimationFinish(wct, finishTransaction, info); } mSplitHandler.onTransitionAnimationComplete(); finishCallback.onTransitionFinished(wct, wctCB); diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java index beabf1842f8d3..4284993a54482 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java @@ -324,6 +324,10 @@ public class Transitions implements RemoteCallable { mRemoteTransitionHandler.removeFiltered(remoteTransition); } + RemoteTransitionHandler getRemoteTransitionHandler() { + return mRemoteTransitionHandler; + } + /** Registers an observer on the lifecycle of transitions. */ public void registerObserver(@NonNull TransitionObserver observer) { mObservers.add(observer); diff --git a/libs/WindowManager/Shell/tests/flicker/src/com/android/wm/shell/flicker/splitscreen/DismissSplitScreenByGoHome.kt b/libs/WindowManager/Shell/tests/flicker/src/com/android/wm/shell/flicker/splitscreen/DismissSplitScreenByGoHome.kt index bd2ffc1a018d7..2e81b30d2e9ad 100644 --- a/libs/WindowManager/Shell/tests/flicker/src/com/android/wm/shell/flicker/splitscreen/DismissSplitScreenByGoHome.kt +++ b/libs/WindowManager/Shell/tests/flicker/src/com/android/wm/shell/flicker/splitscreen/DismissSplitScreenByGoHome.kt @@ -71,7 +71,7 @@ class DismissSplitScreenByGoHome(flicker: FlickerTest) : SplitScreenBase(flicker // TODO(b/245472831): Move back to presubmit after shell transitions landing. @FlakyTest(bugId = 245472831) @Test - fun secondaryAppLayerBecomesInvisible() = flicker.layerBecomesInvisible(primaryApp) + fun secondaryAppLayerBecomesInvisible() = flicker.layerBecomesInvisible(secondaryApp) // TODO(b/245472831): Move back to presubmit after shell transitions landing. @FlakyTest(bugId = 245472831) diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/splitscreen/SplitTransitionTests.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/splitscreen/SplitTransitionTests.java index df78d92a90c8c..1b291468d6dc2 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/splitscreen/SplitTransitionTests.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/splitscreen/SplitTransitionTests.java @@ -278,7 +278,7 @@ public class SplitTransitionTests extends ShellTestCase { // Make sure it cleans-up if recents doesn't restore WindowContainerTransaction commitWCT = new WindowContainerTransaction(); mStageCoordinator.onRecentsInSplitAnimationFinish(commitWCT, - mock(SurfaceControl.Transaction.class)); + mock(SurfaceControl.Transaction.class), mock(TransitionInfo.class)); assertFalse(mStageCoordinator.isSplitScreenVisible()); } @@ -317,7 +317,7 @@ public class SplitTransitionTests extends ShellTestCase { mMainStage.onTaskAppeared(mMainChild, mock(SurfaceControl.class)); mSideStage.onTaskAppeared(mSideChild, mock(SurfaceControl.class)); mStageCoordinator.onRecentsInSplitAnimationFinish(restoreWCT, - mock(SurfaceControl.Transaction.class)); + mock(SurfaceControl.Transaction.class), mock(TransitionInfo.class)); assertTrue(mStageCoordinator.isSplitScreenVisible()); }