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

Commit cc18218a authored by Ikram Gabiyev's avatar Ikram Gabiyev
Browse files

[PiP2] Only accept aspect ratio change if present 

If the client is setting new PictureInPictureParams,
we should check whether a valid aspect ratio is provided.

Cause otherwise, even if we block the actual resize transition,
internal state might get updated leading to wrong bounds scales.
These are later on used if a re-entry state is being saved, and
might reflect incorrect reentry bounds.

We also make sure to clean-up any re-entry state and component related
state in PipBoundsState if the PiP task or activity are CLOSED (e.g.
after a client app crash). This is needed, since some tests, for
instance, do not go through normal PiP removal
and just force stop the test app.

Note: the regression in the BR listed was happening due to app icon
overlay being used when we swipe PiP to home, so one way to verify
is to run the test suite on forrest and check if src-rect-hint is larger
than the destination bounds,

Bug: 380535601
Flag: com.android.wm.shell.enable_pip2
Test: run android.platform.test.scenario \
	.launcher.integration_tests \
	.swipeupapptopip_tests on go/forrest

Change-Id: I40a93f79b5a1db0b2a01b88cfafe0d07374a10c1
parent df18c63b

libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipTaskListener.java

+3 −1
Original line number Diff line number Diff line
@@ -133,7 +133,9 @@ public class PipTaskListener implements ShellTaskOrganizer.TaskListener, 
                taskInfo.topActivity, mPipTransitionState, mPictureInPictureParams, params);

        setPictureInPictureParams(params);

        float newAspectRatio = mPictureInPictureParams.getAspectRatioFloat();

        if (PipUtils.aspectRatioChanged(newAspectRatio, mPipBoundsState.getAspectRatio())) {

        if (params.hasSetAspectRatio()

                && mPipBoundsAlgorithm.isValidPictureInPictureAspectRatio(newAspectRatio)

                && PipUtils.aspectRatioChanged(newAspectRatio, mPipBoundsState.getAspectRatio())) {

            mPipTransitionState.setOnIdlePipTransitionStateRunnable(() -> {

                onAspectRatioChanged(newAspectRatio);

            });

libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipTransition.java

+25 −3
Original line number Diff line number Diff line
@@ -725,6 +725,12 @@ public class PipTransition extends PipTransitionController implements 
                mPipTransitionState.getPipTaskToken());

        mFinishCallback = finishCallback;



        if (isPipClosing(info)) {

            // If PiP is removed via a close (e.g. finishing of the activity), then

            // clear out the PiP cache related to that activity component (e.g. reentry state).

            mPipBoundsState.setLastPipComponentName(null /* lastPipComponentName */);

        }



        finishTransaction.setAlpha(pipChange.getLeash(), 0f);

        if (mPendingRemoveWithFadeout) {

            PipAlphaAnimator animator = new PipAlphaAnimator(mContext, pipChange.getLeash(),
@@ -952,13 +958,29 @@ public class PipTransition extends PipTransitionController implements 


        boolean isPipMovedToBack = info.getType() == TRANSIT_TO_BACK

                && pipChange.getMode() == TRANSIT_TO_BACK;

        boolean isPipClosed = info.getType() == TRANSIT_CLOSE

                && pipChange.getMode() == TRANSIT_CLOSE;

        // If PiP is dismissed by user (i.e. via dismiss button in PiP menu)

        boolean isPipDismissed = info.getType() == TRANSIT_REMOVE_PIP

                && pipChange.getMode() == TRANSIT_TO_BACK;

        // PiP is being removed if the pinned task is either moved to back, closed, or dismissed.

        return isPipMovedToBack || isPipClosed || isPipDismissed;

        return isPipMovedToBack || isPipClosing(info) || isPipDismissed;

    }



    private boolean isPipClosing(@NonNull TransitionInfo info) {

        if (mPipTransitionState.getPipTaskToken() == null) {

            // PiP removal makes sense if enter-PiP has cached a valid pinned task token.

            return false;

        }

        TransitionInfo.Change pipChange = info.getChange(mPipTransitionState.getPipTaskToken());

        TransitionInfo.Change pipActivityChange = info.getChanges().stream().filter(change ->

                change.getTaskInfo() == null && change.getParent() != null

                        && change.getParent() == mPipTransitionState.getPipTaskToken())

                .findFirst().orElse(null);



        boolean isPipTaskClosed = pipChange != null

                && pipChange.getMode() == TRANSIT_CLOSE;

        boolean isPipActivityClosed = pipActivityChange != null

                && pipActivityChange.getMode() == TRANSIT_CLOSE;

        return isPipTaskClosed || isPipActivityClosed;

    }



    private void prepareConfigAtEndActivity(@NonNull SurfaceControl.Transaction startTx,

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip2/phone/PipTaskListenerTest.java

+30 −0
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ import static org.junit.Assert.assertEquals; 
import static org.junit.Assert.assertTrue;

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.Mockito.when;

import static org.mockito.kotlin.MatchersKt.eq;
@@ -193,6 +194,12 @@ public class PipTaskListenerTest { 
                mMockPipTransitionState, mMockPipScheduler, mMockPipBoundsState,

                mMockPipBoundsAlgorithm, mMockShellExecutor);

        mPipTaskListener.addParamsChangedListener(mMockPipParamsChangedCallback);



        // For this test case, any aspect ratio passed is considered within allowed range.

        when(mMockPipBoundsAlgorithm

                .isValidPictureInPictureAspectRatio(anyFloat()))

                .thenReturn(true);



        Rational aspectRatio = new Rational(4, 3);

        when(mMockPipBoundsState.getAspectRatio()).thenReturn(aspectRatio.toFloat());

        String action1 = "action1";
@@ -226,6 +233,29 @@ public class PipTaskListenerTest { 
                .setOnIdlePipTransitionStateRunnable(any(Runnable.class));

    }



    @Test

    public void onTaskInfoChanged_nonValidAspectRatio_doesNotCallbackAspectRatioChanged() {

        mPipTaskListener = new PipTaskListener(mMockContext, mMockShellTaskOrganizer,

                mMockPipTransitionState, mMockPipScheduler, mMockPipBoundsState,

                mMockPipBoundsAlgorithm, mMockShellExecutor);

        mPipTaskListener.addParamsChangedListener(mMockPipParamsChangedCallback);



        String action1 = "action1";

        mPipTaskListener.onTaskInfoChanged(getTaskInfo(null, action1));

        verify(mMockPipTransitionState, times(0))

                .setOnIdlePipTransitionStateRunnable(any(Runnable.class));



        // Define an invalid aspect ratio and try and update the params with it.

        Rational aspectRatio = new Rational(100, 3);

        when(mMockPipBoundsAlgorithm

                .isValidPictureInPictureAspectRatio(eq(aspectRatio.floatValue())))

                .thenReturn(false);



        mPipTaskListener.onTaskInfoChanged(getTaskInfo(aspectRatio, action1));

        verify(mMockPipTransitionState, times(0))

                .setOnIdlePipTransitionStateRunnable(any(Runnable.class));

    }



    @Test

    public void onPipTransitionStateChanged_scheduledBoundsChangeWithAspectRatioChange_schedule() {

        mPipTaskListener = new PipTaskListener(mMockContext, mMockShellTaskOrganizer,