Fix index out of bounds when removing deferred containers

This makes the commit 793442c general to handle the removal from
all level since the hierarchy won't always keep the same shape.

Also rename to handleCompleteDeferredRemoval since it is not a
pure function for checking.

Fixes: 159865315
Test: atest WindowContainerTests#testHandleCompleteDeferredRemoval

Change-Id: I34e02652fb765c1cf78cbd3e86a07e592f02e7df
Merged-In: I34e02652fb765c1cf78cbd3e86a07e592f02e7df

6 files changed, 50 insertions, 43 deletions

diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java
index c4663ca19f87..2e9f70448488 100644
--- a/services/core/java/com/android/server/wm/ActivityRecord.java
+++ b/services/core/java/com/android/server/wm/ActivityRecord.java
@@ -3146,11 +3146,11 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A
     }
 
     @Override
-    boolean checkCompleteDeferredRemoval() {
+    boolean handleCompleteDeferredRemoval() {
         if (mIsExiting) {
             removeIfPossible();
         }
-        return super.checkCompleteDeferredRemoval();
+        return super.handleCompleteDeferredRemoval();
     }
 
     void onRemovedFromDisplay() {
diff --git a/services/core/java/com/android/server/wm/ActivityStack.java b/services/core/java/com/android/server/wm/ActivityStack.java
index b4bc0f5b3a32..8f0de7312ee5 100644
--- a/services/core/java/com/android/server/wm/ActivityStack.java
+++ b/services/core/java/com/android/server/wm/ActivityStack.java
@@ -86,7 +86,6 @@ import static com.android.server.wm.TaskProto.ACTIVITY_TYPE;
 import static com.android.server.wm.TaskProto.ANIMATING_BOUNDS;
 import static com.android.server.wm.TaskProto.BOUNDS;
 import static com.android.server.wm.TaskProto.CREATED_BY_ORGANIZER;
-import static com.android.server.wm.TaskProto.DEFER_REMOVAL;
 import static com.android.server.wm.TaskProto.DISPLAY_ID;
 import static com.android.server.wm.TaskProto.FILLS_PARENT;
 import static com.android.server.wm.TaskProto.LAST_NON_FULLSCREEN_BOUNDS;
@@ -248,11 +247,6 @@ class ActivityStack extends Task {
     private Rect mTmpRect = new Rect();
     private Rect mTmpRect2 = new Rect();
 
-    /** Detach this stack from its display when animation completes. */
-    // TODO: maybe tie this to WindowContainer#removeChild some how...
-    // TODO: This is no longer set. Okay to remove or was the set removed by accident?
-    private boolean mDeferRemoval;
-
     // If this is true, we are in the bounds animating mode. The task will be down or upscaled to
     // perfectly fit the region it would have been cropped to. We may also avoid certain logic we
     // would otherwise apply while resizing, while resizing in the bounds animating mode.
@@ -3232,9 +3226,6 @@ class ActivityStack extends Task {
 
     @Override
     void dump(PrintWriter pw, String prefix, boolean dumpAll) {
-        if (mDeferRemoval) {
-            pw.println(prefix + "mDeferRemoval=true");
-        }
         super.dump(pw, prefix, dumpAll);
         if (!mExitingActivities.isEmpty()) {
             pw.println();
@@ -3304,15 +3295,12 @@ class ActivityStack extends Task {
     }
 
     /** Returns true if a removal action is still being deferred. */
-    boolean checkCompleteDeferredRemoval() {
+    boolean handleCompleteDeferredRemoval() {
         if (isAnimating(TRANSITION | CHILDREN)) {
             return true;
         }
-        if (mDeferRemoval) {
-            removeImmediately();
-        }
 
-        return super.checkCompleteDeferredRemoval();
+        return super.handleCompleteDeferredRemoval();
     }
 
     public DisplayInfo getDisplayInfo() {
@@ -3380,7 +3368,6 @@ class ActivityStack extends Task {
             mLastNonFullscreenBounds.dumpDebug(proto, LAST_NON_FULLSCREEN_BOUNDS);
         }
 
-        proto.write(DEFER_REMOVAL, mDeferRemoval);
         proto.write(ANIMATING_BOUNDS, mBoundsAnimating);
 
         if (mSurfaceControl != null) {
diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java
index 241de2e0e068..c56440785bba 100644
--- a/services/core/java/com/android/server/wm/DisplayContent.java
+++ b/services/core/java/com/android/server/wm/DisplayContent.java
@@ -2739,6 +2739,7 @@ class DisplayContent extends WindowContainer<DisplayContent.DisplayChildWindowCo
     @Override
     void removeImmediately() {
         mRemovingDisplay = true;
+        mDeferredRemoval = false;
         try {
             if (mParentWindow != null) {
                 mParentWindow.removeEmbeddedDisplayContent(this);
@@ -2771,31 +2772,14 @@ class DisplayContent extends WindowContainer<DisplayContent.DisplayChildWindowCo
 
     /** Returns true if a removal action is still being deferred. */
     @Override
-    boolean checkCompleteDeferredRemoval() {
-        boolean stillDeferringRemoval = false;
-
-        for (int i = getChildCount() - 1; i >= 0; --i) {
-            final DisplayChildWindowContainer child = getChildAt(i);
-            stillDeferringRemoval |= child.checkCompleteDeferredRemoval();
-            if (getChildCount() == 0) {
-                // If this display is pending to be removed because it contains an activity with
-                // {@link ActivityRecord#mIsExiting} is true, this display may be removed when
-                // completing the removal of the last activity from
-                // {@link ActivityRecord#checkCompleteDeferredRemoval}.
-                return false;
-            }
-        }
+    boolean handleCompleteDeferredRemoval() {
+        final boolean stillDeferringRemoval = super.handleCompleteDeferredRemoval();
 
         if (!stillDeferringRemoval && mDeferredRemoval) {
             removeImmediately();
             return false;
         }
-        return true;
-    }
-
-    /** @return 'true' if removal of this display content is deferred due to active animation. */
-    boolean isRemovalDeferred() {
-        return mDeferredRemoval;
+        return stillDeferringRemoval;
     }
 
     void adjustForImeIfNeeded() {
diff --git a/services/core/java/com/android/server/wm/RootWindowContainer.java b/services/core/java/com/android/server/wm/RootWindowContainer.java
index 2d30b737b274..119b188dfa00 100644
--- a/services/core/java/com/android/server/wm/RootWindowContainer.java
+++ b/services/core/java/com/android/server/wm/RootWindowContainer.java
@@ -990,9 +990,7 @@ class RootWindowContainer extends WindowContainer<DisplayContent>
         }
 
         // Remove all deferred displays stacks, tasks, and activities.
-        for (int displayNdx = mChildren.size() - 1; displayNdx >= 0; --displayNdx) {
-            mChildren.get(displayNdx).checkCompleteDeferredRemoval();
-        }
+        handleCompleteDeferredRemoval();
 
         forAllDisplays(dc -> {
             dc.getInputMonitor().updateInputWindowsLw(true /*force*/);
diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java
index 2977968f5754..5a73fabaf9d0 100644
--- a/services/core/java/com/android/server/wm/WindowContainer.java
+++ b/services/core/java/com/android/server/wm/WindowContainer.java
@@ -1044,13 +1044,25 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
         return mChildren.peekLast();
     }
 
-    /** Returns true if there is still a removal being deferred */
-    boolean checkCompleteDeferredRemoval() {
+    /**
+     * Removes the containers which were deferred.
+     *
+     * @return {@code true} if there is still a removal being deferred.
+     */
+    boolean handleCompleteDeferredRemoval() {
         boolean stillDeferringRemoval = false;
 
         for (int i = mChildren.size() - 1; i >= 0; --i) {
             final WindowContainer wc = mChildren.get(i);
-            stillDeferringRemoval |= wc.checkCompleteDeferredRemoval();
+            stillDeferringRemoval |= wc.handleCompleteDeferredRemoval();
+            if (!hasChild()) {
+                // All child containers of current level could be removed from a removal of
+                // descendant. E.g. if a display is pending to be removed because it contains an
+                // activity with {@link ActivityRecord#mIsExiting} is true, the display may be
+                // removed when completing the removal of the last activity from
+                // {@link ActivityRecord#checkCompleteDeferredRemoval}.
+                return false;
+            }
         }
 
         return stillDeferringRemoval;
diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java
index 87485eac3412..efc03df877b7 100644
--- a/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java
@@ -27,6 +27,7 @@ import static android.window.DisplayAreaOrganizer.FEATURE_DEFAULT_TASK_CONTAINER
 
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.any;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.anyFloat;
+import static com.android.dx.mockito.inline.extended.ExtendedMockito.anyInt;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.eq;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.mock;
@@ -825,6 +826,31 @@ public class WindowContainerTests extends WindowTestsBase {
     }
 
     @Test
+    public void testHandleCompleteDeferredRemoval() {
+        final DisplayContent displayContent = createNewDisplay();
+        // Do not reparent activity to default display when removing the display.
+        doReturn(true).when(displayContent).shouldDestroyContentOnRemove();
+        final ActivityRecord r = new ActivityTestsBase.StackBuilder(mWm.mRoot)
+                .setDisplay(displayContent).build().getTopMostActivity();
+        // Add a window and make the activity animating so the removal of activity is deferred.
+        createWindow(null, TYPE_BASE_APPLICATION, r, "win");
+        doReturn(true).when(r).isAnimating(anyInt(), anyInt());
+
+        displayContent.remove();
+        // Ensure that ActivityRecord#onRemovedFromDisplay is called.
+        r.destroyed("test");
+        // The removal is deferred, so the activity is still in the display.
+        assertEquals(r, displayContent.getTopMostActivity());
+
+        // Assume the animation is done so the deferred removal can continue.
+        doReturn(false).when(r).isAnimating(anyInt(), anyInt());
+
+        assertFalse(displayContent.handleCompleteDeferredRemoval());
+        assertFalse(displayContent.hasChild());
+        assertFalse(r.hasChild());
+    }
+
+    @Test
     public void testTaskCanApplyAnimation() {
         final ActivityStack stack = createTaskStackOnDisplay(mDisplayContent);
         final Task task = createTaskInStack(stack, 0 /* userId */);