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

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<WindowContainer> targets,
+            @NonNull ArrayMap<WindowContainer, ChangeInfo> 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<WindowContainer> 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;

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;

services/core/java/com/android/server/wm/WindowContainer.java

@@ -541,6 +541,10 @@ class WindowContainer<E extends 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();

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) {