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
This commit is contained in:
Charles Chen
2023-03-13 20:50:20 +08:00
parent 28b2efa802
commit 1af8ee5bd8
5 changed files with 49 additions and 32 deletions

View File

@@ -162,7 +162,7 @@ public final class WindowMetrics {
return WindowMetrics.class.getSimpleName() + ":{"
+ "bounds=" + mBounds
+ ", windowInsets=" + mWindowInsets
+ ", density" + mDensity
+ ", density=" + mDensity
+ "}";
}
}

View File

@@ -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

View File

@@ -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<Size, Size> 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<Size, Size> 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<Size, Size> getActivitiesMinDimensionsPair(
static Pair<Size, Size> 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;

View File

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

View File

@@ -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(