Merge "Try to fix IndexOutBoundsException in Task#removeChild" into rvc-dev am: e938c572b7

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/11836345

Change-Id: Ie83e275e23d979805e75ca8e4fb3b5cb6208cde4

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

@@ -1504,12 +1504,21 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks {
     }
 
     void removeTask(Task task, boolean killProcess, boolean removeFromRecents, String reason) {
-        task.removeTaskActivitiesLocked(reason);
-        cleanUpRemovedTaskLocked(task, killProcess, removeFromRecents);
-        mService.getLockTaskController().clearLockedTask(task);
-        mService.getTaskChangeNotificationController().notifyTaskStackChanged();
-        if (task.isPersistable) {
-            mService.notifyTaskPersisterLocked(null, true);
+        if (task.mInRemoveTask) {
+            // Prevent recursion.
+            return;
+        }
+        task.mInRemoveTask = true;
+        try {
+            task.performClearTask(reason);
+            cleanUpRemovedTaskLocked(task, killProcess, removeFromRecents);
+            mService.getLockTaskController().clearLockedTask(task);
+            mService.getTaskChangeNotificationController().notifyTaskStackChanged();
+            if (task.isPersistable) {
+                mService.notifyTaskPersisterLocked(null, true);
+            }
+        } finally {
+            task.mInRemoveTask = false;
         }
     }
 

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

@@ -435,6 +435,13 @@ class Task extends WindowContainer<WindowContainer> {
     static final int FLAG_FORCE_HIDDEN_FOR_TASK_ORG = 1 << 1;
     private int mForceHiddenFlags = 0;
 
+    // TODO(b/160201781): Revisit double invocation issue in Task#removeChild.
+    /**
+     * Skip {@link ActivityStackSupervisor#removeTask(Task, boolean, boolean, String)} execution if
+     * {@code true} to prevent double traversal of {@link #mChildren} in a loop.
+     */
+    boolean mInRemoveTask;
+
     // When non-null, this is a transaction that will get applied on the next frame returned after
     // a relayout is requested from the client. While this is only valid on a leaf task; since the
     // transaction can effect an ancestor task, this also needs to keep track of the ancestor task
@@ -1496,11 +1503,8 @@ class Task extends WindowContainer<WindowContainer> {
         return autoRemoveRecents || (!hasChild() && !getHasBeenVisible());
     }
 
-    /**
-     * Completely remove all activities associated with an existing
-     * task starting at a specified index.
-     */
-    private void performClearTaskAtIndexLocked(String reason) {
+    /** Completely remove all activities associated with an existing task. */
+    void performClearTask(String reason) {
         // Broken down into to cases to avoid object create due to capturing mStack.
         if (getStack() == null) {
             forAllActivities((r) -> {
@@ -1524,7 +1528,7 @@ class Task extends WindowContainer<WindowContainer> {
      */
     void performClearTaskLocked() {
         mReuseTask = true;
-        performClearTaskAtIndexLocked("clear-task-all");
+        performClearTask("clear-task-all");
         mReuseTask = false;
     }
 
@@ -1585,11 +1589,6 @@ class Task extends WindowContainer<WindowContainer> {
         return false;
     }
 
-    void removeTaskActivitiesLocked(String reason) {
-        // Just remove the entire task.
-        performClearTaskAtIndexLocked(reason);
-    }
-
     String lockTaskAuthToString() {
         switch (mLockTaskAuth) {
             case LOCK_TASK_AUTH_DONT_LOCK: return "LOCK_TASK_AUTH_DONT_LOCK";

services/tests/wmtests/src/com/android/server/wm/TaskTests.java

@@ -28,6 +28,8 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.clearInvocations;
 
 import android.graphics.Point;
@@ -158,4 +160,30 @@ public class TaskTests extends WindowTestsBase {
         assertEquals(activity1, task1.isInTask(activity1));
         assertNull(task1.isInTask(activity2));
     }
+
+    @Test
+    public void testRemoveChildForOverlayTask() {
+        final Task task = createTaskStackOnDisplay(mDisplayContent);
+        final int taskId = task.mTaskId;
+        final ActivityRecord activity1 =
+                WindowTestUtils.createActivityRecordInTask(mDisplayContent, task);
+        final ActivityRecord activity2 =
+                WindowTestUtils.createActivityRecordInTask(mDisplayContent, task);
+        final ActivityRecord activity3 =
+                WindowTestUtils.createActivityRecordInTask(mDisplayContent, task);
+        activity1.setTaskOverlay(true);
+        activity2.setTaskOverlay(true);
+        activity3.setTaskOverlay(true);
+
+        assertEquals(3, task.getChildCount());
+        assertTrue(task.onlyHasTaskOverlayActivities(true));
+
+        task.removeChild(activity1);
+
+        verify(task.mStackSupervisor).removeTask(any(), anyBoolean(), anyBoolean(), anyString());
+        assertEquals(2, task.getChildCount());
+        task.forAllActivities((r) -> {
+            assertTrue(r.finishing);
+        });
+    }
 }