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

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

Block PiP state advancement if not in PiP 

PipDisplayChangeObserver helps PiP component
keep track of system-wide display changes, and
can directly request to advance to bounds-changing
PipTransitionState states.

These should be blocked by PipTransitionState if
we are not in PiP.

We are also tightening the filter on the observer's side;
so if say system crash occurs while we are in PiP and
a pinned window remains on top, we don't want the presence
of the pinned window to be the only filter to advance to new states.

This same logic could apply to display changes when enter PiP
is scheduled but not yet complete: e.g. during fixed rotation.

Bug: 432672577
Flag: EXEMPT bugfix
Test: atest PipTransitionState
Test: atest PipDisplayChangeObserverTest
Change-Id: I4bf92a38ac2a92e241931527d9d4ef77072c4413
parent 758f4dc9

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

+4 −0
Original line number Diff line number Diff line
@@ -481,6 +481,10 @@ public class PipTransitionState { 
                // - there is no drag-to-desktop gesture in progress; otherwise the PiP resize

                //   transition will block the drag-to-desktop transitions from finishing

                return isPipStateIdle() && !mPipDesktopState.isDragToDesktopInProgress();

            case CHANGING_PIP_BOUNDS:

                return mState == SCHEDULED_BOUNDS_CHANGE;

            case CHANGED_PIP_BOUNDS:

                return mState == CHANGING_PIP_BOUNDS;

            default:

                return true;

        }

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

+2 −2
Original line number Diff line number Diff line
@@ -88,7 +88,7 @@ public class PipDisplayChangeObserver implements Transitions.TransitionObserver 


    private void onDisplayChangeStarting(@NonNull TransitionInfo info) {

        final TransitionInfo.Change pipChange = PipTransitionUtils.getPipChange(info);

        if (pipChange == null) return;

        if (pipChange == null || !mPipTransitionState.isPipStateIdle()) return;



        // We do not care about the extras in this case - just make sure to send a non-empty one;

        // since otherwise PipTransitionState might throw an exception.
@@ -103,7 +103,7 @@ public class PipDisplayChangeObserver implements Transitions.TransitionObserver 


    private void onDisplayChangeFinished(@NonNull TransitionInfo info) {

        final TransitionInfo.Change pipChange = PipTransitionUtils.getPipChange(info);

        if (pipChange == null) return;

        if (pipChange == null || !mPipTransitionState.isInPip()) return;



        final Rect endBounds = pipChange.getEndAbsBounds();

        mPipBoundsState.setBounds(endBounds);

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

+26 −0
Original line number Diff line number Diff line
@@ -173,10 +173,31 @@ public class PipTransitionStateTest extends ShellTestCase { 
                PipTransitionState.SCHEDULED_BOUNDS_CHANGE));

    }



    @Test

    public void testShouldTransitionToState_changingPipBounds_afterScheduling_returnsTrue() {

        Bundle extra = new Bundle();

        extra.putParcelable(EXTRA_ENTRY_KEY, mEmptyParcelable);

        mPipTransitionState.setState(PipTransitionState.ENTERED_PIP);

        mPipTransitionState.setState(PipTransitionState.SCHEDULED_BOUNDS_CHANGE, extra);



        Assert.assertTrue(mPipTransitionState.shouldTransitionToState(

                PipTransitionState.CHANGING_PIP_BOUNDS));

    }



    @Test

    public void testShouldTransitionToState_changingPipBounds_withoutScheduling_returnsFalse() {

        mPipTransitionState.setState(PipTransitionState.ENTERED_PIP);



        Assert.assertFalse(mPipTransitionState.shouldTransitionToState(

                PipTransitionState.CHANGING_PIP_BOUNDS));

    }



    @Test

    public void shouldTransitionToState_scheduledBoundsChangeWhileChangingBounds_returnsFalse() {

        Bundle extra = new Bundle();

        extra.putParcelable(EXTRA_ENTRY_KEY, mEmptyParcelable);

        mPipTransitionState.setState(PipTransitionState.ENTERED_PIP);

        mPipTransitionState.setState(PipTransitionState.SCHEDULED_BOUNDS_CHANGE, extra);

        mPipTransitionState.setState(PipTransitionState.CHANGING_PIP_BOUNDS, extra);



        Assert.assertFalse(mPipTransitionState.shouldTransitionToState(
@@ -187,6 +208,7 @@ public class PipTransitionStateTest extends ShellTestCase { 
    public void testResetSameState_scheduledBoundsChange_doNotDispatchStateChanged() {

        Bundle extra = new Bundle();

        extra.putParcelable(EXTRA_ENTRY_KEY, mEmptyParcelable);

        mPipTransitionState.setState(PipTransitionState.ENTERED_PIP);

        mPipTransitionState.setState(PipTransitionState.SCHEDULED_BOUNDS_CHANGE, extra);



        mStateChangedListener = mock(PipTransitionState.PipTransitionStateChangedListener.class);
@@ -202,6 +224,8 @@ public class PipTransitionStateTest extends ShellTestCase { 
        // Choose an initial state.

        Bundle extra = new Bundle();

        extra.putParcelable(EXTRA_ENTRY_KEY, mEmptyParcelable);

        mPipTransitionState.setState(PipTransitionState.ENTERED_PIP);

        mPipTransitionState.setState(PipTransitionState.SCHEDULED_BOUNDS_CHANGE, extra);

        mPipTransitionState.setState(PipTransitionState.CHANGING_PIP_BOUNDS, extra);



        // Add the same PiP transition state changed listener twice.
@@ -250,6 +274,8 @@ public class PipTransitionStateTest extends ShellTestCase { 
        // pick a non-idle state

        Bundle extra = new Bundle();

        extra.putParcelable(EXTRA_ENTRY_KEY, mEmptyParcelable);

        mPipTransitionState.setState(PipTransitionState.ENTERED_PIP);

        mPipTransitionState.setState(PipTransitionState.SCHEDULED_BOUNDS_CHANGE, extra);

        mPipTransitionState.setState(PipTransitionState.CHANGING_PIP_BOUNDS, extra);



        final Runnable onIdleRunnable = () -> {};

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

+28 −0
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import static org.junit.Assert.assertTrue; 
import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.when;

import static org.mockito.kotlin.VerificationKt.clearInvocations;

import static org.mockito.kotlin.VerificationKt.never;

import static org.mockito.kotlin.VerificationKt.verify;
@@ -105,6 +106,8 @@ public class PipDisplayChangeObserverTest { 


    @Test

    public void onTransitionStarting_withPipChange_updatesPipState() {

        when(mMockPipTransitionState.isPipStateIdle()).thenReturn(true);



        final IBinder transition = new Binder();

        final TransitionInfo info = createPipBoundsChangingWithDisplayInfo();

        mPipDisplayChangeObserver.onTransitionReady(transition, info, mStartTx, mFinishTx);
@@ -120,8 +123,30 @@ public class PipDisplayChangeObserverTest { 
                mPipDisplayChangeObserver.getDisplayChangeTransitions().get(transition));

    }



    @Test

    public void onTransitionStarting_withNonIdlePipChange_updatesPipState() {

        when(mMockPipTransitionState.isPipStateIdle()).thenReturn(false);



        final IBinder transition = new Binder();

        final TransitionInfo info = createPipBoundsChangingWithDisplayInfo();

        mPipDisplayChangeObserver.onTransitionReady(transition, info, mStartTx, mFinishTx);



        mPipDisplayChangeObserver.onTransitionStarting(transition);



        verify(mMockPipTransitionState).setIsDisplayChangeScheduled(false);



        // Can't have state advancements when in a non-idle PiP state.

        verify(mMockPipTransitionState, never()).setState(anyInt(), any());



        assertNotNull("Transition should remain cached during animation",

                mPipDisplayChangeObserver.getDisplayChangeTransitions().get(transition));

    }



    @Test

    public void onTransitionFinished_withPipChange_updatesPipStateAndCleansUp() {

        when(mMockPipTransitionState.isPipStateIdle()).thenReturn(true);

        when(mMockPipTransitionState.isInPip()).thenReturn(true);



        final IBinder transition = new Binder();

        final TransitionInfo info = createPipBoundsChangingWithDisplayInfo();

        mPipDisplayChangeObserver.onTransitionReady(transition, info, mStartTx, mFinishTx);
@@ -138,6 +163,9 @@ public class PipDisplayChangeObserverTest { 


    @Test

    public void onTransitionMerged_withPipChange_updatesPipStateAndCleansUp() {

        when(mMockPipTransitionState.isPipStateIdle()).thenReturn(true);

        when(mMockPipTransitionState.isInPip()).thenReturn(true);



        final IBinder mergedTransition = new Binder();

        final IBinder playingTransition = new Binder();

        final TransitionInfo info = createPipBoundsChangingWithDisplayInfo();