From 76b50b76c7ee607bcf0877725069c440a80ffb5a Mon Sep 17 00:00:00 2001 From: Tony Huang Date: Wed, 10 May 2023 08:44:30 +0000 Subject: Fix crash due to null leash on transition When dismiss then quick to recents case, divider leash might be released when dismiss transition finished but recents start animation later and it will get null leash transition change. Fix this by add a null check before it being added. Also remove tricky reparent implementation on finishWCT because it cause another issue on the above case. We need to find another more appropriate implementation and animation for dismiss transition, this will be done on later CL. Fix: 281605386 Test: manual Test: pass existing tests Change-Id: I56d017989482bf0a67c8a80a3569e9fc12c151d9 --- .../shell/splitscreen/SplitScreenTransitions.java | 29 ------------- .../wm/shell/splitscreen/StageCoordinator.java | 50 +++++----------------- .../splitscreen/DismissSplitScreenByDivider.kt | 10 ++--- .../wm/shell/splitscreen/SplitTransitionTests.java | 3 +- 4 files changed, 15 insertions(+), 77 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 a2af93fc42c6..ee36d528a1d0 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 @@ -23,8 +23,6 @@ import static android.view.WindowManager.TRANSIT_TO_BACK; import static android.view.WindowManager.TRANSIT_TO_FRONT; import static com.android.wm.shell.common.split.SplitScreenConstants.FLAG_IS_DIVIDER_BAR; -import static com.android.wm.shell.splitscreen.SplitScreen.STAGE_TYPE_MAIN; -import static com.android.wm.shell.splitscreen.SplitScreen.STAGE_TYPE_UNDEFINED; import static com.android.wm.shell.splitscreen.SplitScreen.stageTypeToString; import static com.android.wm.shell.splitscreen.SplitScreenController.EXIT_REASON_DRAG_DIVIDER; import static com.android.wm.shell.splitscreen.SplitScreenController.exitReasonToString; @@ -183,33 +181,6 @@ class SplitScreenTransitions { onFinish(null /* wct */, null /* wctCB */); } - void applyDismissTransition(@NonNull IBinder transition, @NonNull TransitionInfo info, - @NonNull SurfaceControl.Transaction startTransaction, - @NonNull SurfaceControl.Transaction finishTransaction, - @NonNull Transitions.TransitionFinishCallback finishCallback, - @NonNull WindowContainerToken topRoot, - @NonNull WindowContainerToken mainRoot, @NonNull WindowContainerToken sideRoot, - @NonNull SplitDecorManager mainDecor, @NonNull SplitDecorManager sideDecor) { - if (mPendingDismiss.mDismissTop != STAGE_TYPE_UNDEFINED) { - mFinishCallback = finishCallback; - mAnimatingTransition = transition; - mFinishTransaction = finishTransaction; - - startTransaction.apply(); - - final SplitDecorManager topDecor = mPendingDismiss.mDismissTop == STAGE_TYPE_MAIN - ? mainDecor : sideDecor; - topDecor.fadeOutDecor(() -> { - mTransitions.getMainExecutor().execute(() -> { - onFinish(null /* wct */, null /* wctCB */); - }); - }); - } else { - playAnimation(transition, info, startTransaction, finishTransaction, - finishCallback, mainRoot, sideRoot, topRoot); - } - } - void playResizeAnimation(@NonNull IBinder transition, @NonNull TransitionInfo info, @NonNull SurfaceControl.Transaction startTransaction, @NonNull SurfaceControl.Transaction finishTransaction, 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 749549d1ca55..fcea9ad420bd 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 @@ -311,6 +311,9 @@ public class StageCoordinator implements SplitLayout.SplitLayoutHandler, transitions.addHandler(this); mSplitUnsupportedToast = Toast.makeText(mContext, R.string.dock_non_resizeble_failed_to_dock_text, Toast.LENGTH_SHORT); + // With shell transition, we should update recents tile each callback so set this to true by + // default. + mShouldUpdateRecents = ENABLE_SHELL_TRANSITIONS; } @VisibleForTesting @@ -1464,18 +1467,8 @@ public class StageCoordinator implements SplitLayout.SplitLayoutHandler, private void prepareExitSplitScreen(@StageType int stageToTop, @NonNull WindowContainerTransaction wct) { if (!mMainStage.isActive()) return; - // Set the dismiss-to-top side to fullscreen for dismiss transition. - // Reparent the non-dismiss-to-top side to properly update its visibility. - if (stageToTop == STAGE_TYPE_MAIN) { - wct.setBounds(mMainStage.mRootTaskInfo.token, null /* bounds */); - mSideStage.removeAllTasks(wct, false /* toTop */); - } else if (stageToTop == STAGE_TYPE_SIDE) { - wct.setBounds(mSideStage.mRootTaskInfo.token, null /* bounds */); - mMainStage.deactivate(wct, false /* toTop */); - } else { - mSideStage.removeAllTasks(wct, false /* toTop */); - mMainStage.deactivate(wct, false /* toTop */); - } + mSideStage.removeAllTasks(wct, stageToTop == STAGE_TYPE_SIDE); + mMainStage.deactivate(wct, stageToTop == STAGE_TYPE_MAIN); wct.setReparentLeafTaskIfRelaunch(mRootTaskInfo.token, false /* reparentLeafTaskIfRelaunch */); } @@ -1554,7 +1547,6 @@ public class StageCoordinator implements SplitLayout.SplitLayoutHandler, updateSurfaceBounds(mSplitLayout, t, false /* applyResizingOffset */); t.show(mRootTaskLeash); setSplitsVisible(true); - mShouldUpdateRecents = true; updateRecentTasksSplitPair(); if (!mLogger.hasStartedSession()) { mLogger.logEnter(mSplitLayout.getDividerPositionAsFraction(), @@ -2565,13 +2557,6 @@ public class StageCoordinator implements SplitLayout.SplitLayoutHandler, } else if (mSplitTransitions.isPendingDismiss(transition)) { shouldAnimate = startPendingDismissAnimation( mSplitTransitions.mPendingDismiss, info, startTransaction, finishTransaction); - if (shouldAnimate) { - mSplitTransitions.applyDismissTransition(transition, info, - startTransaction, finishTransaction, finishCallback, mRootTaskInfo.token, - mMainStage.mRootTaskInfo.token, mSideStage.mRootTaskInfo.token, - mMainStage.getSplitDecorManager(), mSideStage.getSplitDecorManager()); - return true; - } } else if (mSplitTransitions.isPendingResize(transition)) { mSplitTransitions.playResizeAnimation(transition, info, startTransaction, finishTransaction, finishCallback, mMainStage.mRootTaskInfo.token, @@ -2760,8 +2745,7 @@ public class StageCoordinator implements SplitLayout.SplitLayoutHandler, mRecentTasks.ifPresent(recentTasks -> { // Notify recents if we are exiting in a way that breaks the pair, and disable further // updates to splits in the recents until we enter split again - if (shouldBreakPairedTaskInRecents(dismissReason) && mShouldUpdateRecents) { - if (toStage == STAGE_TYPE_UNDEFINED) { + if (shouldBreakPairedTaskInRecents(dismissReason)) { for (TransitionInfo.Change change : info.getChanges()) { final ActivityManager.RunningTaskInfo taskInfo = change.getTaskInfo(); if (taskInfo != null @@ -2769,13 +2753,8 @@ public class StageCoordinator implements SplitLayout.SplitLayoutHandler, recentTasks.removeSplitPair(taskInfo.taskId); } } - } else { - recentTasks.removeSplitPair(mMainStage.getTopVisibleChildTaskId()); - recentTasks.removeSplitPair(mSideStage.getTopVisibleChildTaskId()); - } } }); - mShouldUpdateRecents = false; mSplitRequest = null; // Update local states. @@ -2817,18 +2796,6 @@ public class StageCoordinator implements SplitLayout.SplitLayoutHandler, mSplitLayout.release(t); mSplitTransitions.mPendingDismiss = null; return false; - } else { - final @SplitScreen.StageType int dismissTop = dismissTransition.mDismissTop; - // Reparent all tasks after dismiss transition finished. - dismissTransition.setFinishedCallback( - new SplitScreenTransitions.TransitionFinishedCallback() { - @Override - public void onFinished(WindowContainerTransaction wct, - SurfaceControl.Transaction t) { - mSideStage.removeAllTasks(wct, dismissTop == STAGE_TYPE_SIDE); - mMainStage.deactivate(wct, dismissTop == STAGE_TYPE_MAIN); - } - }); } addDividerBarToTransition(info, false /* show */); @@ -2868,6 +2835,11 @@ public class StageCoordinator implements SplitLayout.SplitLayoutHandler, private void addDividerBarToTransition(@NonNull TransitionInfo info, boolean show) { final SurfaceControl leash = mSplitLayout.getDividerLeash(); + if (leash == null || !leash.isValid()) { + Slog.w(TAG, "addDividerBarToTransition but leash was released or not be created"); + return; + } + final TransitionInfo.Change barChange = new TransitionInfo.Change(null /* token */, leash); mSplitLayout.getRefDividerBounds(mTempRect1); barChange.setParent(mRootTaskInfo.token); diff --git a/libs/WindowManager/Shell/tests/flicker/src/com/android/wm/shell/flicker/splitscreen/DismissSplitScreenByDivider.kt b/libs/WindowManager/Shell/tests/flicker/src/com/android/wm/shell/flicker/splitscreen/DismissSplitScreenByDivider.kt index 3fd6d17f27cb..4505b9978b76 100644 --- a/libs/WindowManager/Shell/tests/flicker/src/com/android/wm/shell/flicker/splitscreen/DismissSplitScreenByDivider.kt +++ b/libs/WindowManager/Shell/tests/flicker/src/com/android/wm/shell/flicker/splitscreen/DismissSplitScreenByDivider.kt @@ -81,13 +81,9 @@ class DismissSplitScreenByDivider(override val flicker: FlickerTest) : @Presubmit @Test fun secondaryAppBoundsIsFullscreenAtEnd() { - flicker.assertLayers { - this.isVisible(secondaryApp).then().isInvisible(secondaryApp).then().invoke( - "secondaryAppBoundsIsFullscreenAtEnd" - ) { - val displayBounds = WindowUtils.getDisplayBounds(flicker.scenario.endRotation) - it.visibleRegion(secondaryApp).coversExactly(displayBounds) - } + flicker.assertLayersEnd { + val displayBounds = WindowUtils.getDisplayBounds(flicker.scenario.endRotation) + visibleRegion(secondaryApp).coversExactly(displayBounds) } } 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 b76d2dcc6e1e..de701ec9410c 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 @@ -376,8 +376,7 @@ public class SplitTransitionTests extends ShellTestCase { IBinder transition = mock(IBinder.class); WindowContainerTransaction result = mStageCoordinator.handleRequest(transition, request); - // Don't reparent tasks until the animation is complete. - assertFalse(containsSplitExit(result)); + assertTrue(containsSplitExit(result)); // make sure we haven't made any local changes yet (need to wait until transition is ready) assertTrue(mStageCoordinator.isSplitScreenVisible()); -- cgit v1.2.3-59-g8ed1b