Refactor getDestinationBounds into more readable methods
getDestinationBounds(Rect) was only being used to calculate
the "entry" bounds (e.g. onTaskAppeared, swipeToHome) and
all its callers were passing in null.
getDestinationBounds(Rect, boolean) had only one caller,
onTaskInfoChanged, who did pass the current bounds in to
be adjusted to the aspect ratio.
Instead of having overloaded methods and complex logic to
know whether to calculate default bounds or use the provided
bounds, the method is now split into two methods that better
represent what they're calculating:
1. getEntryDestinationBounds, no arguments, calculates either
default or reentry bounds
2. getAdjustedDestinationBounds, takes in the current bounds
and the new aspect ratio, simply adjusts the bounds to the
aspect ratio.
Bug: 169373982
Test: atest com.android.wm.shell.pip
Change-Id: Ie8adda458fc5a56b9d78245c663dbc54b8525378
diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipBoundsHandler.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipBoundsHandler.java
index ef2f897..16c0566 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipBoundsHandler.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipBoundsHandler.java
@@ -156,41 +156,28 @@
reloadResources(context);
}
- /**
- * See {@link #getDestinationBounds(Rect, boolean)}
- */
- public Rect getDestinationBounds(Rect bounds) {
- return getDestinationBounds(bounds, false /* useCurrentMinEdgeSize */);
- }
+ /** Returns the destination bounds to place the PIP window on entry. */
+ public Rect getEntryDestinationBounds() {
+ final PipBoundsState.PipReentryState reentryState = mPipBoundsState.getReentryState();
+ final boolean shouldRestoreReentryBounds = reentryState != null;
- /**
- * @return {@link Rect} of the destination PiP window bounds.
- */
- public Rect getDestinationBounds(Rect bounds, boolean useCurrentMinEdgeSize) {
- boolean isReentryBounds = false;
- final Rect destinationBounds;
- if (bounds == null) {
- // Calculating initial entry bounds
- final PipBoundsState.PipReentryState state = mPipBoundsState.getReentryState();
+ final Rect destinationBounds = shouldRestoreReentryBounds
+ ? getDefaultBounds(reentryState.getSnapFraction(), reentryState.getSize())
+ : getDefaultBounds(INVALID_SNAP_FRACTION, null /* size */);
- final Rect defaultBounds;
- if (state != null) {
- // Restore to reentry bounds.
- defaultBounds = getDefaultBounds(state.getSnapFraction(), state.getSize());
- isReentryBounds = true;
- } else {
- // Get actual default bounds.
- defaultBounds = getDefaultBounds(INVALID_SNAP_FRACTION, null /* size */);
- }
-
- destinationBounds = new Rect(defaultBounds);
- } else {
- // Just adjusting bounds (e.g. on aspect ratio changed).
- destinationBounds = new Rect(bounds);
- }
if (isValidPictureInPictureAspectRatio(mPipBoundsState.getAspectRatio())) {
transformBoundsToAspectRatio(destinationBounds, mPipBoundsState.getAspectRatio(),
- useCurrentMinEdgeSize, isReentryBounds);
+ false /* useCurrentMinEdgeSize */, shouldRestoreReentryBounds);
+ }
+ return destinationBounds;
+ }
+
+ /** Returns the current bounds adjusted to the new aspect ratio, if valid. */
+ public Rect getAdjustedDestinationBounds(Rect currentBounds, float newAspectRatio) {
+ final Rect destinationBounds = new Rect(currentBounds);
+ if (isValidPictureInPictureAspectRatio(newAspectRatio)) {
+ transformBoundsToAspectRatio(destinationBounds, newAspectRatio,
+ true /* useCurrentMinEdgeSize */, false /* isReentryBounds */);
}
return destinationBounds;
}
diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTaskOrganizer.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTaskOrganizer.java
index 1222c06..0bd74e3 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTaskOrganizer.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTaskOrganizer.java
@@ -340,7 +340,7 @@
mShouldIgnoreEnteringPipTransition = true;
sendOnPipTransitionStarted(componentName, TRANSITION_DIRECTION_TO_PIP);
setBoundsStateForEntry(componentName, pictureInPictureParams, activityInfo);
- return mPipBoundsHandler.getDestinationBounds(null /* bounds */);
+ return mPipBoundsHandler.getEntryDestinationBounds();
}
/**
@@ -526,7 +526,7 @@
return;
}
- final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(null /* bounds */);
+ final Rect destinationBounds = mPipBoundsHandler.getEntryDestinationBounds();
Objects.requireNonNull(destinationBounds, "Missing destination bounds");
final Rect currentBounds = mTaskInfo.configuration.windowConfiguration.getBounds();
@@ -685,9 +685,9 @@
Log.d(TAG, "Ignored onTaskInfoChanged with PiP param: " + newParams);
return;
}
- // Aspect ratio changed, re-calculate destination bounds.
- final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
- mPipBoundsState.getBounds(), true /* useCurrentMinEdgeSize */);
+ // Aspect ratio changed, re-calculate bounds if valid.
+ final Rect destinationBounds = mPipBoundsHandler.getAdjustedDestinationBounds(
+ mPipBoundsState.getBounds(), mPipBoundsState.getAspectRatio());
Objects.requireNonNull(destinationBounds, "Missing destination bounds");
scheduleAnimateResizePip(destinationBounds, mEnterExitAnimationDuration,
null /* updateBoundsCallback */);
@@ -701,8 +701,7 @@
@Override
public void onFixedRotationFinished(int displayId) {
if (mShouldDeferEnteringPip && mState.isInPip()) {
- final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
- null /* bounds */);
+ final Rect destinationBounds = mPipBoundsHandler.getEntryDestinationBounds();
// schedule a regular animation to ensure all the callbacks are still being sent
enterPipWithAlphaAnimation(destinationBounds, 0 /* durationMs */);
}
@@ -777,7 +776,7 @@
return;
}
- final Rect newDestinationBounds = mPipBoundsHandler.getDestinationBounds(null /* bounds */);
+ final Rect newDestinationBounds = mPipBoundsHandler.getEntryDestinationBounds();
if (newDestinationBounds.equals(currentDestinationBounds)) return;
if (animator.getAnimationType() == ANIM_TYPE_BOUNDS) {
animator.updateEndValue(newDestinationBounds);
diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/PipBoundsHandlerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/PipBoundsHandlerTest.java
index 28bfa46..440e23c 100644
--- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/PipBoundsHandlerTest.java
+++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/PipBoundsHandlerTest.java
@@ -17,7 +17,6 @@
package com.android.wm.shell.pip;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import android.graphics.Rect;
@@ -51,8 +50,6 @@
private static final float DEFAULT_ASPECT_RATIO = 1f;
private static final float MIN_ASPECT_RATIO = 0.5f;
private static final float MAX_ASPECT_RATIO = 2f;
- private static final Rect EMPTY_CURRENT_BOUNDS = null;
- private static final Size EMPTY_MINIMAL_SIZE = null;
private PipBoundsHandler mPipBoundsHandler;
private DisplayInfo mDefaultDisplayInfo;
@@ -115,7 +112,7 @@
}
@Test
- public void getDestinationBounds_returnBoundsMatchesAspectRatio() {
+ public void getEntryDestinationBounds_returnBoundsMatchesAspectRatio() {
final float[] aspectRatios = new float[] {
(MIN_ASPECT_RATIO + DEFAULT_ASPECT_RATIO) / 2,
DEFAULT_ASPECT_RATIO,
@@ -123,8 +120,7 @@
};
for (float aspectRatio : aspectRatios) {
mPipBoundsState.setAspectRatio(aspectRatio);
- final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
- EMPTY_CURRENT_BOUNDS);
+ final Rect destinationBounds = mPipBoundsHandler.getEntryDestinationBounds();
final float actualAspectRatio =
destinationBounds.width() / (destinationBounds.height() * 1f);
assertEquals("Destination bounds matches the given aspect ratio",
@@ -133,15 +129,14 @@
}
@Test
- public void getDestinationBounds_invalidAspectRatio_returnsDefaultAspectRatio() {
+ public void getEntryDestinationBounds_invalidAspectRatio_returnsDefaultAspectRatio() {
final float[] invalidAspectRatios = new float[] {
MIN_ASPECT_RATIO / 2,
MAX_ASPECT_RATIO * 2
};
for (float aspectRatio : invalidAspectRatios) {
mPipBoundsState.setAspectRatio(aspectRatio);
- final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
- EMPTY_CURRENT_BOUNDS);
+ final Rect destinationBounds = mPipBoundsHandler.getEntryDestinationBounds();
final float actualAspectRatio =
destinationBounds.width() / (destinationBounds.height() * 1f);
assertEquals("Destination bounds fallbacks to default aspect ratio",
@@ -151,13 +146,14 @@
}
@Test
- public void getDestinationBounds_withCurrentBounds_returnBoundsMatchesAspectRatio() {
+ public void getAdjustedDestinationBounds_returnBoundsMatchesAspectRatio() {
final float aspectRatio = (DEFAULT_ASPECT_RATIO + MAX_ASPECT_RATIO) / 2;
final Rect currentBounds = new Rect(0, 0, 0, 100);
currentBounds.right = (int) (currentBounds.height() * aspectRatio) + currentBounds.left;
mPipBoundsState.setAspectRatio(aspectRatio);
- final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(currentBounds);
+ final Rect destinationBounds = mPipBoundsHandler.getAdjustedDestinationBounds(
+ currentBounds, aspectRatio);
final float actualAspectRatio =
destinationBounds.width() / (destinationBounds.height() * 1f);
@@ -166,7 +162,7 @@
}
@Test
- public void getDestinationBounds_withMinSize_returnMinBounds() {
+ public void getEntryDestinationBounds_withMinSize_returnMinBounds() {
final float[] aspectRatios = new float[] {
(MIN_ASPECT_RATIO + DEFAULT_ASPECT_RATIO) / 2,
DEFAULT_ASPECT_RATIO,
@@ -182,8 +178,7 @@
final Size minimalSize = minimalSizes[i];
mPipBoundsState.setAspectRatio(aspectRatio);
mPipBoundsState.setOverrideMinSize(minimalSize);
- final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
- EMPTY_CURRENT_BOUNDS);
+ final Rect destinationBounds = mPipBoundsHandler.getEntryDestinationBounds();
assertTrue("Destination bounds is no smaller than minimal requirement",
(destinationBounds.width() == minimalSize.getWidth()
&& destinationBounds.height() >= minimalSize.getHeight())
@@ -197,7 +192,7 @@
}
@Test
- public void getDestinationBounds_withCurrentBounds_ignoreMinBounds() {
+ public void getAdjustedDestinationBounds_ignoreMinBounds() {
final float aspectRatio = (DEFAULT_ASPECT_RATIO + MAX_ASPECT_RATIO) / 2;
final Rect currentBounds = new Rect(0, 0, 0, 100);
currentBounds.right = (int) (currentBounds.height() * aspectRatio) + currentBounds.left;
@@ -205,8 +200,8 @@
mPipBoundsState.setAspectRatio(aspectRatio);
mPipBoundsState.setOverrideMinSize(minSize);
- final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
- currentBounds, true /* useCurrentMinEdgeSize */);
+ final Rect destinationBounds = mPipBoundsHandler.getAdjustedDestinationBounds(
+ currentBounds, aspectRatio);
assertTrue("Destination bounds ignores minimal size",
destinationBounds.width() > minSize.getWidth()
@@ -214,33 +209,29 @@
}
@Test
- public void getDestinationBounds_reentryStateExists_restoreLastSize() {
+ public void getEntryDestinationBounds_reentryStateExists_restoreLastSize() {
mPipBoundsState.setAspectRatio(DEFAULT_ASPECT_RATIO);
- final Rect reentryBounds = mPipBoundsHandler.getDestinationBounds(
- EMPTY_CURRENT_BOUNDS);
+ final Rect reentryBounds = mPipBoundsHandler.getEntryDestinationBounds();
reentryBounds.scale(1.25f);
final float reentrySnapFraction = mPipBoundsHandler.getSnapFraction(reentryBounds);
mPipBoundsState.saveReentryState(reentryBounds, reentrySnapFraction);
- final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
- EMPTY_CURRENT_BOUNDS);
+ final Rect destinationBounds = mPipBoundsHandler.getEntryDestinationBounds();
assertEquals(reentryBounds.width(), destinationBounds.width());
assertEquals(reentryBounds.height(), destinationBounds.height());
}
@Test
- public void getDestinationBounds_reentryStateExists_restoreLastPosition() {
+ public void getEntryDestinationBounds_reentryStateExists_restoreLastPosition() {
mPipBoundsState.setAspectRatio(DEFAULT_ASPECT_RATIO);
- final Rect reentryBounds = mPipBoundsHandler.getDestinationBounds(
- EMPTY_CURRENT_BOUNDS);
+ final Rect reentryBounds = mPipBoundsHandler.getEntryDestinationBounds();
reentryBounds.offset(0, -100);
final float reentrySnapFraction = mPipBoundsHandler.getSnapFraction(reentryBounds);
mPipBoundsState.saveReentryState(reentryBounds, reentrySnapFraction);
- final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
- EMPTY_CURRENT_BOUNDS);
+ final Rect destinationBounds = mPipBoundsHandler.getEntryDestinationBounds();
assertBoundsInclusionWithMargin("restoreLastPosition", reentryBounds, destinationBounds);
}
@@ -249,12 +240,10 @@
public void setShelfHeight_offsetBounds() {
final int shelfHeight = 100;
mPipBoundsState.setAspectRatio(DEFAULT_ASPECT_RATIO);
- final Rect oldPosition = mPipBoundsHandler.getDestinationBounds(
- EMPTY_CURRENT_BOUNDS);
+ final Rect oldPosition = mPipBoundsHandler.getEntryDestinationBounds();
mPipBoundsHandler.setShelfHeight(true, shelfHeight);
- final Rect newPosition = mPipBoundsHandler.getDestinationBounds(
- EMPTY_CURRENT_BOUNDS);
+ final Rect newPosition = mPipBoundsHandler.getEntryDestinationBounds();
oldPosition.offset(0, -shelfHeight);
assertBoundsInclusionWithMargin("offsetBounds by shelf", oldPosition, newPosition);
@@ -264,27 +253,23 @@
public void onImeVisibilityChanged_offsetBounds() {
final int imeHeight = 100;
mPipBoundsState.setAspectRatio(DEFAULT_ASPECT_RATIO);
- final Rect oldPosition = mPipBoundsHandler.getDestinationBounds(
- EMPTY_CURRENT_BOUNDS);
+ final Rect oldPosition = mPipBoundsHandler.getEntryDestinationBounds();
mPipBoundsHandler.onImeVisibilityChanged(true, imeHeight);
- final Rect newPosition = mPipBoundsHandler.getDestinationBounds(
- EMPTY_CURRENT_BOUNDS);
+ final Rect newPosition = mPipBoundsHandler.getEntryDestinationBounds();
oldPosition.offset(0, -imeHeight);
assertBoundsInclusionWithMargin("offsetBounds by IME", oldPosition, newPosition);
}
@Test
- public void getDestinationBounds_noReentryState_useDefaultBounds() {
+ public void getEntryDestinationBounds_noReentryState_useDefaultBounds() {
mPipBoundsState.setAspectRatio(DEFAULT_ASPECT_RATIO);
- final Rect defaultBounds = mPipBoundsHandler.getDestinationBounds(
- EMPTY_CURRENT_BOUNDS);
+ final Rect defaultBounds = mPipBoundsHandler.getEntryDestinationBounds();
mPipBoundsState.clearReentryState();
- final Rect actualBounds = mPipBoundsHandler.getDestinationBounds(
- EMPTY_CURRENT_BOUNDS);
+ final Rect actualBounds = mPipBoundsHandler.getEntryDestinationBounds();
assertBoundsInclusionWithMargin("useDefaultBounds", defaultBounds, actualBounds);
}
@@ -297,13 +282,4 @@
+ " with error margin " + ROUNDING_ERROR_MARGIN,
expectedWithMargin.contains(actual));
}
-
- private void assertNonBoundsInclusionWithMargin(String from, Rect expected, Rect actual) {
- final Rect expectedWithMargin = new Rect(expected);
- expectedWithMargin.inset(-ROUNDING_ERROR_MARGIN, -ROUNDING_ERROR_MARGIN);
- assertFalse(from + ": expect " + expected
- + " not contains " + actual
- + " with error margin " + ROUNDING_ERROR_MARGIN,
- expectedWithMargin.contains(actual));
- }
}
diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/PipTaskOrganizerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/PipTaskOrganizerTest.java
index ea6092a..08841bd 100644
--- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/PipTaskOrganizerTest.java
+++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/PipTaskOrganizerTest.java
@@ -18,7 +18,7 @@
import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyFloat;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.doNothing;
@@ -192,8 +192,8 @@
private void preparePipTaskOrg() {
final DisplayInfo info = new DisplayInfo();
mPipBoundsState.setDisplayInfo(info);
- when(mMockPipBoundsHandler.getDestinationBounds(any())).thenReturn(new Rect());
- when(mMockPipBoundsHandler.getDestinationBounds(any(), anyBoolean()))
+ when(mMockPipBoundsHandler.getEntryDestinationBounds()).thenReturn(new Rect());
+ when(mMockPipBoundsHandler.getAdjustedDestinationBounds(any(), anyFloat()))
.thenReturn(new Rect());
mPipBoundsState.setDisplayInfo(info);
mSpiedPipTaskOrganizer.setOneShotAnimationType(PipAnimationController.ANIM_TYPE_ALPHA);