From d7c426431368d2eb046efcb116961050d342ab10 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Wed, 26 Oct 2022 13:18:11 +0800 Subject: [PATCH] Check last parent when calculating Shell transition root Before, we pick root leash only based on the end state hierarchy, which may result the reparent animation covered by other windows between the new parent and old parent. Now, we make sure the root leash is also the ancestor of the old parent. Bug: 207070762 Test: atest WmTests:TransitionTests#testCollectReparentChange Test: manually verify with ActivityEmbedding enter PiP from split screen Change-Id: Ieeb2ce19b47adf4223ba6d58a3be10d7079f4eb3 --- .../com/android/server/wm/Transition.java | 110 +++++++++++++++--- .../server/wm/TransitionController.java | 11 ++ .../android/server/wm/WindowContainer.java | 4 + .../android/server/wm/TransitionTests.java | 23 ++++ 4 files changed, 132 insertions(+), 16 deletions(-) diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java index 32f61978d7303..91a7555262646 100644 --- a/services/core/java/com/android/server/wm/Transition.java +++ b/services/core/java/com/android/server/wm/Transition.java @@ -475,6 +475,48 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe mContainerFreezer.freeze(wc, change.mAbsoluteBounds); } + /** + * Records that a particular container has been reparented. This only effects windows that have + * already been collected in the transition. This should be called before reparenting because + * the old parent may be removed during reparenting, for example: + * {@link Task#shouldRemoveSelfOnLastChildRemoval} + */ + void collectReparentChange(@NonNull WindowContainer wc, @NonNull WindowContainer newParent) { + if (!mChanges.containsKey(wc)) { + // #collectReparentChange() will be called when the window is reparented. Skip if it is + // a window that has not been collected, which means we don't care about this window for + // the current transition. + return; + } + final ChangeInfo change = mChanges.get(wc); + // Use the current common ancestor if there are multiple reparent, and the original parent + // has been detached. Otherwise, use the original parent before the transition. + final WindowContainer prevParent = + change.mStartParent == null || change.mStartParent.isAttached() + ? change.mStartParent + : change.mCommonAncestor; + if (prevParent == null || !prevParent.isAttached()) { + Slog.w(TAG, "Trying to collect reparenting of a window after the previous parent has" + + " been detached: " + wc); + return; + } + if (prevParent == newParent) { + Slog.w(TAG, "Trying to collect reparenting of a window that has not been reparented: " + + wc); + return; + } + if (!newParent.isAttached()) { + Slog.w(TAG, "Trying to collect reparenting of a window that is not attached after" + + " reparenting: " + wc); + return; + } + WindowContainer ancestor = newParent; + while (prevParent != ancestor && !prevParent.isDescendantOf(ancestor)) { + ancestor = ancestor.getParent(); + } + change.mCommonAncestor = ancestor; + } + /** * @return {@code true} if `wc` is a participant or is a descendant of one. */ @@ -1558,20 +1600,7 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe return out; } - // Find the top-most shared ancestor of app targets. - WindowContainer ancestor = topApp.getParent(); - // Go up ancestor parent chain until all targets are descendants. - ancestorLoop: - while (ancestor != null) { - for (int i = sortedTargets.size() - 1; i >= 0; --i) { - final WindowContainer wc = sortedTargets.get(i); - if (!isWallpaper(wc) && !wc.isDescendantOf(ancestor)) { - ancestor = ancestor.getParent(); - continue ancestorLoop; - } - } - break; - } + WindowContainer ancestor = findCommonAncestor(sortedTargets, changes, topApp); // make leash based on highest (z-order) direct child of ancestor with a participant. WindowContainer leashReference = sortedTargets.get(0); @@ -1688,6 +1717,46 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe return out; } + /** + * Finds the top-most common ancestor of app targets. + * + * Makes sure that the previous parent is also a descendant to make sure the animation won't + * be covered by other windows below the previous parent. For example, when reparenting an + * activity from PiP Task to split screen Task. + */ + @NonNull + private static WindowContainer findCommonAncestor( + @NonNull ArrayList targets, + @NonNull ArrayMap changes, + @NonNull WindowContainer topApp) { + WindowContainer ancestor = topApp.getParent(); + // Go up ancestor parent chain until all targets are descendants. Ancestor should never be + // null because all targets are attached. + for (int i = targets.size() - 1; i >= 0; i--) { + final WindowContainer wc = targets.get(i); + if (isWallpaper(wc)) { + // Skip the non-app window. + continue; + } + while (!wc.isDescendantOf(ancestor)) { + ancestor = ancestor.getParent(); + } + + // Make sure the previous parent is also a descendant to make sure the animation won't + // be covered by other windows below the previous parent. For example, when reparenting + // an activity from PiP Task to split screen Task. + final ChangeInfo change = changes.get(wc); + final WindowContainer prevParent = change.mCommonAncestor; + if (prevParent == null || !prevParent.isAttached()) { + continue; + } + while (prevParent != ancestor && !prevParent.isDescendantOf(ancestor)) { + ancestor = ancestor.getParent(); + } + } + return ancestor; + } + private static WindowManager.LayoutParams getLayoutParamsForAnimationsStyle(int type, ArrayList sortedTargets) { // Find the layout params of the top-most application window that is part of the @@ -1806,10 +1875,19 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe @Retention(RetentionPolicy.SOURCE) @interface Flag {} - // Usually "post" change state. + /** + * "Parent" that is also included in the transition. When populating the parent changes, we + * may skip the intermediate parents, so this may not be the actual parent in the hierarchy. + */ WindowContainer mEndParent; - // Parent before change state. + /** Actual parent window before change state. */ WindowContainer mStartParent; + /** + * When the window is reparented during the transition, this is the common ancestor window + * of the {@link #mStartParent} and the current parent. This is needed because the + * {@link #mStartParent} may have been detached when the transition starts. + */ + WindowContainer mCommonAncestor; // State tracking boolean mExistenceChanged = false; diff --git a/services/core/java/com/android/server/wm/TransitionController.java b/services/core/java/com/android/server/wm/TransitionController.java index 26ce4ae8415c4..e4d39b94ab101 100644 --- a/services/core/java/com/android/server/wm/TransitionController.java +++ b/services/core/java/com/android/server/wm/TransitionController.java @@ -533,6 +533,17 @@ class TransitionController { mCollectingTransition.collectVisibleChange(wc); } + /** + * Records that a particular container has been reparented. This only effects windows that have + * already been collected in the transition. This should be called before reparenting because + * the old parent may be removed during reparenting, for example: + * {@link Task#shouldRemoveSelfOnLastChildRemoval} + */ + void collectReparentChange(@NonNull WindowContainer wc, @NonNull WindowContainer newParent) { + if (!isCollecting()) return; + mCollectingTransition.collectReparentChange(wc, newParent); + } + /** @see Transition#mStatusBarTransitionDelay */ void setStatusBarTransitionDelay(long delay) { if (mCollectingTransition == null) return; diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java index 07ae167f5e669..a583ff386e72c 100644 --- a/services/core/java/com/android/server/wm/WindowContainer.java +++ b/services/core/java/com/android/server/wm/WindowContainer.java @@ -541,6 +541,10 @@ class WindowContainer extends ConfigurationContainer< throw new IllegalArgumentException("WC=" + this + " already child of " + mParent); } + // Collect before removing child from old parent, because the old parent may be removed if + // this is the last child in it. + mTransitionController.collectReparentChange(this, newParent); + // The display object before reparenting as that might lead to old parent getting removed // from the display if it no longer has any child. final DisplayContent prevDc = oldParent.getDisplayContent(); diff --git a/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java b/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java index 66e46a2bb187e..54bcbd9adf5ab 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java @@ -1517,6 +1517,29 @@ public class TransitionTests extends WindowTestsBase { transition.abort(); } + @Test + public void testCollectReparentChange() { + registerTestTransitionPlayer(); + + // Reparent activity in transition. + final Task lastParent = createTask(mDisplayContent); + final Task newParent = createTask(mDisplayContent); + final ActivityRecord activity = createActivityRecord(lastParent); + doReturn(true).when(lastParent).shouldRemoveSelfOnLastChildRemoval(); + doNothing().when(activity).setDropInputMode(anyInt()); + activity.mVisibleRequested = true; + + final Transition transition = new Transition(TRANSIT_CHANGE, 0 /* flags */, + activity.mTransitionController, mWm.mSyncEngine); + activity.mTransitionController.moveToCollecting(transition); + transition.collect(activity); + activity.reparent(newParent, POSITION_TOP); + + // ChangeInfo#mCommonAncestor should be set after reparent. + final Transition.ChangeInfo change = transition.mChanges.get(activity); + assertEquals(newParent.getDisplayArea(), change.mCommonAncestor); + } + private static void makeTaskOrganized(Task... tasks) { final ITaskOrganizer organizer = mock(ITaskOrganizer.class); for (Task t : tasks) {