From 70c391aa073135cab66adbb44aaef8166ec64622 Mon Sep 17 00:00:00 2001 From: Gustav Sennton Date: Fri, 21 Feb 2025 09:16:46 +0000 Subject: [1/N] Cancel DragToDesktop on incoming transition if bookend requested The DragToDesktop transition is a book-end transition, meaning it starts a transition (start-drag), and then continues running until we receive a merge-request from a set of available (book-end) transitions (cancel-drag, and end-drag). Book-end transitions can cause deadlocks when an unknown transition is ready before the book-end transition. This happens if we do not handle the merge-request from the unknown transition - since that transition will then block the transition queue (including the book-end transition), and the overall transition will never finish. To avoid such deadlocks we here cancel the DragToDesktop transition if we receive a merge-request from an unknown transition (before the merge-request for our own book-end transition). Out of the cases below, we only handle case nr.2 in this CL (case nr.3 should be handled in the same way before and after this CL). Here BOOKEND means END or CANCEL): 1. START -> UNKNOWN -> finish START, finish UNKNOWN 2. START -> request BOOKEND -> UNKNOWN -> finish START, finish UNKNOWN, start + finish BOOKEND - the BOOKEND transition here runs on its own (it can't be merged into START) - the user is taken to the result of BOOKEND (fullscreen/split for CANCEL, desktop for END) 3. START -> request + merge BOOKEND -> UNKNOWN -> finish BOOKEND, finish UNKNOWN - here we ignore the UNKNOWN transition since we know BOOKEND will finish once its animation is done. - the user is taken to the result of BOOKEND (fullscreen/split for CANCEL, desktop for END) Test: launch a half-transparent activity during drag-to-desktop Bug: 397135730 Bug: 397497184 Flag: com.android.window.flags.enable_drag_to_desktop_incoming_transitions_bugfix Change-Id: Ied74b9f29bd93bc7d77872a67b73ffa23185c983 --- .../wm/shell/desktopmode/DesktopTasksController.kt | 4 + .../desktopmode/DragToDesktopTransitionHandler.kt | 150 ++++++++++++++++++++- .../DragToDesktopTransitionHandlerTest.kt | 143 ++++++++++++++++++-- 3 files changed, 281 insertions(+), 16 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/DesktopTasksController.kt b/libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/DesktopTasksController.kt index 301b79afd537..8279abd66f14 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/DesktopTasksController.kt +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/DesktopTasksController.kt @@ -235,6 +235,10 @@ class DesktopTasksController( removeVisualIndicator() } + override fun onTransitionInterrupted() { + removeVisualIndicator() + } + private fun removeVisualIndicator() { visualIndicator?.fadeOutIndicator { releaseVisualIndicator() } } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/DragToDesktopTransitionHandler.kt b/libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/DragToDesktopTransitionHandler.kt index e943c42dcdfc..f28ca3e8a957 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/DragToDesktopTransitionHandler.kt +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/DragToDesktopTransitionHandler.kt @@ -24,8 +24,10 @@ import android.os.SystemClock import android.os.SystemProperties import android.os.UserHandle import android.view.SurfaceControl +import android.view.SurfaceControl.Transaction import android.view.WindowManager.TRANSIT_CLOSE import android.window.DesktopModeFlags +import android.window.DesktopModeFlags.ENABLE_DRAG_TO_DESKTOP_INCOMING_TRANSITIONS_BUGFIX import android.window.TransitionInfo import android.window.TransitionInfo.Change import android.window.TransitionRequestInfo @@ -185,18 +187,29 @@ sealed class DragToDesktopTransitionHandler( */ fun finishDragToDesktopTransition(wct: WindowContainerTransaction): IBinder? { if (!inProgress) { + logV("finishDragToDesktop: not in progress, returning") // Don't attempt to finish a drag to desktop transition since there is no transition in // progress which means that the drag to desktop transition was never successfully // started. return null } - if (requireTransitionState().startAborted) { + val state = requireTransitionState() + if (state.startAborted) { + logV("finishDragToDesktop: start was aborted, clearing state") // Don't attempt to complete the drag-to-desktop since the start transition didn't // succeed as expected. Just reset the state as if nothing happened. clearState() return null } - return transitions.startTransition(TRANSIT_DESKTOP_MODE_END_DRAG_TO_DESKTOP, wct, this) + if (state.startInterrupted) { + logV("finishDragToDesktop: start was interrupted, returning") + // We should only have interrupted the start transition after receiving a cancel/end + // request, let that existing request play out and just return here. + return null + } + state.endTransitionToken = + transitions.startTransition(TRANSIT_DESKTOP_MODE_END_DRAG_TO_DESKTOP, wct, this) + return state.endTransitionToken } /** @@ -220,6 +233,11 @@ sealed class DragToDesktopTransitionHandler( clearState() return } + if (state.startInterrupted) { + // We should only have interrupted the start transition after receiving a cancel/end + // request, let that existing request play out and just return here. + return + } state.cancelState = cancelState if (state.draggedTaskChange != null && cancelState == CancelState.STANDARD_CANCEL) { @@ -227,7 +245,7 @@ sealed class DragToDesktopTransitionHandler( // transient to start and merge. Animate the cancellation (scale back to original // bounds) first before actually starting the cancel transition so that the wallpaper // is visible behind the animating task. - startCancelAnimation() + state.activeCancelAnimation = startCancelAnimation() } else if ( state.draggedTaskChange != null && (cancelState == CancelState.CANCEL_SPLIT_LEFT || @@ -255,7 +273,7 @@ sealed class DragToDesktopTransitionHandler( ) { if (bubbleController.isEmpty || state !is TransitionState.FromFullscreen) { // TODO(b/388853233): add support for dragging split task to bubble - startCancelAnimation() + state.activeCancelAnimation = startCancelAnimation() } else { // Animation is handled by BubbleController val wct = WindowContainerTransaction() @@ -355,6 +373,19 @@ sealed class DragToDesktopTransitionHandler( ): Boolean { val state = requireTransitionState() + if ( + handleCancelOrExitAfterInterrupt( + transition, + info, + startTransaction, + finishTransaction, + finishCallback, + state, + ) + ) { + return true + } + val isStartDragToDesktop = info.type == TRANSIT_DESKTOP_MODE_START_DRAG_TO_DESKTOP && transition == state.startTransitionToken @@ -537,6 +568,58 @@ sealed class DragToDesktopTransitionHandler( } } + private fun handleCancelOrExitAfterInterrupt( + transition: IBinder, + info: TransitionInfo, + startTransaction: Transaction, + finishTransaction: Transaction, + finishCallback: Transitions.TransitionFinishCallback, + state: TransitionState, + ): Boolean { + if (!ENABLE_DRAG_TO_DESKTOP_INCOMING_TRANSITIONS_BUGFIX.isTrue) { + return false + } + val isCancelDragToDesktop = + info.type == TRANSIT_DESKTOP_MODE_CANCEL_DRAG_TO_DESKTOP && + transition == state.cancelTransitionToken + val isEndDragToDesktop = + info.type == TRANSIT_DESKTOP_MODE_END_DRAG_TO_DESKTOP && + transition == state.endTransitionToken + // We should only receive cancel or end transitions through startAnimation() if the + // start transition was interrupted while a cancel- or end-transition had already + // been requested. Finish the cancel/end transition to avoid having to deal with more + // incoming transitions, and clear the state for the next start-drag transition. + if (!isCancelDragToDesktop && !isEndDragToDesktop) { + return false + } + if (!state.startInterrupted) { + logW( + "Not interrupted, but received startAnimation for cancel/end drag." + + "isCancel=$isCancelDragToDesktop, isEnd=$isEndDragToDesktop" + ) + return false + } + logV( + "startAnimation: interrupted -> " + + "isCancel=$isCancelDragToDesktop, isEnd=$isEndDragToDesktop" + ) + if (isEndDragToDesktop) { + setupEndDragToDesktop(info, startTransaction, finishTransaction) + animateEndDragToDesktop(startTransaction = startTransaction, finishCallback) + } else { // isCancelDragToDesktop + // Similar to when we merge the cancel transition: ensure all tasks involved in the + // cancel transition are shown, and finish the transition immediately. + info.changes.forEach { change -> + startTransaction.show(change.leash) + finishTransaction.show(change.leash) + } + } + startTransaction.apply() + finishCallback.onTransitionFinished(/* wct= */ null) + clearState() + return true + } + /** * Calculates start drag to desktop layers for transition [info]. The leash layer is calculated * based on its change position in the transition, e.g. `appLayer = appLayers - i`, where i is @@ -588,6 +671,7 @@ sealed class DragToDesktopTransitionHandler( ?: error("Start transition expected to be waiting for merge but wasn't") if (isEndTransition) { logV("mergeAnimation: end-transition, target=$mergeTarget") + state.mergedEndTransition = true setupEndDragToDesktop( info, startTransaction = startT, @@ -615,6 +699,41 @@ sealed class DragToDesktopTransitionHandler( return } logW("unhandled merge transition: transitionInfo=$info") + // Handle unknown incoming transitions by finishing the start transition. For now, only do + // this if we've already requested a cancel- or end transition. If we've already merged the + // end-transition, or if the end-transition is running on its own, then just wait until that + // finishes instead. If we've merged the cancel-transition we've finished the + // start-transition and won't reach this code. + if ( + mergeTarget == state.startTransitionToken && + isCancelOrEndTransitionRequested(state) && + !state.mergedEndTransition + ) { + interruptStartTransition(state) + } + } + + private fun isCancelOrEndTransitionRequested(state: TransitionState): Boolean = + state.cancelTransitionToken != null || state.endTransitionToken != null + + private fun interruptStartTransition(state: TransitionState) { + if (!ENABLE_DRAG_TO_DESKTOP_INCOMING_TRANSITIONS_BUGFIX.isTrue) { + return + } + logV("interruptStartTransition") + state.startTransitionFinishCb?.onTransitionFinished(/* wct= */ null) + state.dragAnimator.cancelAnimator() + state.activeCancelAnimation?.removeAllListeners() + state.activeCancelAnimation?.cancel() + state.activeCancelAnimation = null + // Keep the transition state so we can deal with Cancel/End properly in #startAnimation. + state.startInterrupted = true + dragToDesktopStateListener?.onTransitionInterrupted() + // Cancel CUJs here as they won't be accurate now that an incoming transition is playing. + interactionJankMonitor.cancel(CUJ_DESKTOP_MODE_ENTER_APP_HANDLE_DRAG_HOLD) + interactionJankMonitor.cancel(CUJ_DESKTOP_MODE_ENTER_APP_HANDLE_DRAG_RELEASE) + LatencyTracker.getInstance(context) + .onActionCancel(LatencyTracker.ACTION_DESKTOP_MODE_ENTER_APP_HANDLE_DRAG) } protected open fun setupEndDragToDesktop( @@ -781,7 +900,7 @@ sealed class DragToDesktopTransitionHandler( } ?: false } - private fun startCancelAnimation() { + private fun startCancelAnimation(): Animator { val state = requireTransitionState() val dragToDesktopAnimator = state.dragAnimator @@ -798,7 +917,7 @@ sealed class DragToDesktopTransitionHandler( val dx = targetX - x val dy = targetY - y val tx: SurfaceControl.Transaction = transactionSupplier.get() - ValueAnimator.ofFloat(DRAG_FREEFORM_SCALE, 1f) + return ValueAnimator.ofFloat(DRAG_FREEFORM_SCALE, 1f) .setDuration(DRAG_TO_DESKTOP_FINISH_ANIM_DURATION_MS) .apply { addUpdateListener { animator -> @@ -816,6 +935,7 @@ sealed class DragToDesktopTransitionHandler( addListener( object : AnimatorListenerAdapter() { override fun onAnimationEnd(animation: Animator) { + state.activeCancelAnimation = null dragToDesktopStateListener?.onCancelToDesktopAnimationEnd() // Start the cancel transition to restore order. startCancelDragToDesktopTransition() @@ -908,10 +1028,16 @@ sealed class DragToDesktopTransitionHandler( val dragLayer: Int, ) + /** Listener for various events happening during the DragToDesktop transition. */ interface DragToDesktopStateListener { + /** Indicates that the animation into Desktop has started. */ fun onCommitToDesktopAnimationStart() + /** Called when the animation to cancel the desktop-drag has finished. */ fun onCancelToDesktopAnimationEnd() + + /** Indicates that the drag-to-desktop transition has been interrupted. */ + fun onTransitionInterrupted() } sealed class TransitionState { @@ -928,6 +1054,10 @@ sealed class DragToDesktopTransitionHandler( abstract var cancelState: CancelState abstract var startAborted: Boolean abstract val visualIndicator: DesktopModeVisualIndicator? + abstract var startInterrupted: Boolean + abstract var endTransitionToken: IBinder? + abstract var mergedEndTransition: Boolean + abstract var activeCancelAnimation: Animator? data class FromFullscreen( override val draggedTaskId: Int, @@ -943,6 +1073,10 @@ sealed class DragToDesktopTransitionHandler( override var cancelState: CancelState = CancelState.NO_CANCEL, override var startAborted: Boolean = false, override val visualIndicator: DesktopModeVisualIndicator?, + override var startInterrupted: Boolean = false, + override var endTransitionToken: IBinder? = null, + override var mergedEndTransition: Boolean = false, + override var activeCancelAnimation: Animator? = null, var otherRootChanges: MutableList = mutableListOf(), ) : TransitionState() @@ -960,6 +1094,10 @@ sealed class DragToDesktopTransitionHandler( override var cancelState: CancelState = CancelState.NO_CANCEL, override var startAborted: Boolean = false, override val visualIndicator: DesktopModeVisualIndicator?, + override var startInterrupted: Boolean = false, + override var endTransitionToken: IBinder? = null, + override var mergedEndTransition: Boolean = false, + override var activeCancelAnimation: Animator? = null, var splitRootChange: Change? = null, var otherSplitTask: Int, ) : TransitionState() diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/desktopmode/DragToDesktopTransitionHandlerTest.kt b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/desktopmode/DragToDesktopTransitionHandlerTest.kt index 0871d38ceb46..6e7adf368155 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/desktopmode/DragToDesktopTransitionHandlerTest.kt +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/desktopmode/DragToDesktopTransitionHandlerTest.kt @@ -26,6 +26,7 @@ import com.android.internal.jank.Cuj.CUJ_DESKTOP_MODE_ENTER_APP_HANDLE_DRAG_HOLD import com.android.internal.jank.Cuj.CUJ_DESKTOP_MODE_ENTER_APP_HANDLE_DRAG_RELEASE import com.android.internal.jank.InteractionJankMonitor import com.android.window.flags.Flags +import com.android.window.flags.Flags.FLAG_ENABLE_DRAG_TO_DESKTOP_INCOMING_TRANSITIONS_BUGFIX import com.android.wm.shell.RootTaskDisplayAreaOrganizer import com.android.wm.shell.ShellTestCase import com.android.wm.shell.TestRunningTaskInfoBuilder @@ -34,6 +35,7 @@ import com.android.wm.shell.bubbles.BubbleTransitions import com.android.wm.shell.desktopmode.DesktopModeTransitionTypes.TRANSIT_DESKTOP_MODE_CANCEL_DRAG_TO_DESKTOP import com.android.wm.shell.desktopmode.DesktopModeTransitionTypes.TRANSIT_DESKTOP_MODE_END_DRAG_TO_DESKTOP import com.android.wm.shell.desktopmode.DesktopModeTransitionTypes.TRANSIT_DESKTOP_MODE_START_DRAG_TO_DESKTOP +import com.android.wm.shell.desktopmode.DragToDesktopTransitionHandler.CancelState import com.android.wm.shell.desktopmode.DragToDesktopTransitionHandler.Companion.DRAG_TO_DESKTOP_FINISH_ANIM_DURATION_MS import com.android.wm.shell.shared.split.SplitScreenConstants.SPLIT_POSITION_BOTTOM_OR_RIGHT import com.android.wm.shell.shared.split.SplitScreenConstants.SPLIT_POSITION_TOP_OR_LEFT @@ -56,6 +58,7 @@ import org.mockito.ArgumentMatchers.anyInt import org.mockito.ArgumentMatchers.eq import org.mockito.Mock import org.mockito.MockitoSession +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argThat import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -118,6 +121,14 @@ class DragToDesktopTransitionHandlerTest : ShellTestCase() { .strictness(Strictness.LENIENT) .mockStatic(SystemProperties::class.java) .startMocking() + whenever( + transitions.startTransition( + eq(TRANSIT_DESKTOP_MODE_END_DRAG_TO_DESKTOP), + /* wct= */ any(), + eq(defaultHandler), + ) + ) + .thenReturn(mock()) } @After @@ -679,17 +690,11 @@ class DragToDesktopTransitionHandlerTest : ShellTestCase() { val startTransition = startDrag(defaultHandler, task) val endTransition = mock() defaultHandler.onTaskResizeAnimationListener = mock() - defaultHandler.mergeAnimation( + mergeAnimation( transition = endTransition, - info = - createTransitionInfo( - type = TRANSIT_DESKTOP_MODE_END_DRAG_TO_DESKTOP, - draggedTask = task, - ), - startT = mock(), - finishT = mock(), + type = TRANSIT_DESKTOP_MODE_END_DRAG_TO_DESKTOP, + task = task, mergeTarget = startTransition, - finishCallback = mock(), ) defaultHandler.onTransitionConsumed(endTransition, aborted = true, mock()) @@ -700,6 +705,123 @@ class DragToDesktopTransitionHandlerTest : ShellTestCase() { .cancel(eq(CUJ_DESKTOP_MODE_ENTER_APP_HANDLE_DRAG_HOLD)) } + @Test + @EnableFlags(FLAG_ENABLE_DRAG_TO_DESKTOP_INCOMING_TRANSITIONS_BUGFIX) + fun mergeOtherTransition_cancelAndEndNotYetRequested_doesntInterruptsStartDrag() { + val finishCallback = mock() + val task = createTask() + defaultHandler.onTaskResizeAnimationListener = mock() + val startTransition = startDrag(defaultHandler, task, finishCallback = finishCallback) + + mergeInterruptingTransition(mergeTarget = startTransition) + + verify(finishCallback, never()).onTransitionFinished(anyOrNull()) + verify(dragAnimator, never()).cancelAnimator() + } + + @Test + @EnableFlags(FLAG_ENABLE_DRAG_TO_DESKTOP_INCOMING_TRANSITIONS_BUGFIX) + fun mergeOtherTransition_endDragAlreadyMerged_doesNotInterruptStartDrag() { + val startDragFinishCallback = mock() + val task = createTask() + val startTransition = + startDrag(defaultHandler, task, finishCallback = startDragFinishCallback) + defaultHandler.onTaskResizeAnimationListener = mock() + mergeAnimation( + type = TRANSIT_DESKTOP_MODE_END_DRAG_TO_DESKTOP, + task = task, + mergeTarget = startTransition, + ) + + mergeInterruptingTransition(mergeTarget = startTransition) + + verify(startDragFinishCallback, never()).onTransitionFinished(anyOrNull()) + } + + @Test + @EnableFlags(FLAG_ENABLE_DRAG_TO_DESKTOP_INCOMING_TRANSITIONS_BUGFIX) + fun startEndAnimation_otherTransitionInterruptedStartAfterEndRequest_finishImmediately() { + val task1 = createTask() + val startTransition = startDrag(defaultHandler, task1) + val endTransition = + defaultHandler.finishDragToDesktopTransition(WindowContainerTransaction()) + val startTransaction = mock() + val endDragFinishCallback = mock() + defaultHandler.onTaskResizeAnimationListener = mock() + mergeInterruptingTransition(mergeTarget = startTransition) + + val didAnimate = + defaultHandler.startAnimation( + transition = requireNotNull(endTransition), + info = + createTransitionInfo( + type = TRANSIT_DESKTOP_MODE_END_DRAG_TO_DESKTOP, + draggedTask = task1, + ), + startTransaction = startTransaction, + finishTransaction = mock(), + finishCallback = endDragFinishCallback, + ) + + assertThat(didAnimate).isTrue() + verify(startTransaction).apply() + verify(endDragFinishCallback).onTransitionFinished(anyOrNull()) + } + + @Test + @EnableFlags(FLAG_ENABLE_DRAG_TO_DESKTOP_INCOMING_TRANSITIONS_BUGFIX) + fun startDrag_otherTransitionInterruptedStartAfterEndRequested_animatesDragWhenReady() { + val task1 = createTask() + val startTransition = startDrag(defaultHandler, task1) + verify(dragAnimator).startAnimation() + val endTransition = + defaultHandler.finishDragToDesktopTransition(WindowContainerTransaction()) + defaultHandler.onTaskResizeAnimationListener = mock() + mergeInterruptingTransition(mergeTarget = startTransition) + defaultHandler.startAnimation( + transition = requireNotNull(endTransition), + info = + createTransitionInfo( + type = TRANSIT_DESKTOP_MODE_END_DRAG_TO_DESKTOP, + draggedTask = task1, + ), + startTransaction = mock(), + finishTransaction = mock(), + finishCallback = mock(), + ) + + startDrag(defaultHandler, createTask()) + + verify(dragAnimator, times(2)).startAnimation() + } + + private fun mergeInterruptingTransition(mergeTarget: IBinder) { + defaultHandler.mergeAnimation( + transition = mock(), + info = createTransitionInfo(type = TRANSIT_OPEN, draggedTask = createTask()), + startT = mock(), + finishT = mock(), + mergeTarget = mergeTarget, + finishCallback = mock(), + ) + } + + private fun mergeAnimation( + transition: IBinder = mock(), + type: Int, + mergeTarget: IBinder, + task: RunningTaskInfo, + ) { + defaultHandler.mergeAnimation( + transition = transition, + info = createTransitionInfo(type = type, draggedTask = task), + startT = mock(), + finishT = mock(), + mergeTarget = mergeTarget, + finishCallback = mock(), + ) + } + @Test fun getAnimationFraction_returnsFraction() { val fraction = @@ -785,6 +907,7 @@ class DragToDesktopTransitionHandlerTest : ShellTestCase() { finishTransaction: SurfaceControl.Transaction = mock(), homeChange: TransitionInfo.Change? = createHomeChange(), transitionRootLeash: SurfaceControl = mock(), + finishCallback: Transitions.TransitionFinishCallback = mock(), ): IBinder { whenever(dragAnimator.position).thenReturn(PointF()) // Simulate transition is started and is ready to animate. @@ -800,7 +923,7 @@ class DragToDesktopTransitionHandlerTest : ShellTestCase() { ), startTransaction = startTransaction, finishTransaction = finishTransaction, - finishCallback = {}, + finishCallback = finishCallback, ) return transition } -- cgit v1.2.3-59-g8ed1b