From 1af8ee5bd8b6d5df6bab3c23dcbfe7610ca2981b Mon Sep 17 00:00:00 2001 From: Charles Chen Date: Mon, 13 Mar 2023 20:50:20 +0800 Subject: [PATCH] Checks SplitAttributes when reusing container Before this CL, we checks whether a container can be reused based on rules. However, there's a corner case that the SplitAttributesCalculator function is updated before a new activity is started. It may lead to the split pair has different splitAttributes even if activities matches the same rule. This CL also takes whether the calculated SplitAttributes match the SplitAttributes of the container into account. Bug: 241043377 Test: atest SplitAttributesCalculatorTest SplitControllerTest Change-Id: I4bd4797d9462fccc9be56fd1cdb422e27a79bcff --- core/java/android/view/WindowMetrics.java | 2 +- .../extensions/embedding/SplitController.java | 48 ++++++++++++++----- .../extensions/embedding/SplitPresenter.java | 18 +++---- .../embedding/SplitControllerTest.java | 10 ++-- .../embedding/SplitPresenterTest.java | 3 +- 5 files changed, 49 insertions(+), 32 deletions(-) diff --git a/core/java/android/view/WindowMetrics.java b/core/java/android/view/WindowMetrics.java index b74b80eeec2cf..7ad43c76efaa9 100644 --- a/core/java/android/view/WindowMetrics.java +++ b/core/java/android/view/WindowMetrics.java @@ -162,7 +162,7 @@ public final class WindowMetrics { return WindowMetrics.class.getSimpleName() + ":{" + "bounds=" + mBounds + ", windowInsets=" + mWindowInsets - + ", density" + mDensity + + ", density=" + mDensity + "}"; } } 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 404429ad41d39..89f4890c254ed 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java @@ -40,7 +40,9 @@ import static androidx.window.extensions.embedding.SplitContainer.isStickyPlaceh import static androidx.window.extensions.embedding.SplitContainer.shouldFinishAssociatedContainerWhenAdjacent; import static androidx.window.extensions.embedding.SplitContainer.shouldFinishAssociatedContainerWhenStacked; import static androidx.window.extensions.embedding.SplitPresenter.RESULT_EXPAND_FAILED_NO_TF_INFO; +import static androidx.window.extensions.embedding.SplitPresenter.getActivitiesMinDimensionsPair; import static androidx.window.extensions.embedding.SplitPresenter.getActivityIntentMinDimensionsPair; +import static androidx.window.extensions.embedding.SplitPresenter.getTaskWindowMetrics; import static androidx.window.extensions.embedding.SplitPresenter.shouldShowSplit; import android.app.Activity; @@ -1037,9 +1039,15 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen final TaskFragmentContainer primaryContainer = getContainerWithActivity( primaryActivity); final SplitContainer splitContainer = getActiveSplitForContainer(primaryContainer); - final WindowMetrics taskWindowMetrics = mPresenter.getTaskWindowMetrics(primaryActivity); + final TaskContainer.TaskProperties taskProperties = mPresenter + .getTaskProperties(primaryActivity); + final SplitAttributes calculatedSplitAttributes = mPresenter.computeSplitAttributes( + taskProperties, splitRule, splitRule.getDefaultSplitAttributes(), + getActivitiesMinDimensionsPair(primaryActivity, secondaryActivity)); if (splitContainer != null && primaryContainer == splitContainer.getPrimaryContainer() - && canReuseContainer(splitRule, splitContainer.getSplitRule(), taskWindowMetrics)) { + && canReuseContainer(splitRule, splitContainer.getSplitRule(), + getTaskWindowMetrics(taskProperties.getConfiguration()), + calculatedSplitAttributes, splitContainer.getCurrentSplitAttributes())) { // Can launch in the existing secondary container if the rules share the same // presentation. final TaskFragmentContainer secondaryContainer = splitContainer.getSecondaryContainer(); @@ -1058,7 +1066,8 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen } } // Create new split pair. - mPresenter.createNewSplitContainer(wct, primaryActivity, secondaryActivity, splitRule); + mPresenter.createNewSplitContainer(wct, primaryActivity, secondaryActivity, splitRule, + calculatedSplitAttributes); return true; } @@ -1283,9 +1292,16 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen } final TaskFragmentContainer existingContainer = getContainerWithActivity(primaryActivity); final SplitContainer splitContainer = getActiveSplitForContainer(existingContainer); - final WindowMetrics taskWindowMetrics = mPresenter.getTaskWindowMetrics(primaryActivity); + final TaskContainer.TaskProperties taskProperties = mPresenter + .getTaskProperties(primaryActivity); + final WindowMetrics taskWindowMetrics = getTaskWindowMetrics( + taskProperties.getConfiguration()); + final SplitAttributes calculatedSplitAttributes = mPresenter.computeSplitAttributes( + taskProperties, splitRule, splitRule.getDefaultSplitAttributes(), + getActivityIntentMinDimensionsPair(primaryActivity, intent)); if (splitContainer != null && existingContainer == splitContainer.getPrimaryContainer() - && (canReuseContainer(splitRule, splitContainer.getSplitRule(), taskWindowMetrics) + && (canReuseContainer(splitRule, splitContainer.getSplitRule(), taskWindowMetrics, + calculatedSplitAttributes, splitContainer.getCurrentSplitAttributes()) // TODO(b/231845476) we should always respect clearTop. || !respectClearTop) && mPresenter.expandSplitContainerIfNeeded(wct, splitContainer, primaryActivity, @@ -1296,7 +1312,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen } // Create a new TaskFragment to split with the primary activity for the new activity. return mPresenter.createNewSplitWithEmptySideContainer(wct, primaryActivity, intent, - splitRule); + splitRule, calculatedSplitAttributes); } /** @@ -2273,21 +2289,29 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen } /** - * If the two rules have the same presentation, we can reuse the same {@link SplitContainer} if - * there is any. + * If the two rules have the same presentation, and the calculated {@link SplitAttributes} + * matches the {@link SplitAttributes} of {@link SplitContainer}, we can reuse the same + * {@link SplitContainer} if there is any. */ private static boolean canReuseContainer(@NonNull SplitRule rule1, @NonNull SplitRule rule2, - @NonNull WindowMetrics parentWindowMetrics) { + @NonNull WindowMetrics parentWindowMetrics, + @NonNull SplitAttributes calculatedSplitAttributes, + @NonNull SplitAttributes containerSplitAttributes) { if (!isContainerReusableRule(rule1) || !isContainerReusableRule(rule2)) { return false; } - return haveSamePresentation((SplitPairRule) rule1, (SplitPairRule) rule2, - parentWindowMetrics); + return areRulesSamePresentation((SplitPairRule) rule1, (SplitPairRule) rule2, + parentWindowMetrics) + // Besides rules, we should also check whether the SplitContainer's splitAttributes + // matches the current splitAttributes or not. The splitAttributes may change + // if the app chooses different SplitAttributes calculator function before a new + // activity is started even they match the same splitRule. + && calculatedSplitAttributes.equals(containerSplitAttributes); } /** Whether the two rules have the same presentation. */ @VisibleForTesting - static boolean haveSamePresentation(@NonNull SplitPairRule rule1, + static boolean areRulesSamePresentation(@NonNull SplitPairRule rule1, @NonNull SplitPairRule rule2, @NonNull WindowMetrics parentWindowMetrics) { if (rule1.getTag() != null || rule2.getTag() != null) { // Tag must be unique if it is set. We don't want to reuse the container if the rules 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 040851181e92b..20b6ae29939c8 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitPresenter.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitPresenter.java @@ -174,12 +174,9 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer { @NonNull TaskFragmentContainer createNewSplitWithEmptySideContainer( @NonNull WindowContainerTransaction wct, @NonNull Activity primaryActivity, - @NonNull Intent secondaryIntent, @NonNull SplitPairRule rule) { + @NonNull Intent secondaryIntent, @NonNull SplitPairRule rule, + @NonNull SplitAttributes splitAttributes) { final TaskProperties taskProperties = getTaskProperties(primaryActivity); - final Pair minDimensionsPair = getActivityIntentMinDimensionsPair( - primaryActivity, secondaryIntent); - final SplitAttributes splitAttributes = computeSplitAttributes(taskProperties, rule, - rule.getDefaultSplitAttributes(), minDimensionsPair); final Rect primaryRelBounds = getRelBoundsForPosition(POSITION_START, taskProperties, splitAttributes); final TaskFragmentContainer primaryContainer = prepareContainerForActivity(wct, @@ -217,15 +214,12 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer { * same container as the primary activity, a new container will be * created and the activity will be re-parented to it. * @param rule The split rule to be applied to the container. + * @param splitAttributes The {@link SplitAttributes} to apply */ void createNewSplitContainer(@NonNull WindowContainerTransaction wct, @NonNull Activity primaryActivity, @NonNull Activity secondaryActivity, - @NonNull SplitPairRule rule) { + @NonNull SplitPairRule rule, @NonNull SplitAttributes splitAttributes) { final TaskProperties taskProperties = getTaskProperties(primaryActivity); - final Pair minDimensionsPair = getActivitiesMinDimensionsPair(primaryActivity, - secondaryActivity); - final SplitAttributes splitAttributes = computeSplitAttributes(taskProperties, rule, - rule.getDefaultSplitAttributes(), minDimensionsPair); final Rect primaryRelBounds = getRelBoundsForPosition(POSITION_START, taskProperties, splitAttributes); final TaskFragmentContainer primaryContainer = prepareContainerForActivity(wct, @@ -654,7 +648,7 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer { } @NonNull - private static Pair getActivitiesMinDimensionsPair( + static Pair getActivitiesMinDimensionsPair( @NonNull Activity primaryActivity, @NonNull Activity secondaryActivity) { return new Pair<>(getMinDimensions(primaryActivity), getMinDimensions(secondaryActivity)); } @@ -1027,7 +1021,7 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer { } @NonNull - private static WindowMetrics getTaskWindowMetrics(@NonNull Configuration taskConfiguration) { + static WindowMetrics getTaskWindowMetrics(@NonNull Configuration taskConfiguration) { final Rect taskBounds = taskConfiguration.windowConfiguration.getBounds(); // TODO(b/190433398): Supply correct insets. final float density = taskConfiguration.densityDpi * DisplayMetrics.DENSITY_DEFAULT_SCALE; 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 cbdc262c05948..ff08782e8cd8d 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 @@ -565,7 +565,6 @@ public class SplitControllerTest { assertNotNull(mSplitController.getActiveSplitForContainers(primaryContainer, container)); assertTrue(primaryContainer.areLastRequestedBoundsEqual(null)); assertTrue(container.areLastRequestedBoundsEqual(null)); - assertEquals(container, mSplitController.getContainerWithActivity(secondaryActivity)); } @Test @@ -1008,9 +1007,8 @@ public class SplitControllerTest { assertTrue(result); assertSplitPair(primaryActivity, mActivity, true /* matchParentBounds */); - assertEquals(mSplitController.getContainerWithActivity(secondaryActivity), - mSplitController.getContainerWithActivity(mActivity)); - verify(mSplitPresenter, never()).createNewSplitContainer(any(), any(), any(), any()); + assertTrue(mSplitController.getContainerWithActivity(mActivity) + .areLastRequestedBoundsEqual(new Rect())); } @Test @@ -1215,7 +1213,7 @@ public class SplitControllerTest { .build(); assertTrue("Rules must have same presentation if tags are null and has same properties.", - SplitController.haveSamePresentation(splitRule1, splitRule2, + SplitController.areRulesSamePresentation(splitRule1, splitRule2, new WindowMetrics(TASK_BOUNDS, WindowInsets.CONSUMED))); splitRule2 = createSplitPairRuleBuilder( @@ -1230,7 +1228,7 @@ public class SplitControllerTest { assertFalse("Rules must have different presentations if tags are not equal regardless" + "of other properties", - SplitController.haveSamePresentation(splitRule1, splitRule2, + SplitController.areRulesSamePresentation(splitRule1, splitRule2, new WindowMetrics(TASK_BOUNDS, WindowInsets.CONSUMED))); } 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 83301564d7a4c..8dd13846ab3d7 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 @@ -678,7 +678,8 @@ public class SplitPresenterTest { .setShouldClearTop(false) .build(); - mPresenter.createNewSplitContainer(mTransaction, mActivity, secondaryActivity, rule); + mPresenter.createNewSplitContainer(mTransaction, mActivity, secondaryActivity, rule, + SPLIT_ATTRIBUTES); assertEquals(primaryTf, mController.getContainerWithActivity(mActivity)); final TaskFragmentContainer secondaryTf = mController.getContainerWithActivity(