From 93a5865a6fa071fb5876ac80374bf066c63c8b35 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Tue, 30 Aug 2022 17:16:42 +0800 Subject: [PATCH] Make sure the secondary TaskFragment is above the primary Before, we may reuse the existing TaskFragment for secondary activity, which may be below the primary TaskFragment. Now, we make sure the secondary TaskFragment is always on top. Bug: 244254584 Test: atest WMJetpackUnitTests:SplitPresenterTest Test: atest WMJetpackUnitTests:TaskFragmentContainerTest Change-Id: Id4c240af71fbd344126ae8bbbad57d9c2150d550 --- .../extensions/embedding/SplitController.java | 3 +++ .../extensions/embedding/SplitPresenter.java | 8 +++++-- .../extensions/embedding/TaskContainer.java | 4 ++++ .../embedding/TaskFragmentContainer.java | 12 +++++++++++ .../embedding/SplitPresenterTest.java | 21 +++++++++++++++++++ .../embedding/TaskFragmentContainerTest.java | 12 +++++++++++ 6 files changed, 58 insertions(+), 2 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 02af9160301c5..4102732fd80eb 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java @@ -792,6 +792,9 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen * Checks if there is a rule to split the two activities. If there is one, puts them into split * and returns {@code true}. Otherwise, returns {@code false}. */ + // Suppress GuardedBy warning because lint ask to mark this method as + // @GuardedBy(mPresenter.mController.mLock), which is mLock itself + @SuppressWarnings("GuardedBy") @GuardedBy("mLock") private boolean putActivitiesIntoSplitIfNecessary(@NonNull WindowContainerTransaction wct, @NonNull Activity primaryActivity, @NonNull Activity secondaryActivity) { 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 2b069d72e46ff..2ef8e4c64855c 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitPresenter.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitPresenter.java @@ -38,6 +38,7 @@ import android.view.WindowInsets; import android.view.WindowMetrics; import android.window.WindowContainerTransaction; +import androidx.annotation.GuardedBy; import androidx.annotation.IntDef; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -171,6 +172,7 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer { * created and the activity will be re-parented to it. * @param rule The split rule to be applied to the container. */ + @GuardedBy("mController.mLock") void createNewSplitContainer(@NonNull WindowContainerTransaction wct, @NonNull Activity primaryActivity, @NonNull Activity secondaryActivity, @NonNull SplitPairRule rule) { @@ -187,8 +189,10 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer { final TaskFragmentContainer curSecondaryContainer = mController.getContainerWithActivity( secondaryActivity); TaskFragmentContainer containerToAvoid = primaryContainer; - if (rule.shouldClearTop() && curSecondaryContainer != null) { - // Do not reuse the current TaskFragment if the rule is to clear top. + if (curSecondaryContainer != null + && (rule.shouldClearTop() || primaryContainer.isAbove(curSecondaryContainer))) { + // Do not reuse the current TaskFragment if the rule is to clear top, or if it is below + // the primary TaskFragment. containerToAvoid = curSecondaryContainer; } final TaskFragmentContainer secondaryContainer = prepareContainerForActivity(wct, 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 77e26c07f304d..45645b2018c34 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java @@ -162,4 +162,8 @@ class TaskContainer { } return null; } + + int indexOf(@NonNull TaskFragmentContainer child) { + return mContainers.indexOf(child); + } } 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 11c0db3206463..344ffc759c46a 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java @@ -501,6 +501,18 @@ class TaskFragmentContainer { return new Size(maxMinWidth, maxMinHeight); } + /** Whether the current TaskFragment is above the {@code other} TaskFragment. */ + boolean isAbove(@NonNull TaskFragmentContainer other) { + if (mTaskContainer != other.mTaskContainer) { + throw new IllegalArgumentException( + "Trying to compare two TaskFragments in different Task."); + } + if (this == other) { + throw new IllegalArgumentException("Trying to compare a TaskFragment with itself."); + } + return mTaskContainer.indexOf(this) > mTaskContainer.indexOf(other); + } + @Override public String toString() { return toString(true /* includeContainersToFinishOnExit */); diff --git a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitPresenterTest.java b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitPresenterTest.java index da724d9d9311d..25f0e25eec75c 100644 --- a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitPresenterTest.java +++ b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitPresenterTest.java @@ -39,6 +39,7 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -247,6 +248,26 @@ public class SplitPresenterTest { verify(mPresenter).expandTaskFragment(mTransaction, secondaryTf.getTaskFragmentToken()); } + @Test + public void testCreateNewSplitContainer_secondaryAbovePrimary() { + final Activity secondaryActivity = createMockActivity(); + final TaskFragmentContainer bottomTf = mController.newContainer(secondaryActivity, TASK_ID); + final TaskFragmentContainer primaryTf = mController.newContainer(mActivity, TASK_ID); + final SplitPairRule rule = new SplitPairRule.Builder(pair -> + pair.first == mActivity && pair.second == secondaryActivity, pair -> false, + metrics -> true) + .setShouldClearTop(false) + .build(); + + mPresenter.createNewSplitContainer(mTransaction, mActivity, secondaryActivity, rule); + + assertEquals(primaryTf, mController.getContainerWithActivity(mActivity)); + final TaskFragmentContainer secondaryTf = mController.getContainerWithActivity( + secondaryActivity); + assertNotEquals(bottomTf, secondaryTf); + assertTrue(secondaryTf.isAbove(primaryTf)); + } + private Activity createMockActivity() { final Activity activity = mock(Activity.class); final Configuration activityConfig = new Configuration(); 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 index 6cbecff81be55..1bc81ee3dcc71 100644 --- 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 @@ -287,6 +287,18 @@ public class TaskFragmentContainerTest { assertEquals(1, container.collectNonFinishingActivities().size()); } + @Test + public void testIsAbove() { + final TaskContainer taskContainer = new TaskContainer(TASK_ID); + final TaskFragmentContainer container0 = new TaskFragmentContainer(null /* activity */, + mIntent, taskContainer, mController); + final TaskFragmentContainer container1 = new TaskFragmentContainer(null /* activity */, + mIntent, taskContainer, mController); + + assertTrue(container1.isAbove(container0)); + assertFalse(container0.isAbove(container1)); + } + @Test public void testGetBottomMostActivity() { final TaskContainer taskContainer = new TaskContainer(TASK_ID);