Merge "Allow delete empty TaskFragment in PiP" into tm-dev am: d41df35243 am: b52ab7d11e
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/17800404
Change-Id: Ie7b7e0ac95ddf43b10f5a364205230f7d3fc5abe
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
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 33a41ec..e20cef2 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 @@
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 @@
return;
}
+ final WindowContainerTransaction wct = new WindowContainerTransaction();
final boolean wasInPip = isInPictureInPicture(container);
container.setInfo(taskFragmentInfo);
final boolean isInPip = isInPictureInPicture(container);
@@ -159,13 +161,13 @@
// 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 @@
// 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 @@
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 @@
}
/** 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 @@
// 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 @@
// 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 @@
}
}
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 b55c16e..1b49585 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.NonNull;
import androidx.annotation.Nullable;
-import java.util.Collection;
import java.util.concurrent.Executor;
/**
@@ -73,16 +72,12 @@
}
/**
- * 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 @@
* 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 @@
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 20c929b..871f545 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 @@
// 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 @@
// 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 a26a4b6..72519dc 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 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 @@
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 @@
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 0000000..97896c2
--- /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 c1c8b81..67f7ff7 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 @@
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 d135de0..a297608 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 @@
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