Merge "Allow delete empty TaskFragment in PiP" into tm-dev

7 files changed, 183 insertions, 33 deletions

diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java
index 33a41ecd49fa..e20cef2bec4e 100644
--- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java
+++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java
@@ -60,7 +60,8 @@ import java.util.function.Consumer;
 public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmentCallback,
         ActivityEmbeddingComponent {
 
-    private final SplitPresenter mPresenter;
+    @VisibleForTesting
+    final SplitPresenter mPresenter;
 
     // Currently applied split configuration.
     private final List<EmbeddingRule> mSplitRules = new ArrayList<>();
@@ -149,6 +150,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
             return;
         }
 
+        final WindowContainerTransaction wct = new WindowContainerTransaction();
         final boolean wasInPip = isInPictureInPicture(container);
         container.setInfo(taskFragmentInfo);
         final boolean isInPip = isInPictureInPicture(container);
@@ -159,13 +161,13 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
                 // Do not finish the dependents if the last activity is reparented to PiP.
                 // Instead, the original split should be cleanup, and the dependent may be expanded
                 // to fullscreen.
-                cleanupForEnterPip(container);
-                mPresenter.cleanupContainer(container, false /* shouldFinishDependent */);
+                cleanupForEnterPip(wct, container);
+                mPresenter.cleanupContainer(container, false /* shouldFinishDependent */, wct);
             } else {
                 // Do not finish the dependents if this TaskFragment was cleared due to launching
                 // activity in the Task.
                 final boolean shouldFinishDependent = !taskFragmentInfo.isTaskClearedForReuse();
-                mPresenter.cleanupContainer(container, shouldFinishDependent);
+                mPresenter.cleanupContainer(container, shouldFinishDependent, wct);
             }
         } else if (wasInPip && isInPip) {
             // No update until exit PIP.
@@ -174,12 +176,13 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
             // Enter PIP.
             // All overrides will be cleanup.
             container.setLastRequestedBounds(null /* bounds */);
-            cleanupForEnterPip(container);
+            cleanupForEnterPip(wct, container);
         } else if (wasInPip) {
             // Exit PIP.
             // Updates the presentation of the container. Expand or launch placeholder if needed.
-            mPresenter.updateContainer(container);
+            updateContainer(wct, container);
         }
+        mPresenter.applyTransaction(wct);
         updateCallbackIfNecessary();
     }
 
@@ -188,7 +191,15 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
         final TaskFragmentContainer container = getContainer(taskFragmentInfo.getFragmentToken());
         if (container != null) {
             // Cleanup if the TaskFragment vanished is not requested by the organizer.
-            mPresenter.cleanupContainer(container, true /* shouldFinishDependent */);
+            removeContainer(container);
+            // Make sure the top container is updated.
+            final TaskFragmentContainer newTopContainer = getTopActiveContainer(
+                    container.getTaskId());
+            if (newTopContainer != null) {
+                final WindowContainerTransaction wct = new WindowContainerTransaction();
+                updateContainer(wct, newTopContainer);
+                mPresenter.applyTransaction(wct);
+            }
             updateCallbackIfNecessary();
         }
         cleanupTaskFragment(taskFragmentInfo.getFragmentToken());
@@ -452,7 +463,8 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
     }
 
     /** Cleanups all the dependencies when the TaskFragment is entering PIP. */
-    private void cleanupForEnterPip(@NonNull TaskFragmentContainer container) {
+    private void cleanupForEnterPip(@NonNull WindowContainerTransaction wct,
+            @NonNull TaskFragmentContainer container) {
         final int taskId = container.getTaskId();
         final TaskContainer taskContainer = mTaskContainers.get(taskId);
         if (taskContainer == null) {
@@ -482,7 +494,9 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
         // If there is any TaskFragment split with the PIP TaskFragment, update their presentations
         // since the split is dismissed.
         // We don't want to close any of them even if they are dependencies of the PIP TaskFragment.
-        mPresenter.updateContainers(containersToUpdate);
+        for (TaskFragmentContainer containerToUpdate : containersToUpdate) {
+            updateContainer(wct, containerToUpdate);
+        }
     }
 
     /**
@@ -502,6 +516,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
         // TaskFragment there.
         taskContainer.mFinishedContainer.add(container.getTaskFragmentToken());
 
+        // Cleanup any split references.
         final List<SplitContainer> containersToRemove = new ArrayList<>();
         for (SplitContainer splitContainer : taskContainer.mSplitContainers) {
             if (container.equals(splitContainer.getSecondaryContainer())
@@ -510,6 +525,11 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
             }
         }
         taskContainer.mSplitContainers.removeAll(containersToRemove);
+
+        // Cleanup any dependent references.
+        for (TaskFragmentContainer containerToUpdate : taskContainer.mContainers) {
+            containerToUpdate.removeContainerToFinishOnExit(container);
+        }
     }
 
     /**
diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitPresenter.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitPresenter.java
index b55c16e3b45d..1b49585ed7dc 100644
--- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitPresenter.java
+++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitPresenter.java
@@ -36,7 +36,6 @@ import androidx.annotation.IntDef;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
-import java.util.Collection;
 import java.util.concurrent.Executor;
 
 /**
@@ -73,16 +72,12 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer {
     }
 
     /**
-     * Updates the presentation of the provided containers.
+     * Deletes the specified container and all other associated and dependent containers in the same
+     * transaction.
      */
-    void updateContainers(@NonNull Collection<TaskFragmentContainer> containers) {
-        if (containers.isEmpty()) {
-            return;
-        }
+    void cleanupContainer(@NonNull TaskFragmentContainer container, boolean shouldFinishDependent) {
         final WindowContainerTransaction wct = new WindowContainerTransaction();
-        for (TaskFragmentContainer container : containers) {
-            mController.updateContainer(wct, container);
-        }
+        cleanupContainer(container, shouldFinishDependent, wct);
         applyTransaction(wct);
     }
 
@@ -90,9 +85,8 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer {
      * Deletes the specified container and all other associated and dependent containers in the same
      * transaction.
      */
-    void cleanupContainer(@NonNull TaskFragmentContainer container, boolean shouldFinishDependent) {
-        final WindowContainerTransaction wct = new WindowContainerTransaction();
-
+    void cleanupContainer(@NonNull TaskFragmentContainer container, boolean shouldFinishDependent,
+            @NonNull WindowContainerTransaction wct) {
         container.finish(shouldFinishDependent, this, wct, mController);
 
         final TaskFragmentContainer newTopContainer = mController.getTopActiveContainer(
@@ -100,8 +94,6 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer {
         if (newTopContainer != null) {
             mController.updateContainer(wct, newTopContainer);
         }
-
-        applyTransaction(wct);
     }
 
     /**
diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java
index 20c929b26acb..871f545d203a 100644
--- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java
+++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java
@@ -257,7 +257,8 @@ class TaskFragmentContainer {
 
         // Finish dependent containers
         for (TaskFragmentContainer container : mContainersToFinishOnExit) {
-            if (controller.shouldRetainAssociatedContainer(this, container)) {
+            if (container.mIsFinished
+                    || controller.shouldRetainAssociatedContainer(this, container)) {
                 continue;
             }
             container.finish(true /* shouldFinishDependent */, presenter,
@@ -267,18 +268,13 @@ class TaskFragmentContainer {
 
         // Finish associated activities
         for (Activity activity : mActivitiesToFinishOnExit) {
-            if (controller.shouldRetainAssociatedActivity(this, activity)) {
+            if (activity.isFinishing()
+                    || controller.shouldRetainAssociatedActivity(this, activity)) {
                 continue;
             }
             activity.finish();
         }
         mActivitiesToFinishOnExit.clear();
-
-        // Finish activities that were being re-parented to this container.
-        for (Activity activity : mPendingAppearedActivities) {
-            activity.finish();
-        }
-        mPendingAppearedActivities.clear();
     }
 
     boolean isFinished() {
diff --git a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java
index a26a4b657d68..72519dc6da5f 100644
--- a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java
+++ b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java
@@ -17,13 +17,20 @@
 package androidx.window.extensions.embedding;
 
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;
+import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify;
 
 import static com.google.common.truth.Truth.assertWithMessage;
 
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 
+import android.app.Activity;
+import android.content.res.Configuration;
+import android.content.res.Resources;
 import android.platform.test.annotations.Presubmit;
+import android.window.TaskFragmentInfo;
 
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import androidx.test.filters.SmallTest;
@@ -32,12 +39,14 @@ import androidx.window.extensions.embedding.SplitController.TaskContainer;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
 
 /**
  * Test class for {@link SplitController}.
  *
  * Build/Install/Run:
- *  atest WMJetpackUnitTests:SplitController
+ *  atest WMJetpackUnitTests:SplitControllerTest
  */
 @Presubmit
 @SmallTest
@@ -45,12 +54,24 @@ import org.junit.runner.RunWith;
 public class SplitControllerTest {
     private static final int TASK_ID = 10;
 
+    @Mock
+    private Activity mActivity;
+    @Mock
+    private Resources mActivityResources;
+    @Mock
+    private TaskFragmentInfo mInfo;
     private SplitController mSplitController;
+    private SplitPresenter mSplitPresenter;
 
     @Before
     public void setUp() {
+        MockitoAnnotations.initMocks(this);
         mSplitController = new SplitController();
+        mSplitPresenter = mSplitController.mPresenter;
         spyOn(mSplitController);
+        spyOn(mSplitPresenter);
+        doReturn(mActivityResources).when(mActivity).getResources();
+        doReturn(new Configuration()).when(mActivityResources).getConfiguration();
     }
 
     @Test
@@ -83,4 +104,17 @@ public class SplitControllerTest {
         assertWithMessage("Must return null because tf1 has no running activity.")
                 .that(mSplitController.getTopActiveContainer(TASK_ID)).isNull();
     }
+
+    @Test
+    public void testOnTaskFragmentVanished() {
+        final TaskFragmentContainer tf = mSplitController.newContainer(mActivity, TASK_ID);
+        doReturn(tf.getTaskFragmentToken()).when(mInfo).getFragmentToken();
+
+        // The TaskFragment has been removed in the server, we only need to cleanup the reference.
+        mSplitController.onTaskFragmentVanished(mInfo);
+
+        verify(mSplitPresenter, never()).deleteTaskFragment(any(), any());
+        verify(mSplitController).removeContainer(tf);
+        verify(mActivity, never()).finish();
+    }
 }
diff --git a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java
new file mode 100644
index 000000000000..97896c2c0a57
--- /dev/null
+++ b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java
@@ -0,0 +1,97 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package androidx.window.extensions.embedding;
+
+import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.clearInvocations;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.never;
+
+import android.app.Activity;
+import android.platform.test.annotations.Presubmit;
+import android.window.TaskFragmentInfo;
+import android.window.WindowContainerTransaction;
+
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+import androidx.test.filters.SmallTest;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+import java.util.ArrayList;
+
+/**
+ * Test class for {@link TaskFragmentContainer}.
+ *
+ * Build/Install/Run:
+ *  atest WMJetpackUnitTests:TaskFragmentContainerTest
+ */
+@Presubmit
+@SmallTest
+@RunWith(AndroidJUnit4.class)
+public class TaskFragmentContainerTest {
+    private static final int TASK_ID = 10;
+
+    @Mock
+    private SplitPresenter mPresenter;
+    @Mock
+    private SplitController mController;
+    @Mock
+    private Activity mActivity;
+    @Mock
+    private TaskFragmentInfo mInfo;
+
+    @Before
+    public void setup() {
+        MockitoAnnotations.initMocks(this);
+    }
+
+    @Test
+    public void testFinish() {
+        final TaskFragmentContainer container = new TaskFragmentContainer(mActivity, TASK_ID);
+        final WindowContainerTransaction wct = new WindowContainerTransaction();
+
+        // Only remove the activity, but not clear the reference until appeared.
+        container.finish(true /* shouldFinishDependent */, mPresenter, wct, mController);
+
+        verify(mActivity).finish();
+        verify(mPresenter, never()).deleteTaskFragment(any(), any());
+        verify(mController, never()).removeContainer(any());
+
+        // Calling twice should not finish activity again.
+        clearInvocations(mActivity);
+        container.finish(true /* shouldFinishDependent */, mPresenter, wct, mController);
+
+        verify(mActivity, never()).finish();
+        verify(mPresenter, never()).deleteTaskFragment(any(), any());
+        verify(mController, never()).removeContainer(any());
+
+        // Remove all references after the container has appeared in server.
+        doReturn(new ArrayList<>()).when(mInfo).getActivities();
+        container.setInfo(mInfo);
+        container.finish(true /* shouldFinishDependent */, mPresenter, wct, mController);
+
+        verify(mActivity, never()).finish();
+        verify(mPresenter).deleteTaskFragment(wct, container.getTaskFragmentToken());
+        verify(mController).removeContainer(container);
+    }
+}
diff --git a/services/core/java/com/android/server/wm/WindowOrganizerController.java b/services/core/java/com/android/server/wm/WindowOrganizerController.java
index c1c8b81f2c32..67f7ff701981 100644
--- a/services/core/java/com/android/server/wm/WindowOrganizerController.java
+++ b/services/core/java/com/android/server/wm/WindowOrganizerController.java
@@ -1522,7 +1522,10 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
             sendTaskFragmentOperationFailure(organizer, errorCallbackToken, exception);
             return 0;
         }
-        if (taskFragment.isEmbeddedTaskFragmentInPip()) {
+        if (taskFragment.isEmbeddedTaskFragmentInPip()
+                // When the Task enters PiP before the organizer removes the empty TaskFragment, we
+                // should allow it to do the cleanup.
+                && taskFragment.getTopNonFinishingActivity() != null) {
             final Throwable exception = new IllegalArgumentException(
                     "Not allowed to delete TaskFragment in PIP Task");
             sendTaskFragmentOperationFailure(organizer, errorCallbackToken, exception);
diff --git a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java
index d135de0fc921..a297608af480 100644
--- a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java
+++ b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java
@@ -527,6 +527,14 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
         verify(mAtm.mWindowOrganizerController).sendTaskFragmentOperationFailure(eq(mIOrganizer),
                 eq(errorToken), any(IllegalArgumentException.class));
         assertNotNull(mAtm.mWindowOrganizerController.getTaskFragment(mFragmentToken));
+
+        // Allow organizer to delete empty TaskFragment for cleanup.
+        final Task task = mTaskFragment.getTask();
+        mTaskFragment.removeChild(mTaskFragment.getTopMostActivity());
+        mAtm.mWindowOrganizerController.applyTransaction(mTransaction);
+
+        assertNull(mAtm.mWindowOrganizerController.getTaskFragment(mFragmentToken));
+        assertNull(task.getTopChild());
     }
 
     @Test