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

This commit is contained in:
Chris Li
2022-04-18 15:26:56 +00:00
committed by Android (Google) Code Review
7 changed files with 183 additions and 33 deletions

View File

@@ -60,7 +60,8 @@ import java.util.function.Consumer;
public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmentCallback, public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmentCallback,
ActivityEmbeddingComponent { ActivityEmbeddingComponent {
private final SplitPresenter mPresenter; @VisibleForTesting
final SplitPresenter mPresenter;
// Currently applied split configuration. // Currently applied split configuration.
private final List<EmbeddingRule> mSplitRules = new ArrayList<>(); private final List<EmbeddingRule> mSplitRules = new ArrayList<>();
@@ -149,6 +150,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
return; return;
} }
final WindowContainerTransaction wct = new WindowContainerTransaction();
final boolean wasInPip = isInPictureInPicture(container); final boolean wasInPip = isInPictureInPicture(container);
container.setInfo(taskFragmentInfo); container.setInfo(taskFragmentInfo);
final boolean isInPip = isInPictureInPicture(container); 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. // 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 // Instead, the original split should be cleanup, and the dependent may be expanded
// to fullscreen. // to fullscreen.
cleanupForEnterPip(container); cleanupForEnterPip(wct, container);
mPresenter.cleanupContainer(container, false /* shouldFinishDependent */); mPresenter.cleanupContainer(container, false /* shouldFinishDependent */, wct);
} else { } else {
// Do not finish the dependents if this TaskFragment was cleared due to launching // Do not finish the dependents if this TaskFragment was cleared due to launching
// activity in the Task. // activity in the Task.
final boolean shouldFinishDependent = !taskFragmentInfo.isTaskClearedForReuse(); final boolean shouldFinishDependent = !taskFragmentInfo.isTaskClearedForReuse();
mPresenter.cleanupContainer(container, shouldFinishDependent); mPresenter.cleanupContainer(container, shouldFinishDependent, wct);
} }
} else if (wasInPip && isInPip) { } else if (wasInPip && isInPip) {
// No update until exit PIP. // No update until exit PIP.
@@ -174,12 +176,13 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
// Enter PIP. // Enter PIP.
// All overrides will be cleanup. // All overrides will be cleanup.
container.setLastRequestedBounds(null /* bounds */); container.setLastRequestedBounds(null /* bounds */);
cleanupForEnterPip(container); cleanupForEnterPip(wct, container);
} else if (wasInPip) { } else if (wasInPip) {
// Exit PIP. // Exit PIP.
// Updates the presentation of the container. Expand or launch placeholder if needed. // Updates the presentation of the container. Expand or launch placeholder if needed.
mPresenter.updateContainer(container); updateContainer(wct, container);
} }
mPresenter.applyTransaction(wct);
updateCallbackIfNecessary(); updateCallbackIfNecessary();
} }
@@ -188,7 +191,15 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
final TaskFragmentContainer container = getContainer(taskFragmentInfo.getFragmentToken()); final TaskFragmentContainer container = getContainer(taskFragmentInfo.getFragmentToken());
if (container != null) { if (container != null) {
// Cleanup if the TaskFragment vanished is not requested by the organizer. // 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(); updateCallbackIfNecessary();
} }
cleanupTaskFragment(taskFragmentInfo.getFragmentToken()); cleanupTaskFragment(taskFragmentInfo.getFragmentToken());
@@ -452,7 +463,8 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
} }
/** Cleanups all the dependencies when the TaskFragment is entering PIP. */ /** 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 int taskId = container.getTaskId();
final TaskContainer taskContainer = mTaskContainers.get(taskId); final TaskContainer taskContainer = mTaskContainers.get(taskId);
if (taskContainer == null) { 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 // If there is any TaskFragment split with the PIP TaskFragment, update their presentations
// since the split is dismissed. // since the split is dismissed.
// We don't want to close any of them even if they are dependencies of the PIP TaskFragment. // 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. // TaskFragment there.
taskContainer.mFinishedContainer.add(container.getTaskFragmentToken()); taskContainer.mFinishedContainer.add(container.getTaskFragmentToken());
// Cleanup any split references.
final List<SplitContainer> containersToRemove = new ArrayList<>(); final List<SplitContainer> containersToRemove = new ArrayList<>();
for (SplitContainer splitContainer : taskContainer.mSplitContainers) { for (SplitContainer splitContainer : taskContainer.mSplitContainers) {
if (container.equals(splitContainer.getSecondaryContainer()) if (container.equals(splitContainer.getSecondaryContainer())
@@ -510,6 +525,11 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
} }
} }
taskContainer.mSplitContainers.removeAll(containersToRemove); taskContainer.mSplitContainers.removeAll(containersToRemove);
// Cleanup any dependent references.
for (TaskFragmentContainer containerToUpdate : taskContainer.mContainers) {
containerToUpdate.removeContainerToFinishOnExit(container);
}
} }
/** /**

View File

@@ -36,7 +36,6 @@ import androidx.annotation.IntDef;
import androidx.annotation.NonNull; import androidx.annotation.NonNull;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import java.util.Collection;
import java.util.concurrent.Executor; 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) { void cleanupContainer(@NonNull TaskFragmentContainer container, boolean shouldFinishDependent) {
if (containers.isEmpty()) {
return;
}
final WindowContainerTransaction wct = new WindowContainerTransaction(); final WindowContainerTransaction wct = new WindowContainerTransaction();
for (TaskFragmentContainer container : containers) { cleanupContainer(container, shouldFinishDependent, wct);
mController.updateContainer(wct, container);
}
applyTransaction(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 * Deletes the specified container and all other associated and dependent containers in the same
* transaction. * transaction.
*/ */
void cleanupContainer(@NonNull TaskFragmentContainer container, boolean shouldFinishDependent) { void cleanupContainer(@NonNull TaskFragmentContainer container, boolean shouldFinishDependent,
final WindowContainerTransaction wct = new WindowContainerTransaction(); @NonNull WindowContainerTransaction wct) {
container.finish(shouldFinishDependent, this, wct, mController); container.finish(shouldFinishDependent, this, wct, mController);
final TaskFragmentContainer newTopContainer = mController.getTopActiveContainer( final TaskFragmentContainer newTopContainer = mController.getTopActiveContainer(
@@ -100,8 +94,6 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer {
if (newTopContainer != null) { if (newTopContainer != null) {
mController.updateContainer(wct, newTopContainer); mController.updateContainer(wct, newTopContainer);
} }
applyTransaction(wct);
} }
/** /**

View File

@@ -257,7 +257,8 @@ class TaskFragmentContainer {
// Finish dependent containers // Finish dependent containers
for (TaskFragmentContainer container : mContainersToFinishOnExit) { for (TaskFragmentContainer container : mContainersToFinishOnExit) {
if (controller.shouldRetainAssociatedContainer(this, container)) { if (container.mIsFinished
|| controller.shouldRetainAssociatedContainer(this, container)) {
continue; continue;
} }
container.finish(true /* shouldFinishDependent */, presenter, container.finish(true /* shouldFinishDependent */, presenter,
@@ -267,18 +268,13 @@ class TaskFragmentContainer {
// Finish associated activities // Finish associated activities
for (Activity activity : mActivitiesToFinishOnExit) { for (Activity activity : mActivitiesToFinishOnExit) {
if (controller.shouldRetainAssociatedActivity(this, activity)) { if (activity.isFinishing()
|| controller.shouldRetainAssociatedActivity(this, activity)) {
continue; continue;
} }
activity.finish(); activity.finish();
} }
mActivitiesToFinishOnExit.clear(); mActivitiesToFinishOnExit.clear();
// Finish activities that were being re-parented to this container.
for (Activity activity : mPendingAppearedActivities) {
activity.finish();
}
mPendingAppearedActivities.clear();
} }
boolean isFinished() { boolean isFinished() {

View File

@@ -17,13 +17,20 @@
package androidx.window.extensions.embedding; package androidx.window.extensions.embedding;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn; 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 com.google.common.truth.Truth.assertWithMessage;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock; 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.platform.test.annotations.Presubmit;
import android.window.TaskFragmentInfo;
import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.filters.SmallTest; import androidx.test.filters.SmallTest;
@@ -32,12 +39,14 @@ import androidx.window.extensions.embedding.SplitController.TaskContainer;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
/** /**
* Test class for {@link SplitController}. * Test class for {@link SplitController}.
* *
* Build/Install/Run: * Build/Install/Run:
* atest WMJetpackUnitTests:SplitController * atest WMJetpackUnitTests:SplitControllerTest
*/ */
@Presubmit @Presubmit
@SmallTest @SmallTest
@@ -45,12 +54,24 @@ import org.junit.runner.RunWith;
public class SplitControllerTest { public class SplitControllerTest {
private static final int TASK_ID = 10; private static final int TASK_ID = 10;
@Mock
private Activity mActivity;
@Mock
private Resources mActivityResources;
@Mock
private TaskFragmentInfo mInfo;
private SplitController mSplitController; private SplitController mSplitController;
private SplitPresenter mSplitPresenter;
@Before @Before
public void setUp() { public void setUp() {
MockitoAnnotations.initMocks(this);
mSplitController = new SplitController(); mSplitController = new SplitController();
mSplitPresenter = mSplitController.mPresenter;
spyOn(mSplitController); spyOn(mSplitController);
spyOn(mSplitPresenter);
doReturn(mActivityResources).when(mActivity).getResources();
doReturn(new Configuration()).when(mActivityResources).getConfiguration();
} }
@Test @Test
@@ -83,4 +104,17 @@ public class SplitControllerTest {
assertWithMessage("Must return null because tf1 has no running activity.") assertWithMessage("Must return null because tf1 has no running activity.")
.that(mSplitController.getTopActiveContainer(TASK_ID)).isNull(); .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();
}
} }

View File

@@ -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);
}
}

View File

@@ -1522,7 +1522,10 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
sendTaskFragmentOperationFailure(organizer, errorCallbackToken, exception); sendTaskFragmentOperationFailure(organizer, errorCallbackToken, exception);
return 0; 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( final Throwable exception = new IllegalArgumentException(
"Not allowed to delete TaskFragment in PIP Task"); "Not allowed to delete TaskFragment in PIP Task");
sendTaskFragmentOperationFailure(organizer, errorCallbackToken, exception); sendTaskFragmentOperationFailure(organizer, errorCallbackToken, exception);

View File

@@ -527,6 +527,14 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
verify(mAtm.mWindowOrganizerController).sendTaskFragmentOperationFailure(eq(mIOrganizer), verify(mAtm.mWindowOrganizerController).sendTaskFragmentOperationFailure(eq(mIOrganizer),
eq(errorToken), any(IllegalArgumentException.class)); eq(errorToken), any(IllegalArgumentException.class));
assertNotNull(mAtm.mWindowOrganizerController.getTaskFragment(mFragmentToken)); 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 @Test