Avoid detached surface from returning to hierarchy

Transition#buildFinishTransaction contains the operation that
reparent the animation target to original parent. If the target
is removed before the transition finishes, when the finish
transaction applies, the removed surface may attach to hierarchy
again (becomes a handleNotAlive layer).

By deferring the WindowToken removal until after the transition is
finished, the removal transaction always applies after the finish
transaction. Then the the removed window won't attach to its original
parent again.

Note that the WindowState can still be removed directly, so it
(empty children) won't affect animation by showing something.

Bug: 366098095
Flag: EXEMPT bugfix
Test: atest WindowTokenTests#testTokenRemovalProcess
Test: Run the script (The output should be empty):
      for i in {1..500}
      do
        adb shell wm size 200x200
        adb shell input keyevent 24
        adb shell wm size 1080x2400
        adb shell input keyevent 25
      done
      adb shell dumpsys SurfaceFlinger | grep type=2020
Change-Id: Ie9c5857b2c387c4380bbb7718cb366bfe9cd4b7e

6 files changed, 51 insertions, 22 deletions

diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java
index 0b36c7eb5fdf..31f4d081d913 100644
--- a/services/core/java/com/android/server/wm/ActivityRecord.java
+++ b/services/core/java/com/android/server/wm/ActivityRecord.java
@@ -880,8 +880,6 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A
     })
     @interface SplashScreenBehavior { }
 
-    // TODO: Have a WindowContainer state for tracking exiting/deferred removal.
-    boolean mIsExiting;
     // Force an app transition to be ran in the case the visibility of the app did not change.
     // We use this for the case of moving a Root Task to the back with multiple activities, and the
     // top activity enters PIP; the bottom activity's visibility stays the same, but we need to
@@ -1227,10 +1225,9 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A
             pw.print(" lastAllDrawn="); pw.print(mLastAllDrawn);
             pw.println(")");
         }
-        if (mStartingData != null || firstWindowDrawn || mIsExiting) {
+        if (mStartingData != null || firstWindowDrawn) {
             pw.print(prefix); pw.print("startingData="); pw.print(mStartingData);
-            pw.print(" firstWindowDrawn="); pw.print(firstWindowDrawn);
-            pw.print(" mIsExiting="); pw.println(mIsExiting);
+            pw.print(" firstWindowDrawn="); pw.println(firstWindowDrawn);
         }
         if (mStartingWindow != null || mStartingData != null || mStartingSurface != null
                 || startingMoved || mVisibleSetFromTransferredStartingWindow) {
@@ -4370,21 +4367,6 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A
         super.removeImmediately();
     }
 
-    @Override
-    void removeIfPossible() {
-        mIsExiting = false;
-        removeAllWindowsIfPossible();
-        removeImmediately();
-    }
-
-    @Override
-    boolean handleCompleteDeferredRemoval() {
-        if (mIsExiting) {
-            removeIfPossible();
-        }
-        return super.handleCompleteDeferredRemoval();
-    }
-
     void onRemovedFromDisplay() {
         if (mRemovingFromDisplay) {
             return;
diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java
index 188b368c47c5..1659f7bc6eed 100644
--- a/services/core/java/com/android/server/wm/Transition.java
+++ b/services/core/java/com/android/server/wm/Transition.java
@@ -1753,7 +1753,6 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
         }
     }
 
-    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE)
     static boolean containsChangeFor(WindowContainer wc, ArrayList<ChangeInfo> list) {
         for (int i = list.size() - 1; i >= 0; --i) {
             if (list.get(i).mContainer == wc) return true;
diff --git a/services/core/java/com/android/server/wm/TransitionController.java b/services/core/java/com/android/server/wm/TransitionController.java
index b7fe32713100..87bdfa4f5d75 100644
--- a/services/core/java/com/android/server/wm/TransitionController.java
+++ b/services/core/java/com/android/server/wm/TransitionController.java
@@ -471,6 +471,16 @@ class TransitionController {
         return false;
     }
 
+    /** Returns {@code true} if the `wc` is a target of a playing transition. */
+    boolean isPlayingTarget(@NonNull WindowContainer<?> wc) {
+        for (int i = mPlayingTransitions.size() - 1; i >= 0; --i) {
+            if (Transition.containsChangeFor(wc, mPlayingTransitions.get(i).mTargets)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     /** Returns {@code true} if the finishing transition contains `wc`. */
     boolean inFinishingTransition(WindowContainer<?> wc) {
         return mFinishingTransition != null && mFinishingTransition.isInTransition(wc);
diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java
index ebf645d84f95..f4ad0307d24b 100644
--- a/services/core/java/com/android/server/wm/WindowManagerService.java
+++ b/services/core/java/com/android/server/wm/WindowManagerService.java
@@ -2109,7 +2109,7 @@ public class WindowManagerService extends IWindowManager.Stub
         ProtoLog.v(WM_DEBUG_ADD_REMOVE, "Removing %s from %s", win, token);
         // Window will already be removed from token before this post clean-up method is called.
         if (token.isEmpty() && !token.mPersistOnEmpty) {
-            token.removeImmediately();
+            token.removeIfPossible();
         }
 
         if (win.mActivityRecord != null) {
diff --git a/services/core/java/com/android/server/wm/WindowToken.java b/services/core/java/com/android/server/wm/WindowToken.java
index 7e7ca12cd44e..5bde8b5a507c 100644
--- a/services/core/java/com/android/server/wm/WindowToken.java
+++ b/services/core/java/com/android/server/wm/WindowToken.java
@@ -90,6 +90,9 @@ class WindowToken extends WindowContainer<WindowState> {
     // Is key dispatching paused for this token?
     boolean paused = false;
 
+    /** Whether this container should be removed when it no longer animates. */
+    boolean mIsExiting;
+
     /** The owner has {@link android.Manifest.permission#MANAGE_APP_TOKENS} */
     final boolean mOwnerCanManageAppTokens;
 
@@ -276,6 +279,28 @@ class WindowToken extends WindowContainer<WindowState> {
         }
     }
 
+    @Override
+    void removeIfPossible() {
+        if (mTransitionController.isPlayingTarget(this)) {
+            // Defer removing this container until the transition is finished. So the removal can
+            // execute after the finish transaction (see Transition#buildFinishTransaction) which
+            // may reparent it to original parent.
+            mIsExiting = true;
+            return;
+        }
+        mIsExiting = false;
+        removeAllWindowsIfPossible();
+        removeImmediately();
+    }
+
+    @Override
+    boolean handleCompleteDeferredRemoval() {
+        if (mIsExiting) {
+            removeIfPossible();
+        }
+        return super.handleCompleteDeferredRemoval();
+    }
+
     /**
      * @return The scale for applications running in compatibility mode. Multiply the size in the
      *         application by this scale will be the size in the screen.
@@ -725,6 +750,9 @@ class WindowToken extends WindowContainer<WindowState> {
             pw.print("fixedRotationConfig=");
             pw.println(mFixedRotationTransformState.mRotatedOverrideConfiguration);
         }
+        if (mIsExiting) {
+            pw.print(prefix); pw.println("isExiting=true");
+        }
     }
 
     @Override
diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowTokenTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowTokenTests.java
index 714eb4b3c093..35328a0e1dc0 100644
--- a/services/tests/wmtests/src/com/android/server/wm/WindowTokenTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/WindowTokenTests.java
@@ -23,6 +23,7 @@ import static android.view.WindowManager.LayoutParams.TYPE_APPLICATION_OVERLAY;
 import static android.view.WindowManager.LayoutParams.TYPE_STATUS_BAR;
 import static android.view.WindowManager.LayoutParams.TYPE_TOAST;
 
+import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;
 import static com.android.server.policy.WindowManagerPolicy.TRANSIT_EXIT;
 
@@ -157,7 +158,16 @@ public class WindowTokenTests extends WindowTestsBase {
         // Verify that the other token window is still around.
         assertEquals(1, token.getWindowsCount());
 
+        final TransitionController transitionController = token.mTransitionController;
+        spyOn(transitionController);
+        doReturn(true).when(transitionController).isPlayingTarget(token);
         window2.removeImmediately();
+        assertTrue(token.mIsExiting);
+        assertNotNull("Defer removal for playing transition", token.getParent());
+
+        doReturn(false).when(transitionController).isPlayingTarget(token);
+        token.handleCompleteDeferredRemoval();
+        assertFalse(token.mIsExiting);
         // Verify that the token is no-longer attached to its parent
         assertNull(token.getParent());
         // Verify that the token windows are no longer attached to it.