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

4 files changed, 53 insertions, 91 deletions

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 ef2f897a52e7..16c0566a29b5 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 @@ public class PipBoundsHandler {
         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 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 */);
-            }
+        final Rect destinationBounds = shouldRestoreReentryBounds
+                ? getDefaultBounds(reentryState.getSnapFraction(), reentryState.getSize())
+                : 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 1222c060bd98..0bd74e39bb5e 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 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
         mShouldIgnoreEnteringPipTransition = true;
         sendOnPipTransitionStarted(componentName, TRANSITION_DIRECTION_TO_PIP);
         setBoundsStateForEntry(componentName, pictureInPictureParams, activityInfo);
-        return mPipBoundsHandler.getDestinationBounds(null /* bounds */);
+        return mPipBoundsHandler.getEntryDestinationBounds();
     }
 
     /**
@@ -526,7 +526,7 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
             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 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
             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 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
     @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 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
             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 28bfa46308ef..440e23ccedb7 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 @@ public class PipBoundsHandlerTest extends ShellTestCase {
     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 @@ public class PipBoundsHandlerTest extends ShellTestCase {
     }
 
     @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 @@ public class PipBoundsHandlerTest extends ShellTestCase {
         };
         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 @@ public class PipBoundsHandlerTest extends ShellTestCase {
     }
 
     @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 @@ public class PipBoundsHandlerTest extends ShellTestCase {
     }
 
     @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 @@ public class PipBoundsHandlerTest extends ShellTestCase {
     }
 
     @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 @@ public class PipBoundsHandlerTest extends ShellTestCase {
             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 @@ public class PipBoundsHandlerTest extends ShellTestCase {
     }
 
     @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 @@ public class PipBoundsHandlerTest extends ShellTestCase {
 
         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 @@ public class PipBoundsHandlerTest extends ShellTestCase {
     }
 
     @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 class PipBoundsHandlerTest extends ShellTestCase {
     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 class PipBoundsHandlerTest extends ShellTestCase {
     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 @@ public class PipBoundsHandlerTest extends ShellTestCase {
                 + " 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 ea6092a8f774..08841bd2b1b2 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 @@ package com.android.wm.shell.pip;
 
 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 @@ public class PipTaskOrganizerTest extends ShellTestCase {
     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);