From 3e93b2623c8cede5e83b5b906133136d41fc3bab Mon Sep 17 00:00:00 2001 From: Charles Chen Date: Fri, 22 Apr 2022 12:34:50 +0800 Subject: [PATCH] Update SplitContainer for non-finishing containers If an app is split with an Activity from another process by Activity Embedding feature, and is launched twice with different orientations, there's a chance that the app still shows in previous orientation at the second launch even if the orientation is changed. The root cause is that, the app goes back to background before the secondary container receives its TaskFragmentInfo. Also, there's no pendingActivities and #onActivityPostCreated won't be invoked, and also #updateContainer to change its layout, because the Activity is from other process. Moreover, in #onTaskFragmentParentInfoChanged, updateContainer is skipped because secondary container is empty(No pending activities, and hasn't received its TaskFragmentInfo). This CL fixes the issue by verifying if one or both containers are finished instead. fixes: 228906392 Test: atest SplitControllerTest Test: manual - reproducible steps in bug Change-Id: Ifba4b56c9225089776835aeeef7c4a13249b55bf --- .../extensions/embedding/SplitController.java | 14 ++-- .../extensions/embedding/TaskContainer.java | 2 + .../embedding/TaskFragmentContainer.java | 7 +- .../embedding/SplitControllerTest.java | 78 +++++++++++++++++++ 4 files changed, 91 insertions(+), 10 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 b370e59ac7c83..402551fd466c2 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java @@ -619,14 +619,13 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen } final List splitContainers = mTaskContainers.get(container.getTaskId()) .mSplitContainers; - if (splitContainers == null - || splitContainer != splitContainers.get(splitContainers.size() - 1)) { + if (splitContainer != splitContainers.get(splitContainers.size() - 1)) { // Skip position update - it isn't the topmost split. return; } - if (splitContainer.getPrimaryContainer().isEmpty() - || splitContainer.getSecondaryContainer().isEmpty()) { - // Skip position update - one or both containers are empty. + if (splitContainer.getPrimaryContainer().isFinished() + || splitContainer.getSecondaryContainer().isFinished()) { + // Skip position update - one or both containers are finished. return; } if (dismissPlaceholderIfNecessary(splitContainer)) { @@ -643,7 +642,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen private SplitContainer getActiveSplitForContainer(@NonNull TaskFragmentContainer container) { final List splitContainers = mTaskContainers.get(container.getTaskId()) .mSplitContainers; - if (splitContainers == null) { + if (splitContainers.isEmpty()) { return null; } for (int i = splitContainers.size() - 1; i >= 0; i--) { @@ -721,7 +720,8 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen return true; } - private boolean dismissPlaceholderIfNecessary(@NonNull SplitContainer splitContainer) { + @VisibleForTesting + boolean dismissPlaceholderIfNecessary(@NonNull SplitContainer splitContainer) { if (!splitContainer.isPlaceholderContainer()) { return false; } diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java index 3c0762d81494e..916e07e24a728 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java @@ -47,9 +47,11 @@ class TaskContainer { private int mWindowingMode = WINDOWING_MODE_UNDEFINED; /** Active TaskFragments in this Task. */ + @NonNull final List mContainers = new ArrayList<>(); /** Active split pairs in this Task. */ + @NonNull final List mSplitContainers = new ArrayList<>(); /** 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 871f545d203a1..b2ceb514325b6 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java @@ -319,14 +319,15 @@ class TaskFragmentContainer { private String toString(boolean includeContainersToFinishOnExit) { return "TaskFragmentContainer{" + " token=" + mToken - + " info=" + mInfo + " topNonFinishingActivity=" + getTopNonFinishingActivity() + + " runningActivityCount=" + getRunningActivityCount() + + " isFinished=" + mIsFinished + + " lastRequestedBounds=" + mLastRequestedBounds + " pendingAppearedActivities=" + mPendingAppearedActivities + (includeContainersToFinishOnExit ? " containersToFinishOnExit=" + containersToFinishOnExitToString() : "") + " activitiesToFinishOnExit=" + mActivitiesToFinishOnExit - + " isFinished=" + mIsFinished - + " lastRequestedBounds=" + mLastRequestedBounds + + " info=" + mInfo + "}"; } 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 e0fda58fd6649..32c61eaa6b50d 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 @@ -25,6 +25,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -35,6 +37,7 @@ import android.content.res.Resources; import android.graphics.Rect; 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; @@ -45,6 +48,8 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.List; + /** * Test class for {@link SplitController}. * @@ -64,6 +69,8 @@ public class SplitControllerTest { private Resources mActivityResources; @Mock private TaskFragmentInfo mInfo; + @Mock + private WindowContainerTransaction mTransaction; private SplitController mSplitController; private SplitPresenter mSplitPresenter; @@ -140,4 +147,75 @@ public class SplitControllerTest { assertNotNull(taskContainer); assertEquals(TASK_BOUNDS, taskContainer.getTaskBounds()); } + + @Test + public void testUpdateContainer() { + // Make SplitController#launchPlaceholderIfNecessary(TaskFragmentContainer) return true + // and verify if shouldContainerBeExpanded() not called. + final TaskFragmentContainer tf = mSplitController.newContainer(mActivity, TASK_ID); + spyOn(tf); + doReturn(mActivity).when(tf).getTopNonFinishingActivity(); + doReturn(true).when(tf).isEmpty(); + doReturn(true).when(mSplitController).launchPlaceholderIfNecessary(mActivity); + doNothing().when(mSplitPresenter).updateSplitContainer(any(), any(), any()); + + mSplitController.updateContainer(mTransaction, tf); + + verify(mSplitController, never()).shouldContainerBeExpanded(any()); + + // Verify if tf should be expanded, getTopActiveContainer() won't be called + doReturn(null).when(tf).getTopNonFinishingActivity(); + doReturn(true).when(mSplitController).shouldContainerBeExpanded(tf); + + mSplitController.updateContainer(mTransaction, tf); + + verify(mSplitController, never()).getTopActiveContainer(TASK_ID); + + // Verify if tf is not in split, dismissPlaceholderIfNecessary won't be called. + doReturn(false).when(mSplitController).shouldContainerBeExpanded(tf); + + mSplitController.updateContainer(mTransaction, tf); + + verify(mSplitController, never()).dismissPlaceholderIfNecessary(any()); + + // Verify if tf is not in the top splitContainer, + final SplitContainer splitContainer = mock(SplitContainer.class); + doReturn(tf).when(splitContainer).getPrimaryContainer(); + doReturn(tf).when(splitContainer).getSecondaryContainer(); + final List splitContainers = + mSplitController.getTaskContainer(TASK_ID).mSplitContainers; + splitContainers.add(splitContainer); + // Add a mock SplitContainer on top of splitContainer + splitContainers.add(1, mock(SplitContainer.class)); + + mSplitController.updateContainer(mTransaction, tf); + + verify(mSplitController, never()).dismissPlaceholderIfNecessary(any()); + + // Verify if one or both containers in the top SplitContainer are finished, + // dismissPlaceholder() won't be called. + splitContainers.remove(1); + doReturn(true).when(tf).isFinished(); + + mSplitController.updateContainer(mTransaction, tf); + + verify(mSplitController, never()).dismissPlaceholderIfNecessary(any()); + + // Verify if placeholder should be dismissed, updateSplitContainer() won't be called. + doReturn(false).when(tf).isFinished(); + doReturn(true).when(mSplitController) + .dismissPlaceholderIfNecessary(splitContainer); + + mSplitController.updateContainer(mTransaction, tf); + + verify(mSplitPresenter, never()).updateSplitContainer(any(), any(), any()); + + // Verify if the top active split is updated if both of its containers are not finished. + doReturn(false).when(mSplitController) + .dismissPlaceholderIfNecessary(splitContainer); + + mSplitController.updateContainer(mTransaction, tf); + + verify(mSplitPresenter).updateSplitContainer(eq(splitContainer), eq(tf), eq(mTransaction)); + } }