Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a61e9fec authored by jorgegil@google.com's avatar jorgegil@google.com
Browse files

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
parent 32026d51

libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipBoundsHandler.java

+18 −31
Original line number Diff line number Diff line
@@ -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;

    }

libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTaskOrganizer.java

+7 −8
Original line number Diff line number Diff line
@@ -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);

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/PipBoundsHandlerTest.java

+25 −49
Original line number Diff line number Diff line
@@ -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));

    }

}

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/PipTaskOrganizerTest.java

+3 −3
Original line number Diff line number Diff line
@@ -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);