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
diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java
index 0b36c7e..31f4d08 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 @@
})
@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 @@
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 @@
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 188b368..1659f7b 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 @@
}
}
- @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 b7fe327..87bdfa4 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 @@
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 ebf645d..f4ad030 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 @@
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 7e7ca12..5bde8b5 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 @@
// 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 @@
}
}
+ @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 @@
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 714eb4b..35328a0e 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_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 @@
// 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.