Merge "Fix ordering of finish transactions w/ bookends" into main (1efa2137) · Commits · e / os / android_frameworks_base

libs/WindowManager/Shell/src/com/android/wm/shell/docs/transitions.md

+16 −3

Original line number	Diff line number	Diff line
		@@ -73,13 +73,12 @@ When starting a transient-launch transition, there are several possible outcomes
		launch transition).

		#### Finishing the transient-launch transition

		When restoring the transient order in the 3rd flow above, it is recommended to do it in a new
		transition and <span style="color:orange">not</span> via the WindowContainerTransaction in
		`TransitionFinishCallback#onTransitionFinished()` provided when starting the transition.

		Changes to the window hierarchy via the finish transaction are not applied in sync with other
		transitions that are collecting and aplying, and are also not observable in Shell in any way.
		transitions that are collecting and applying, and are also not observable in Shell in any way.
		Starting a new transition instead ensures both. (The finish transaction can still be used if there
		are non-transition affecting properties (ie. container properties) that need to be updated as a part
		of finishing the transient-launch transition).
		@@ -116,3 +115,17 @@ This means that when using transient-launch transitions with a bookend transitio
		ever queued (or already posted) after it. You can do so by preempting the bookend transition
		(finishing the transient-launch transition), or handling the merge of the new transition (so it
		doesn't queue).

		#### Finish-transactions with bookend transitions
		Another thing to keep in mind when using bookend transitions is the ordering of the finish \
		transactions. In particular, if cleanup of tasks is necessary, then you need to ensure that the
		cleanup happens on the bookend's finish transaction and not the finish transaction provided when
		starting the animation, otherwise properties on the tasks in the transition could be overwritten by
		the default finish transaction created by Core (see `Transition#buildFinishTransaction()`).

		ie.
		1) Start START (Core builds finishT1 to reset state)
		2) Start BOOKEND (Core builds finishT2 to reset state)
		3) Merge BOOKEND into START
		4) <<< Finish transaction properties should be updated here and not before 2)
		5) Finish BOOKEND (Shell transitions applies finishT1 then finishT2 in order)

libs/WindowManager/Shell/src/com/android/wm/shell/recents/RecentsTransitionHandler.java

+31 −1

Original line number	Diff line number	Diff line
		@@ -88,6 +88,7 @@ import com.android.wm.shell.transition.Transitions;

		import java.util.ArrayList;
		import java.util.function.Consumer;
		import java.util.function.Supplier;

		/**
		* Handles the Recents (overview) animation. Only one of these can run at a time. A recents
		@@ -108,6 +109,7 @@ public class RecentsTransitionHandler implements Transitions.TransitionHandler,
		private IApplicationThread mAnimApp = null;
		private final ArrayList<RecentsController> mControllers = new ArrayList<>();
		private final ArrayList<RecentsTransitionStateListener> mStateListeners = new ArrayList<>();
		private Supplier<SurfaceControl.Transaction> mFinishTransactionSupplier = null;

		/**
		* List of other handlers which might need to mix recents with other things. These are checked
		@@ -164,6 +166,16 @@ public class RecentsTransitionHandler implements Transitions.TransitionHandler,
		mBackgroundColor = color;
		}

		/**
		* Used for testing to provide a supplier for the transaction used in
		* RecentsController#finishInner() which later gets merged into the final finish transaction.
		*/
		@VisibleForTesting
		public void setFinishTransactionSupplier(
		Supplier<SurfaceControl.Transaction> finishTransactionSupplier) {
		mFinishTransactionSupplier = finishTransactionSupplier;
		}

		/**
		* Starts a new real/synthetic recents transition.
		*/
		@@ -413,6 +425,8 @@ public class RecentsTransitionHandler implements Transitions.TransitionHandler,
		// enableRecentsBookendTransition() is enabled
		private IBinder mPendingFinishTransition;
		private IResultReceiver mPendingRunnerFinishCb;
		// This stores the pending finish transaction to merge with the actual finish transaction
		private SurfaceControl.Transaction mPendingFinishTransaction;

		RecentsController(IRecentsAnimationRunner listener) {
		mInstanceId = System.identityHashCode(this);
		@@ -558,6 +572,10 @@ public class RecentsTransitionHandler implements Transitions.TransitionHandler,
		mPipTransaction = null;
		mPendingRunnerFinishCb = null;
		mPendingFinishTransition = null;
		if (mPendingFinishTransaction != null) {
		mPendingFinishTransaction.close();
		}
		mPendingFinishTransaction = null;
		mControllers.remove(this);
		for (int i = 0; i < mStateListeners.size(); i++) {
		mStateListeners.get(i).onTransitionStateChanged(TRANSITION_STATE_NOT_RUNNING);
		@@ -1381,7 +1399,9 @@ public class RecentsTransitionHandler implements Transitions.TransitionHandler,
		mInstanceId, toHome, sendUserLeaveHint, mWillFinishToHome, mState,
		mPausingTasks != null, reason);

		final SurfaceControl.Transaction t = mFinishTransaction;
		final SurfaceControl.Transaction t = mFinishTransactionSupplier != null
		? mFinishTransactionSupplier.get()
		: new SurfaceControl.Transaction();
		final WindowContainerTransaction wct = new WindowContainerTransaction();

		// The following code must set this if it is changing anything in core that might affect
		@@ -1520,6 +1540,7 @@ public class RecentsTransitionHandler implements Transitions.TransitionHandler,
		// In this case, we've already started the PIP transition, so we can
		// clean up immediately
		mPendingRunnerFinishCb = runnerFinishCb;
		mPendingFinishTransaction = t;
		onFinishInner(null);
		return;
		}
		@@ -1540,6 +1561,7 @@ public class RecentsTransitionHandler implements Transitions.TransitionHandler,
		"[%d] RecentsController.finishInner: "
		+ "Queuing TRANSIT_END_RECENTS_TRANSITION", mInstanceId);
		mPendingRunnerFinishCb = runnerFinishCb;
		mPendingFinishTransaction = t;
		mPendingFinishTransition = mTransitions.startTransition(
		TRANSIT_END_RECENTS_TRANSITION, wct,
		new PendingFinishTransitionHandler());
		@@ -1548,15 +1570,18 @@ public class RecentsTransitionHandler implements Transitions.TransitionHandler,
		"[%d] RecentsController.finishInner: Non-transition affecting wct",
		mInstanceId);
		mPendingRunnerFinishCb = runnerFinishCb;
		mPendingFinishTransaction = t;
		onFinishInner(wct);
		}
		} else {
		// If there's no work to do, just go ahead and clean up
		mPendingRunnerFinishCb = runnerFinishCb;
		mPendingFinishTransaction = t;
		onFinishInner(null /* wct */);
		}
		} else {
		mPendingRunnerFinishCb = runnerFinishCb;
		mPendingFinishTransaction = t;
		onFinishInner(wct);
		}
		}
		@@ -1570,6 +1595,11 @@ public class RecentsTransitionHandler implements Transitions.TransitionHandler,
		final Transitions.TransitionFinishCallback finishCb = mFinishCB;
		final IResultReceiver runnerFinishCb = mPendingRunnerFinishCb;

		// We merge the cleanup transaction (prepared in finishInner()) with the last accepted
		// finish transaction to ensure that it's applied after the default finish transaction
		// created for the bookend transition (if it's used)
		mFinishTransaction.merge(mPendingFinishTransaction);

		cleanUp();
		finishCb.onTransitionFinished(wct);
		if (runnerFinishCb != null) {

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/recents/RecentsTransitionHandlerTest.java

+8 −3

Original line number	Diff line number	Diff line
		@@ -121,8 +121,6 @@ public class RecentsTransitionHandlerTest extends ShellTestCase {
		@Mock
		private DisplayInsetsController mDisplayInsetsController;
		@Mock
		private IRecentTasksListener mRecentTasksListener;
		@Mock
		private TaskStackTransitionObserver mTaskStackTransitionObserver;
		@Mock
		private Transitions mTransitions;
		@@ -169,6 +167,10 @@ public class RecentsTransitionHandlerTest extends ShellTestCase {
		doReturn(mMainExecutor).when(mTransitions).getMainExecutor();
		mRecentsTransitionHandler = new RecentsTransitionHandler(mShellInit, mShellTaskOrganizer,
		mTransitions, mRecentTasksController, mock(HomeTransitionObserver.class));
		// By default use a mock finish transaction since we are sending transitions that don't have
		// real surface controls
		mRecentsTransitionHandler.setFinishTransactionSupplier(
		() -> mock(SurfaceControl.Transaction.class));

		mShellInit.init();
		}
		@@ -394,6 +396,7 @@ public class RecentsTransitionHandlerTest extends ShellTestCase {
		SurfaceControl leash = mergeTransitionInfo.getChanges().get(0).getLeash();
		final IBinder transition = startRecentsTransition(/* synthetic= */ false);
		SurfaceControl.Transaction finishT = mock(SurfaceControl.Transaction.class);
		mRecentsTransitionHandler.setFinishTransactionSupplier(() -> finishT);
		mRecentsTransitionHandler.startAnimation(
		transition, createTransitionInfo(), new StubTransaction(), new StubTransaction(),
		mock(Transitions.TransitionFinishCallback.class));
		@@ -421,8 +424,10 @@ public class RecentsTransitionHandlerTest extends ShellTestCase {
		SurfaceControl leash = transitionInfo.getChanges().get(0).getLeash();
		final IBinder transition = startRecentsTransition(/* synthetic= */ false);
		SurfaceControl.Transaction finishT = mock(SurfaceControl.Transaction.class);
		mRecentsTransitionHandler.setFinishTransactionSupplier(() -> finishT);
		mRecentsTransitionHandler.startAnimation(
		transition, transitionInfo, new StubTransaction(), finishT,
		transition, transitionInfo, new StubTransaction(),
		new StubTransaction(),
		mock(Transitions.TransitionFinishCallback.class));

		mRecentsTransitionHandler.findController(transition).finish(/* toHome= */ false,

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/transition/ShellTransitionTests.java

+2 −0

Original line number	Diff line number	Diff line
		@@ -1327,6 +1327,8 @@ public class ShellTransitionTests extends ShellTestCase {
		final RecentsTransitionHandler recentsHandler =
		new RecentsTransitionHandler(shellInit, mock(ShellTaskOrganizer.class), transitions,
		mockRecentsTaskController, mock(HomeTransitionObserver.class));
		recentsHandler.setFinishTransactionSupplier(
		() -> mock(SurfaceControl.Transaction.class));
		transitions.replaceDefaultHandlerForTest(mDefaultHandler);
		shellInit.init();