Merge "Refactor PipDisplayChangeObserver to avoid flicker" into main (694d5269) · Commits · e / os / android_frameworks_base

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

+1 −1

Original line number	Diff line number	Diff line
		@@ -490,7 +490,7 @@ public class PipController implements ConfigurationChangeListener,
		mPipTransitionState.setState(PipTransitionState.SCHEDULED_BOUNDS_CHANGE, extra);
		});
		} else {
		mPipTransitionState.setIsPipBoundsChangingWithDisplay(true);
		mPipTransitionState.setIsDisplayChangeScheduled(true);
		t.setBounds(mPipTransitionState.getPipTaskToken(), mPipBoundsState.getBounds());
		}
		// Update the size spec in PipBoundsState afterwards.

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

+31 −16

Original line number	Diff line number	Diff line
		@@ -174,7 +174,13 @@ public class PipTransitionState {

		private boolean mInFixedRotation = false;

		private boolean mIsPipBoundsChangingWithDisplay = false;
		/*
		* A flag to keep track of the period between a display change being requested and the point
		* when display change transition starts playing.
		* We keep track of this separately from bounds change states, since display change transition
		* request can come in at any point while a PiP resize is still scheduled or running.
		*/
		private boolean mIsDisplayChangeScheduled = false;

		/**
		* An interface to track state updates as we progress through PiP transitions.
		@@ -400,22 +406,31 @@ public class PipTransitionState {
		}

		/**
		* @return true if a display change is ungoing with a PiP bounds change.
		* @return true if PiP state affecting display change has been requested but hasn't played yet.
		*/
		public boolean isPipBoundsChangingWithDisplay() {
		return mIsPipBoundsChangingWithDisplay;
		public boolean isDisplayChangeScheduled() {
		return mIsDisplayChangeScheduled;
		}

		/**
		* Sets the PiP bounds change with display change flag.
		* Sets the display change scheduled flag.
		*
		* The caller is expected to:
		* <ul>
		* <li>reset this flag once display change transition starts playing,</li>
		* <li>synchronously set PiP transition state to SCHEDULED_BOUNDS_CHANGE,
		* putting PiP back into a non-idle state.</li>
		* <li>progress as usual to CHANGING_PIP_BOUNDS followed by CHANGED_PIP_BOUNDS states
		* to put PiP back into an idle state</li>
		* </ul>
		*
		* <p>Note: this won't run the on-idle runnable as we are expected to be put into a non-idle
		* state immediately after.</p>
		*/
		public void setIsPipBoundsChangingWithDisplay(boolean isPipBoundsChangingWithDisplay) {
		public void setIsDisplayChangeScheduled(boolean isDisplayChangeScheduled) {
		ProtoLog.v(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE,
		"%s: Set mIsPipBoundsChangingWithDisplay=%b", TAG, isPipBoundsChangingWithDisplay);
		mIsPipBoundsChangingWithDisplay = isPipBoundsChangingWithDisplay;
		if (!isPipBoundsChangingWithDisplay) {
		maybeRunOnIdlePipTransitionStateCallback();
		}
		"%s: Set mIsDisplayChangeScheduled=%b", TAG, isDisplayChangeScheduled);
		mIsDisplayChangeScheduled = isDisplayChangeScheduled;
		}

		/**
		@@ -490,14 +505,14 @@ public class PipTransitionState {
		public boolean isPipStateIdle() {
		// This needs to be a valid in-PiP state that isn't a transient state.
		return (mState == ENTERED_PIP \|\| mState == CHANGED_PIP_BOUNDS)
		&& !isInFixedRotation() && !isPipBoundsChangingWithDisplay();
		&& !isInFixedRotation() && !isDisplayChangeScheduled();
		}

		@Override
		public String toString() {
		return String.format("PipTransitionState(mState=%s, mInSwipePipToHomeTransition=%b, "
		+ "mIsPipBoundsChangingWithDisplay=%b, mInFixedRotation=%b",
		stateToString(mState), mInSwipePipToHomeTransition, mIsPipBoundsChangingWithDisplay,
		+ "mIsDisplayChangeScheduled=%b, mInFixedRotation=%b",
		stateToString(mState), mInSwipePipToHomeTransition, mIsDisplayChangeScheduled,
		mInFixedRotation);
		}

		@@ -507,7 +522,7 @@ public class PipTransitionState {
		pw.println(prefix + TAG);
		pw.println(innerPrefix + "mState=" + stateToString(mState));
		pw.println(innerPrefix + "mInFixedRotation=" + mInFixedRotation);
		pw.println(innerPrefix + "mIsPipBoundsChangingWithDisplay="
		+ mIsPipBoundsChangingWithDisplay);
		pw.println(innerPrefix + "mIsDisplayChangeScheduled="
		+ mIsDisplayChangeScheduled);
		}
		}

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

+42 −22

Original line number	Diff line number	Diff line
		@@ -17,13 +17,13 @@
		package com.android.wm.shell.pip2.phone.transition;

		import android.graphics.Rect;
		import android.os.Bundle;
		import android.os.IBinder;
		import android.util.Pair;
		import android.util.ArrayMap;
		import android.view.SurfaceControl;
		import android.window.TransitionInfo;

		import androidx.annotation.NonNull;
		import androidx.annotation.Nullable;
		import androidx.annotation.VisibleForTesting;

		import com.android.wm.shell.common.pip.PipBoundsState;
		@@ -32,15 +32,13 @@ import com.android.wm.shell.shared.TransitionUtil;
		import com.android.wm.shell.transition.Transitions;

		/**
		* An implementation of {@link Transitions.TransitionObserver} to track of external transitions
		* that might affect a PiP task as well.
		* An implementation of {@link Transitions.TransitionObserver} to track external display change
		* transitions that might affect a PiP task as well.
		*/
		public class PipDisplayChangeObserver implements Transitions.TransitionObserver {
		private final PipTransitionState mPipTransitionState;
		private final PipBoundsState mPipBoundsState;

		@Nullable
		private Pair<IBinder, TransitionInfo> mDisplayChangeTransition;
		private final ArrayMap<IBinder, TransitionInfo> mDisplayChangeTransitions = new ArrayMap<>();

		public PipDisplayChangeObserver(PipTransitionState pipTransitionState,
		PipBoundsState pipBoundsState) {
		@@ -53,41 +51,63 @@ public class PipDisplayChangeObserver implements Transitions.TransitionObserver
		@NonNull SurfaceControl.Transaction startTransaction,
		@NonNull SurfaceControl.Transaction finishTransaction) {
		if (TransitionUtil.hasDisplayChange(info)) {
		mDisplayChangeTransition = new Pair<>(transition, info);
		mDisplayChangeTransitions.put(transition, info);
		}
		}

		@Override
		public void onTransitionStarting(@NonNull IBinder transition) {
		if (mDisplayChangeTransitions.containsKey(transition)) {
		// The display change transition is being played now, so it's safe to reset
		// the display change scheduled flag.
		mPipTransitionState.setIsDisplayChangeScheduled(false);
		onDisplayChangeStarting(mDisplayChangeTransitions.get(transition));
		}
		}

		@Override
		public void onTransitionMerged(@NonNull IBinder merged, @NonNull IBinder playing) {
		if (mDisplayChangeTransition != null
		&& mDisplayChangeTransition.first == merged) {
		maybeUpdatePipStateOnDisplayChange(mDisplayChangeTransition.second /* info */);
		mPipTransitionState.setIsPipBoundsChangingWithDisplay(false);
		mDisplayChangeTransition = null;
		if (mDisplayChangeTransitions.containsKey(merged)) {
		onDisplayChangeFinished(mDisplayChangeTransitions.get(merged));
		mDisplayChangeTransitions.remove(merged);
		}
		}

		@Override
		public void onTransitionFinished(@NonNull IBinder transition, boolean aborted) {
		if (mDisplayChangeTransition != null
		&& mDisplayChangeTransition.first == transition) {
		maybeUpdatePipStateOnDisplayChange(mDisplayChangeTransition.second /* info */);
		mPipTransitionState.setIsPipBoundsChangingWithDisplay(false);
		mDisplayChangeTransition = null;
		if (mDisplayChangeTransitions.containsKey(transition)) {
		onDisplayChangeFinished(mDisplayChangeTransitions.get(transition));
		mDisplayChangeTransitions.remove(transition);
		}
		}

		@VisibleForTesting
		@Nullable
		Pair<IBinder, TransitionInfo> getDisplayChangeTransition() {
		return mDisplayChangeTransition;
		ArrayMap<IBinder, TransitionInfo> getDisplayChangeTransitions() {
		return mDisplayChangeTransitions;
		}

		private void onDisplayChangeStarting(@NonNull TransitionInfo info) {
		final TransitionInfo.Change pipChange = PipTransitionUtils.getPipChange(info);
		if (pipChange == null) 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.
		final Bundle extra = new Bundle();
		extra.putInt("", 0);
		// It is safe to advance to PiP bounds changing states, since this display change
		// transition can never start playing while another PiP transition is already playing.
		// This should help us block any interactions with PiP during this period.
		mPipTransitionState.setState(PipTransitionState.SCHEDULED_BOUNDS_CHANGE, extra);
		mPipTransitionState.setState(PipTransitionState.CHANGING_PIP_BOUNDS, extra);
		}

		private void maybeUpdatePipStateOnDisplayChange(@NonNull TransitionInfo info) {
		private void onDisplayChangeFinished(@NonNull TransitionInfo info) {
		final TransitionInfo.Change pipChange = PipTransitionUtils.getPipChange(info);
		if (pipChange == null) return;

		final Rect endBounds = pipChange.getEndAbsBounds();
		mPipBoundsState.setBounds(endBounds);
		// Indicate that this display changing transition that was altering PiP bounds is over.
		mPipTransitionState.setState(PipTransitionState.CHANGED_PIP_BOUNDS);
		}
		}

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

+8 −5

Original line number	Diff line number	Diff line
		@@ -268,14 +268,16 @@ public class PipTransitionStateTest extends ShellTestCase {
		}

		@Test
		public void testSetIsPipBoundsChangingWithDisplay_toFalse_thenIdle() {
		public void testSetIsDisplayChangeScheduled_toFalse_thenIdleWithoutRunnableSent() {
		when(mMainHandler.obtainMessage(anyInt())).thenAnswer(invocation ->
		new Message().setWhat(invocation.getArgument(0)));

		// Pick an initially idle ENTERED_PIP state
		mPipTransitionState.setState(PipTransitionState.ENTERED_PIP);

		// Enter an non-idle state as PiP bounds change with the display
		mPipTransitionState.setIsPipBoundsChangingWithDisplay(true);
		mPipTransitionState.setIsDisplayChangeScheduled(true);
		Assert.assertFalse("PiP should not be idle", mPipTransitionState.isPipStateIdle());

		final Runnable onIdleRunnable = mock(Runnable.class);
		mPipTransitionState.setOnIdlePipTransitionStateRunnable(onIdleRunnable);
		@@ -283,9 +285,10 @@ public class PipTransitionStateTest extends ShellTestCase {
		// We are supposed to be in a non-idle state, so the runnable should not be posted yet.
		verify(mMainHandler, never()).sendMessage(any());

		mPipTransitionState.setIsPipBoundsChangingWithDisplay(false);
		verify(mMainHandler, times(1))
		.sendMessage(argThat(msg -> msg.getCallback() == onIdleRunnable));
		mPipTransitionState.setIsDisplayChangeScheduled(false);
		// We aren't supposed to post the on-idle runnable upon reset of display change scheduled
		// flag, as we are expected to be put back into a non-idle state while display change plays.
		verify(mMainHandler, never()).sendMessage(any());
		}

		@Test

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

+75 −48

Original line number	Diff line number	Diff line
		@@ -20,11 +20,13 @@ import static android.app.WindowConfiguration.WINDOWING_MODE_PINNED;
		import static android.view.WindowManager.TRANSIT_CHANGE;
		import static android.window.TransitionInfo.FLAG_IS_DISPLAY;

		import static org.junit.Assert.assertNotNull;
		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.kotlin.VerificationKt.clearInvocations;
		import static org.mockito.kotlin.VerificationKt.never;
		import static org.mockito.kotlin.VerificationKt.times;
		import static org.mockito.kotlin.VerificationKt.verify;

		import android.app.ActivityManager;
		@@ -42,8 +44,6 @@ import androidx.test.filters.SmallTest;
		import com.android.wm.shell.common.pip.PipBoundsState;
		import com.android.wm.shell.pip2.phone.PipTransitionState;

		import junit.framework.Assert;

		import org.junit.Before;
		import org.junit.Test;
		import org.junit.runner.RunWith;
		@@ -79,76 +79,97 @@ public class PipDisplayChangeObserverTest {
		}

		@Test
		public void onTransitionReady_withPipAndDisplayChange_cachesDisplayChangeTransition() {
		Assert.assertNull(mPipDisplayChangeObserver.getDisplayChangeTransition());
		public void onTransitionReady_withDisplayChange_cachesTransition() {
		final IBinder transition = new Binder();
		final TransitionInfo info = createPipBoundsChangingWithDisplayInfo();

		final IBinder transitionToken = new Binder();
		mPipDisplayChangeObserver.onTransitionReady(transitionToken,
		createPipBoundsChangingWithDisplayInfo(), mStartTx, mFinishTx);
		assertTrue("Map should be empty before test",
		mPipDisplayChangeObserver.getDisplayChangeTransitions().isEmpty());

		Assert.assertNotNull(mPipDisplayChangeObserver.getDisplayChangeTransition());
		Assert.assertSame(transitionToken,
		mPipDisplayChangeObserver.getDisplayChangeTransition().first);
		mPipDisplayChangeObserver.onTransitionReady(transition, info, mStartTx, mFinishTx);

		assertNotNull("Display change transition should be cached",
		mPipDisplayChangeObserver.getDisplayChangeTransitions().get(transition));
		}

		@Test
		public void onTransitionFinished_withPipAndDisplayChange_updatePipState() {
		final IBinder transitionToken = new Binder();
		final TransitionInfo info = createPipBoundsChangingWithDisplayInfo();
		public void onTransitionReady_withoutDisplayChange_doesNotCacheTransition() {
		final IBinder transition = new Binder();
		final TransitionInfo info = new TransitionInfo(TRANSIT_CHANGE, 0); // No FLAG_IS_DISPLAY

		mPipDisplayChangeObserver.onTransitionReady(transitionToken, info, mStartTx, mFinishTx);
		clearInvocations(mMockPipTransitionState);
		mPipDisplayChangeObserver.onTransitionFinished(transitionToken, false /* aborted */);
		mPipDisplayChangeObserver.onTransitionReady(transition, info, mStartTx, mFinishTx);

		verify(mMockPipTransitionState, times(1)).setIsPipBoundsChangingWithDisplay(eq(false));
		verify(mMockPipBoundsState, times(1)).setBounds(eq(mPipEndBounds));
		Assert.assertNull(mPipDisplayChangeObserver.getDisplayChangeTransition());
		assertTrue("Map should remain empty for non-display-change transitions",
		mPipDisplayChangeObserver.getDisplayChangeTransitions().isEmpty());
		}

		@Test
		public void onTransitionMerged_withPipAndDisplayChange_updatePipState() {
		final IBinder transitionToken = new Binder();
		final IBinder playingTransitionToken = new Binder();
		public void onTransitionStarting_withPipChange_updatesPipState() {
		final IBinder transition = new Binder();
		final TransitionInfo info = createPipBoundsChangingWithDisplayInfo();
		mPipDisplayChangeObserver.onTransitionReady(transition, info, mStartTx, mFinishTx);

		mPipDisplayChangeObserver.onTransitionReady(transitionToken, info, mStartTx, mFinishTx);
		clearInvocations(mMockPipTransitionState);
		mPipDisplayChangeObserver.onTransitionMerged(transitionToken, playingTransitionToken);
		mPipDisplayChangeObserver.onTransitionStarting(transition);

		verify(mMockPipTransitionState).setIsDisplayChangeScheduled(false);
		verify(mMockPipTransitionState).setState(
		eq(PipTransitionState.SCHEDULED_BOUNDS_CHANGE), any());
		verify(mMockPipTransitionState).setState(eq(PipTransitionState.CHANGING_PIP_BOUNDS), any());

		verify(mMockPipTransitionState, times(1)).setIsPipBoundsChangingWithDisplay(eq(false));
		verify(mMockPipBoundsState, times(1)).setBounds(eq(mPipEndBounds));
		Assert.assertNull(mPipDisplayChangeObserver.getDisplayChangeTransition());
		assertNotNull("Transition should remain cached during animation",
		mPipDisplayChangeObserver.getDisplayChangeTransitions().get(transition));
		}

		@Test
		public void onTransitionFinished_withDisplayChangeOnly_noPipStateUpdate() {
		final IBinder transitionToken = new Binder();
		final TransitionInfo info = new TransitionInfo(TRANSIT_CHANGE, 0 /* flags */);
		info.addChange(createDisplayChange());
		public void onTransitionFinished_withPipChange_updatesPipStateAndCleansUp() {
		final IBinder transition = new Binder();
		final TransitionInfo info = createPipBoundsChangingWithDisplayInfo();
		mPipDisplayChangeObserver.onTransitionReady(transition, info, mStartTx, mFinishTx);
		mPipDisplayChangeObserver.onTransitionStarting(transition);
		clearInvocations(mMockPipTransitionState);

		mPipDisplayChangeObserver.onTransitionFinished(transition, false /* aborted */);

		verify(mMockPipBoundsState).setBounds(eq(mPipEndBounds));
		verify(mMockPipTransitionState).setState(PipTransitionState.CHANGED_PIP_BOUNDS);
		assertTrue("Transition should be removed from cache after finishing",
		mPipDisplayChangeObserver.getDisplayChangeTransitions().isEmpty());
		}

		mPipDisplayChangeObserver.onTransitionReady(transitionToken, info, mStartTx, mFinishTx);
		@Test
		public void onTransitionMerged_withPipChange_updatesPipStateAndCleansUp() {
		final IBinder mergedTransition = new Binder();
		final IBinder playingTransition = new Binder();
		final TransitionInfo info = createPipBoundsChangingWithDisplayInfo();
		mPipDisplayChangeObserver.onTransitionReady(mergedTransition, info, mStartTx, mFinishTx);
		mPipDisplayChangeObserver.onTransitionStarting(mergedTransition);
		clearInvocations(mMockPipTransitionState);
		mPipDisplayChangeObserver.onTransitionFinished(transitionToken, false /* aborted */);

		verify(mMockPipTransitionState, times(1)).setIsPipBoundsChangingWithDisplay(eq(false));
		verify(mMockPipBoundsState, never()).setBounds(any());
		Assert.assertNull(mPipDisplayChangeObserver.getDisplayChangeTransition());
		mPipDisplayChangeObserver.onTransitionMerged(mergedTransition, playingTransition);

		verify(mMockPipBoundsState).setBounds(eq(mPipEndBounds));
		verify(mMockPipTransitionState).setState(PipTransitionState.CHANGED_PIP_BOUNDS);
		assertTrue("Transition should be removed from cache after merging",
		mPipDisplayChangeObserver.getDisplayChangeTransitions().isEmpty());
		}

		@Test
		public void onTransitionMerged_withDisplayChangeOnly_noPipStateUpdate() {
		final IBinder transitionToken = new Binder();
		final IBinder playingTransitionToken = new Binder();
		final TransitionInfo info = new TransitionInfo(TRANSIT_CHANGE, 0 /* flags */);
		info.addChange(createDisplayChange());
		public void onTransitionFinished_displayChangeOnly_noPipStateUpdateAndCleansUp() {
		final IBinder transition = new Binder();
		final TransitionInfo info = createDisplayChangeOnlyInfo();
		mPipDisplayChangeObserver.onTransitionReady(transition, info, mStartTx, mFinishTx);

		mPipDisplayChangeObserver.onTransitionStarting(transition);
		verify(mMockPipTransitionState, never()).setState(anyInt());

		mPipDisplayChangeObserver.onTransitionReady(transitionToken, info, mStartTx, mFinishTx);
		clearInvocations(mMockPipTransitionState);
		mPipDisplayChangeObserver.onTransitionMerged(transitionToken, playingTransitionToken);

		verify(mMockPipTransitionState, times(1)).setIsPipBoundsChangingWithDisplay(eq(false));
		verify(mMockPipBoundsState, never()).setBounds(eq(mPipEndBounds));
		Assert.assertNull(mPipDisplayChangeObserver.getDisplayChangeTransition());
		mPipDisplayChangeObserver.onTransitionFinished(transition, false /* aborted */);

		verify(mMockPipBoundsState, never()).setBounds(any());
		verify(mMockPipTransitionState, never()).setState(anyInt());
		assertTrue("Transition should be removed from cache after finishing",
		mPipDisplayChangeObserver.getDisplayChangeTransitions().isEmpty());
		}

		private TransitionInfo createPipBoundsChangingWithDisplayInfo() {
		@@ -159,6 +180,12 @@ public class PipDisplayChangeObserverTest {
		return info;
		}

		private TransitionInfo createDisplayChangeOnlyInfo() {
		final TransitionInfo info = new TransitionInfo(TRANSIT_CHANGE, 0 /* flags */);
		info.addChange(createDisplayChange());
		return info;
		}

		private TransitionInfo.Change createPipChange() {
		final TransitionInfo.Change pipChange = new TransitionInfo.Change(mPipToken, mPipLeash);
		final ActivityManager.RunningTaskInfo pipTaskInfo = new ActivityManager.RunningTaskInfo();