From 745db83b5e1231c069295a59b8889a6c38983ecf Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Sat, 18 Feb 2023 00:48:15 +0800 Subject: [PATCH] Handle animating exit windows with shell transition WindowState#isWinVisibleLw can only detect legacy transition. So when using shell transition, if an activity relayout to invisible before the transition is finished, its surface will be destroyed when animating, which looks bad. Since there were too may visibility methods, and isWinVisibleLw can be covered by isVisible()+isDisplayed() which is able to consider legacy and shell transition. So just delete it. Other usages of isWinVisibleLw: - shouldKeepVisibleDeadAppWindow: unused dead code - imeWindow: non-activity window so it equals to isVisible(). Besides, make shell transition have the same behavior as legacy transition to call onExitAnimationDone when the animation is finished to destroy the surfaces. Also simplify tryStartExitingAnimation: - Remove returned focusMayChange because it should be already done by visibility change. - Remove condition for wallpaper target because it was added for lockscreen (commit 6136b7e) 10 years ago, but notification shade (now it is lockscreen) was excluded for other issue in recent version. That means the original problem was gone. Fix: 247005789 Test: atest WindowManagerServiceTests#testRelayoutExitingWindow Change-Id: I2255fdad39560fd3dff589985687bea93a00a038 --- .../server/wm/TaskSnapshotController.java | 2 +- .../server/wm/TransitionController.java | 15 ++++++ .../server/wm/WindowManagerService.java | 52 +++++++------------ .../com/android/server/wm/WindowState.java | 16 ++---- .../server/wm/WindowManagerServiceTests.java | 31 +++++++---- 5 files changed, 60 insertions(+), 56 deletions(-) diff --git a/services/core/java/com/android/server/wm/TaskSnapshotController.java b/services/core/java/com/android/server/wm/TaskSnapshotController.java index 2037fdc103524..679f0f5920356 100644 --- a/services/core/java/com/android/server/wm/TaskSnapshotController.java +++ b/services/core/java/com/android/server/wm/TaskSnapshotController.java @@ -220,7 +220,7 @@ class TaskSnapshotController extends AbsAppSnapshotController mPlayingTransitions = new ArrayList<>(); + /** + * The windows that request to be invisible while it is in transition. After the transition + * is finished and the windows are no longer animating, their surfaces will be destroyed. + */ + final ArrayList mAnimatingExitWindows = new ArrayList<>(); + final Lock mRunningLock = new Lock(); private final IBinder.DeathRecipient mTransitionPlayerDeath; @@ -658,6 +664,15 @@ class TransitionController { mPlayingTransitions.remove(record); updateRunningRemoteAnimation(record, false /* isPlaying */); record.finishTransition(); + for (int i = mAnimatingExitWindows.size() - 1; i >= 0; i--) { + final WindowState w = mAnimatingExitWindows.get(i); + if (w.mAnimatingExit && w.mHasSurface && !w.inTransition()) { + w.onExitAnimationDone(); + } + if (!w.mAnimatingExit || !w.mHasSurface) { + mAnimatingExitWindows.remove(i); + } + } mRunningLock.doNotifyLocked(); } diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index d5bf9f9a516c6..841aa692276b4 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -2425,7 +2425,7 @@ public class WindowManagerService extends IWindowManager.Stub if (wallpaperMayMove) { displayContent.mWallpaperController.adjustWallpaperWindows(); } - focusMayChange = tryStartExitingAnimation(win, winAnimator, focusMayChange); + tryStartExitingAnimation(win, winAnimator); } } @@ -2610,8 +2610,7 @@ public class WindowManagerService extends IWindowManager.Stub } } - private boolean tryStartExitingAnimation(WindowState win, WindowStateAnimator winAnimator, - boolean focusMayChange) { + private void tryStartExitingAnimation(WindowState win, WindowStateAnimator winAnimator) { // Try starting an animation; if there isn't one, we // can destroy the surface right away. int transit = WindowManagerPolicy.TRANSIT_EXIT; @@ -2619,39 +2618,30 @@ public class WindowManagerService extends IWindowManager.Stub transit = WindowManagerPolicy.TRANSIT_PREVIEW_DONE; } - if (win.isWinVisibleLw() && win.mDisplayContent.okToAnimate()) { + if (win.isVisible() && win.isDisplayed() && win.mDisplayContent.okToAnimate()) { String reason = null; if (winAnimator.applyAnimationLocked(transit, false)) { // This is a WMCore-driven window animation. reason = "applyAnimation"; - focusMayChange = true; - win.mAnimatingExit = true; - } else if ( - // This is already animating via a WMCore-driven window animation - win.isSelfAnimating(0 /* flags */, ANIMATION_TYPE_WINDOW_ANIMATION) - // Or already animating as part of a legacy app-transition - || win.isAnimating(PARENTS | TRANSITION, - ANIMATION_TYPE_APP_TRANSITION | ANIMATION_TYPE_RECENTS) - // Or already animating as part of a shell-transition. - || (win.inTransition() - // Filter out non-app windows since transitions don't animate those - // (but may still "wait" on them for readiness) - && (win.mActivityRecord != null || win.mIsWallpaper))) { - // TODO(b/247005789): set mAnimatingExit somewhere in shell-transitions setup. - reason = "animating"; - win.mAnimatingExit = true; - } else if (win.mDisplayContent.mWallpaperController.isWallpaperTarget(win) - && win.mAttrs.type != TYPE_NOTIFICATION_SHADE) { - reason = "isWallpaperTarget"; - // If the wallpaper is currently behind this app window, they should be updated - // in a transaction to avoid artifacts. - // For NotificationShade, sysui is in charge of running window animation and it - // updates the client view visibility only after both NotificationShade and the - // wallpaper are hidden. So the exit animation is not needed and can destroy its - // surface immediately. - win.mAnimatingExit = true; + } else if (win.isSelfAnimating(0 /* flags */, ANIMATION_TYPE_WINDOW_ANIMATION)) { + // This is already animating via a WMCore-driven window animation. + reason = "selfAnimating"; + } else { + if (win.mTransitionController.isShellTransitionsEnabled()) { + // Already animating as part of a shell-transition. Currently this only handles + // activity window because other types should be WMCore-driven. + if ((win.mActivityRecord != null && win.mActivityRecord.inTransition())) { + win.mTransitionController.mAnimatingExitWindows.add(win); + reason = "inTransition"; + } + } else if (win.isAnimating(PARENTS | TRANSITION, + ANIMATION_TYPE_APP_TRANSITION | ANIMATION_TYPE_RECENTS)) { + // Already animating as part of a legacy app-transition. + reason = "inLegacyTransition"; + } } if (reason != null) { + win.mAnimatingExit = true; ProtoLog.d(WM_DEBUG_ANIM, "Set animatingExit: reason=startExitingAnimation/%s win=%s", reason, win); } @@ -2667,8 +2657,6 @@ public class WindowManagerService extends IWindowManager.Stub if (mAccessibilityController.hasCallbacks()) { mAccessibilityController.onWindowTransition(win, transit); } - - return focusMayChange; } private int createSurfaceControl(SurfaceControl outSurfaceControl, int result, diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 581939ae937a0..87e87b95fdfd5 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -1921,16 +1921,6 @@ class WindowState extends WindowContainer implements WindowManagerP return !isWallpaper || mToken.isVisibleRequested(); } - /** - * Is this window visible, ignoring its app token? It is not visible if there is no surface, - * or we are in the process of running an exit animation that will remove the surface. - */ - // TODO: Can we consolidate this with #isVisible() or have a more appropriate name for this? - boolean isWinVisibleLw() { - return (mActivityRecord == null || mActivityRecord.isVisibleRequested() - || mActivityRecord.isAnimating(TRANSITION | PARENTS)) && isVisible(); - } - /** * The same as isVisible(), but follows the current hidden state of the associated app token, * not the pending requested hidden state. @@ -2530,7 +2520,7 @@ class WindowState extends WindowContainer implements WindowManagerP } // If we are not currently running the exit animation, we need to see about starting one - wasVisible = isWinVisibleLw(); + wasVisible = isVisible(); if (keepVisibleDeadWindow) { ProtoLog.v(WM_DEBUG_ADD_REMOVE, @@ -2556,7 +2546,7 @@ class WindowState extends WindowContainer implements WindowManagerP // look weird if its orientation is changed. && !inRelaunchingActivity(); - if (wasVisible) { + if (wasVisible && isDisplayed()) { final int transit = (!startingWindow) ? TRANSIT_EXIT : TRANSIT_PREVIEW_DONE; // Try starting an animation. @@ -3111,7 +3101,7 @@ class WindowState extends WindowContainer implements WindowManagerP * interacts with it. */ private boolean shouldKeepVisibleDeadAppWindow() { - if (!isWinVisibleLw() || mActivityRecord == null || !mActivityRecord.isClientVisible()) { + if (!isVisible() || mActivityRecord == null || !mActivityRecord.isClientVisible()) { // Not a visible app window or the app isn't dead. return false; } diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowManagerServiceTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowManagerServiceTests.java index 4dd5de3b999c1..7d6cf4a63c43d 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowManagerServiceTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowManagerServiceTests.java @@ -49,12 +49,12 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -91,7 +91,6 @@ import com.android.compatibility.common.util.AdoptShellPermissionsRule; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; /** @@ -103,9 +102,6 @@ import org.junit.runner.RunWith; @RunWith(WindowTestRunner.class) public class WindowManagerServiceTests extends WindowTestsBase { - @Rule - public ExpectedException mExpectedException = ExpectedException.none(); - @Rule public AdoptShellPermissionsRule mAdoptShellPermissionsRule = new AdoptShellPermissionsRule( InstrumentationRegistry.getInstrumentation().getUiAutomation(), @@ -198,16 +194,20 @@ public class WindowManagerServiceTests extends WindowTestsBase { public void testRelayoutExitingWindow() { final WindowState win = createWindow(null, TYPE_BASE_APPLICATION, "appWin"); final WindowSurfaceController surfaceController = mock(WindowSurfaceController.class); - doReturn(true).when(surfaceController).hasSurface(); - spyOn(win); - doReturn(true).when(win).isAnimationRunningSelfOrParent(); win.mWinAnimator.mSurfaceController = surfaceController; + win.mWinAnimator.mDrawState = WindowStateAnimator.HAS_DRAWN; + doReturn(true).when(surfaceController).hasSurface(); + spyOn(win.mTransitionController); + doReturn(true).when(win.mTransitionController).isShellTransitionsEnabled(); + doReturn(true).when(win.mTransitionController).inTransition( + eq(win.mActivityRecord)); win.mViewVisibility = View.VISIBLE; win.mHasSurface = true; win.mActivityRecord.mAppStopped = true; - win.mActivityRecord.setVisibleRequested(false); - win.mActivityRecord.setVisible(false); mWm.mWindowMap.put(win.mClient.asBinder(), win); + spyOn(mWm.mWindowPlacerLocked); + // Skip unnecessary operations of relayout. + doNothing().when(mWm.mWindowPlacerLocked).performSurfacePlacement(anyBoolean()); final int w = 100; final int h = 200; final ClientWindowFrames outFrames = new ClientWindowFrames(); @@ -216,6 +216,17 @@ public class WindowManagerServiceTests extends WindowTestsBase { final InsetsState outInsetsState = new InsetsState(); final InsetsSourceControl.Array outControls = new InsetsSourceControl.Array(); final Bundle outBundle = new Bundle(); + mWm.relayoutWindow(win.mSession, win.mClient, win.mAttrs, w, h, View.GONE, 0, 0, 0, + outFrames, outConfig, outSurfaceControl, outInsetsState, outControls, outBundle); + // The window is in transition, so its destruction is deferred. + assertTrue(win.mAnimatingExit); + assertFalse(win.mDestroying); + assertTrue(win.mTransitionController.mAnimatingExitWindows.contains(win)); + + win.mAnimatingExit = false; + win.mViewVisibility = View.VISIBLE; + win.mActivityRecord.setVisibleRequested(false); + win.mActivityRecord.setVisible(false); mWm.relayoutWindow(win.mSession, win.mClient, win.mAttrs, w, h, View.GONE, 0, 0, 0, outFrames, outConfig, outSurfaceControl, outInsetsState, outControls, outBundle); // Because the window is already invisible, it doesn't need to apply exiting animation