From 50355ae4e4c4318ddd1df293f922ad79579bee7c Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Thu, 30 Jun 2022 23:32:04 +0800 Subject: [PATCH] Don't notify animation finished if no animation This removes commit ab9fcb2 to avoid potential recursion of surface placement: onAnimationFinished -> onExitAnimationDone -> destroySurfaces -> postWindowRemoveCleanupLocked -> performSurfacePlacementNoTrace. The original case no longer occurs without the invocation. And now the window with mRemoveOnExit=true will be removed by WindowState #handleCompleteDeferredRemoval. And if there is only mAnimatingExit, it will be handled by WS#cleanupAnimatingExitWindow. The test testStuckExitingWindow is also removed because no matter with calling onAnimationFinished or not in postApplyAnimation, the test always passes. Verified the original issue case: change language and swipe up. There is no leftover EXITING window. Even hardcode to set the exit flags unexpectedly, there is still no problem. Bug: 237481770 Test: WindowStateTests#testDeferredRemovalByAnimating WindowContainerTests#testHandleCompleteDeferredRemoval Change-Id: If10cd3c334476e078fdd4d4db05bf8b37db84ccc --- .../com/android/server/wm/ActivityRecord.java | 4 ---- .../android/server/wm/ActivityRecordTests.java | 17 +---------------- .../com/android/server/wm/WindowStateTests.java | 1 + 3 files changed, 2 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index 62427e1d8e523..8925799636229 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -5336,11 +5336,7 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A ANIMATION_TYPE_APP_TRANSITION | ANIMATION_TYPE_WINDOW_ANIMATION | ANIMATION_TYPE_RECENTS); if (!delayed) { - // We aren't delayed anything, but exiting windows rely on the animation finished - // callback being called in case the ActivityRecord was pretending to be delayed, - // which we might have done because we were in closing/opening apps list. if (!usingShellTransitions) { - onAnimationFinished(ANIMATION_TYPE_APP_TRANSITION, null /* AnimationAdapter */); if (visible) { // The token was made immediately visible, there will be no entrance animation. // We need to inform the client the enter animation was finished. diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java index 0c3b270518cfd..3f3d01a14f801 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java @@ -49,7 +49,6 @@ import static android.os.InputConstants.DEFAULT_DISPATCHING_TIMEOUT_MILLIS; import static android.os.Process.NOBODY_UID; import static android.view.Display.DEFAULT_DISPLAY; import static android.view.InsetsState.ITYPE_IME; -import static android.view.WindowManager.LayoutParams.FIRST_APPLICATION_WINDOW; import static android.view.WindowManager.LayoutParams.FIRST_SUB_WINDOW; import static android.view.WindowManager.LayoutParams.FLAG_ALT_FOCUSABLE_IM; import static android.view.WindowManager.LayoutParams.FLAG_DISMISS_KEYGUARD; @@ -2538,21 +2537,6 @@ public class ActivityRecordTests extends WindowTestsBase { || activity.containsDismissKeyguardWindow()); } - @Test - public void testStuckExitingWindow() { - final WindowState closingWindow = createWindow(null, FIRST_APPLICATION_WINDOW, - "closingWindow"); - closingWindow.mAnimatingExit = true; - closingWindow.mRemoveOnExit = true; - closingWindow.mActivityRecord.commitVisibility( - false /* visible */, true /* performLayout */); - - // We pretended that we were running an exit animation, but that should have been cleared up - // by changing visibility of ActivityRecord - closingWindow.removeIfPossible(); - assertTrue(closingWindow.mRemoved); - } - @Test public void testSetOrientation() { final ActivityRecord activity = new ActivityBuilder(mAtm).setCreateTask(true).build(); @@ -3149,6 +3133,7 @@ public class ActivityRecordTests extends WindowTestsBase { mDisplayContent.mOpeningApps.clear(); app.mActivityRecord.commitVisibility(false, false); app.mActivityRecord.onWindowsGone(); + mDisplayContent.computeImeTargetIfNeeded(app.mActivityRecord); assertTrue(app.mActivityRecord.mLastImeShown); assertTrue(app.mActivityRecord.mImeInsetsFrozenUntilStartInput); diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java index fb7400bc83ef6..355413f512408 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java @@ -992,6 +992,7 @@ public class WindowStateTests extends WindowTestsBase { assertTrue(app.mActivityRecord.mImeInsetsFrozenUntilStartInput); // Verify the IME insets is visible on app, but not for app2 during app task switching. + mDisplayContent.computeImeTargetIfNeeded(app.mActivityRecord); assertTrue(app.getInsetsState().getSource(ITYPE_IME).isVisible()); assertFalse(app2.getInsetsState().getSource(ITYPE_IME).isVisible()); }