Ensure that PiP mode changed callback if animation is interrupted
- If a PiP enter animation callback is interrupted, the activity is never
actually put into PiP mode, and will never receive
onPictureInPictureModeChanged(false) even if enterPictureInPictureMode()
returns true due to the change being deduped (it was never in that mode).
In this specific case, force a callback to be made to the app so that it
has a signal that it is no longer in PiP mode.
Bug: 63749396
Test: bit FrameworksServicesTests:com.android.server.wm.BoundsAnimationControllerTests
Test: android.server.cts.ActivityManagerPinnedStackTests
Test: #testEnterPipInterruptedCallbacks
Change-Id: I301c70e4fb0f2175dd6d7b5feae065b41df2878d
diff --git a/services/core/java/com/android/server/am/ActivityRecord.java b/services/core/java/com/android/server/am/ActivityRecord.java
index 43e2650..4fd2719 100644
--- a/services/core/java/com/android/server/am/ActivityRecord.java
+++ b/services/core/java/com/android/server/am/ActivityRecord.java
@@ -659,14 +659,14 @@
}
}
- void updatePictureInPictureMode(Rect targetStackBounds) {
+ void updatePictureInPictureMode(Rect targetStackBounds, boolean forceUpdate) {
if (task == null || task.getStack() == null || app == null || app.thread == null) {
return;
}
final boolean inPictureInPictureMode = (task.getStackId() == PINNED_STACK_ID) &&
(targetStackBounds != null);
- if (inPictureInPictureMode != mLastReportedPictureInPictureMode) {
+ if (inPictureInPictureMode != mLastReportedPictureInPictureMode || forceUpdate) {
// Picture-in-picture mode changes also trigger a multi-window mode change as well, so
// update that here in order
mLastReportedPictureInPictureMode = inPictureInPictureMode;
@@ -681,8 +681,7 @@
private void schedulePictureInPictureModeChanged(Configuration overrideConfig) {
try {
app.thread.schedulePictureInPictureModeChanged(appToken,
- mLastReportedPictureInPictureMode,
- overrideConfig);
+ mLastReportedPictureInPictureMode, overrideConfig);
} catch (Exception e) {
// If process died, no one cares.
}
diff --git a/services/core/java/com/android/server/am/ActivityStackSupervisor.java b/services/core/java/com/android/server/am/ActivityStackSupervisor.java
index 54bb526..a5d21fa 100644
--- a/services/core/java/com/android/server/am/ActivityStackSupervisor.java
+++ b/services/core/java/com/android/server/am/ActivityStackSupervisor.java
@@ -4411,31 +4411,29 @@
return;
}
- scheduleUpdatePictureInPictureModeIfNeeded(task, stack.mBounds, false /* immediate */);
+ scheduleUpdatePictureInPictureModeIfNeeded(task, stack.mBounds);
}
- void scheduleUpdatePictureInPictureModeIfNeeded(TaskRecord task, Rect targetStackBounds,
- boolean immediate) {
-
- if (immediate) {
- mHandler.removeMessages(REPORT_PIP_MODE_CHANGED_MSG);
- for (int i = task.mActivities.size() - 1; i >= 0; i--) {
- final ActivityRecord r = task.mActivities.get(i);
- if (r.app != null && r.app.thread != null) {
- r.updatePictureInPictureMode(targetStackBounds);
- }
+ void scheduleUpdatePictureInPictureModeIfNeeded(TaskRecord task, Rect targetStackBounds) {
+ for (int i = task.mActivities.size() - 1; i >= 0; i--) {
+ final ActivityRecord r = task.mActivities.get(i);
+ if (r.app != null && r.app.thread != null) {
+ mPipModeChangedActivities.add(r);
}
- } else {
- for (int i = task.mActivities.size() - 1; i >= 0; i--) {
- final ActivityRecord r = task.mActivities.get(i);
- if (r.app != null && r.app.thread != null) {
- mPipModeChangedActivities.add(r);
- }
- }
- mPipModeChangedTargetStackBounds = targetStackBounds;
+ }
+ mPipModeChangedTargetStackBounds = targetStackBounds;
- if (!mHandler.hasMessages(REPORT_PIP_MODE_CHANGED_MSG)) {
- mHandler.sendEmptyMessage(REPORT_PIP_MODE_CHANGED_MSG);
+ if (!mHandler.hasMessages(REPORT_PIP_MODE_CHANGED_MSG)) {
+ mHandler.sendEmptyMessage(REPORT_PIP_MODE_CHANGED_MSG);
+ }
+ }
+
+ void updatePictureInPictureMode(TaskRecord task, Rect targetStackBounds, boolean forceUpdate) {
+ mHandler.removeMessages(REPORT_PIP_MODE_CHANGED_MSG);
+ for (int i = task.mActivities.size() - 1; i >= 0; i--) {
+ final ActivityRecord r = task.mActivities.get(i);
+ if (r.app != null && r.app.thread != null) {
+ r.updatePictureInPictureMode(targetStackBounds, forceUpdate);
}
}
}
@@ -4497,7 +4495,8 @@
synchronized (mService) {
for (int i = mPipModeChangedActivities.size() - 1; i >= 0; i--) {
final ActivityRecord r = mPipModeChangedActivities.remove(i);
- r.updatePictureInPictureMode(mPipModeChangedTargetStackBounds);
+ r.updatePictureInPictureMode(mPipModeChangedTargetStackBounds,
+ false /* forceUpdate */);
}
}
} break;
diff --git a/services/core/java/com/android/server/am/PinnedActivityStack.java b/services/core/java/com/android/server/am/PinnedActivityStack.java
index 392fbb2..a601ee1 100644
--- a/services/core/java/com/android/server/am/PinnedActivityStack.java
+++ b/services/core/java/com/android/server/am/PinnedActivityStack.java
@@ -91,15 +91,16 @@
return mWindowContainerController.deferScheduleMultiWindowModeChanged();
}
- public void updatePictureInPictureModeForPinnedStackAnimation(Rect targetStackBounds) {
+ public void updatePictureInPictureModeForPinnedStackAnimation(Rect targetStackBounds,
+ boolean forceUpdate) {
// It is guaranteed that the activities requiring the update will be in the pinned stack at
// this point (either reparented before the animation into PiP, or before reparenting after
// the animation out of PiP)
synchronized(this) {
ArrayList<TaskRecord> tasks = getAllTasks();
for (int i = 0; i < tasks.size(); i++ ) {
- mStackSupervisor.scheduleUpdatePictureInPictureModeIfNeeded(tasks.get(i),
- targetStackBounds, true /* immediate */);
+ mStackSupervisor.updatePictureInPictureMode(tasks.get(i), targetStackBounds,
+ forceUpdate);
}
}
}
diff --git a/services/core/java/com/android/server/wm/BoundsAnimationController.java b/services/core/java/com/android/server/wm/BoundsAnimationController.java
index cff2fad..7953ee4 100644
--- a/services/core/java/com/android/server/wm/BoundsAnimationController.java
+++ b/services/core/java/com/android/server/wm/BoundsAnimationController.java
@@ -21,7 +21,6 @@
import static com.android.server.wm.WindowManagerDebugConfig.TAG_WM;
import android.animation.AnimationHandler;
-import android.animation.AnimationHandler.AnimationFrameCallbackProvider;
import android.animation.Animator;
import android.animation.ValueAnimator;
import android.annotation.IntDef;
@@ -32,13 +31,11 @@
import android.os.Debug;
import android.util.ArrayMap;
import android.util.Slog;
-import android.view.Choreographer;
import android.view.animation.AnimationUtils;
import android.view.animation.Interpolator;
import android.view.WindowManagerInternal;
import com.android.internal.annotations.VisibleForTesting;
-import com.android.internal.graphics.SfVsyncFrameCallbackProvider;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
@@ -142,9 +139,6 @@
// True if this this animation was canceled and will be replaced the another animation from
// the same {@link #BoundsAnimationTarget} target.
private boolean mSkipFinalResize;
- // True if this animation replaced a previous animation of the same
- // {@link #BoundsAnimationTarget} target.
- private final boolean mSkipAnimationStart;
// True if this animation was canceled by the user, not as a part of a replacing animation
private boolean mSkipAnimationEnd;
@@ -159,6 +153,7 @@
// Whether to schedule PiP mode changes on animation start/end
private @SchedulePipModeChangedState int mSchedulePipModeChangedState;
+ private @SchedulePipModeChangedState int mPrevSchedulePipModeChangedState;
// Depending on whether we are animating from
// a smaller to a larger size
@@ -171,14 +166,14 @@
BoundsAnimator(BoundsAnimationTarget target, Rect from, Rect to,
@SchedulePipModeChangedState int schedulePipModeChangedState,
- boolean moveFromFullscreen, boolean moveToFullscreen,
- boolean replacingExistingAnimation) {
+ @SchedulePipModeChangedState int prevShedulePipModeChangedState,
+ boolean moveFromFullscreen, boolean moveToFullscreen) {
super();
mTarget = target;
mFrom.set(from);
mTo.set(to);
- mSkipAnimationStart = replacingExistingAnimation;
mSchedulePipModeChangedState = schedulePipModeChangedState;
+ mPrevSchedulePipModeChangedState = prevShedulePipModeChangedState;
mMoveFromFullscreen = moveFromFullscreen;
mMoveToFullscreen = moveToFullscreen;
addUpdateListener(this);
@@ -200,7 +195,7 @@
@Override
public void onAnimationStart(Animator animation) {
if (DEBUG) Slog.d(TAG, "onAnimationStart: mTarget=" + mTarget
- + " mSkipAnimationStart=" + mSkipAnimationStart
+ + " mPrevSchedulePipModeChangedState=" + mPrevSchedulePipModeChangedState
+ " mSchedulePipModeChangedState=" + mSchedulePipModeChangedState);
mFinishAnimationAfterTransition = false;
mTmpRect.set(mFrom.left, mFrom.top, mFrom.left + mFrozenTaskWidth,
@@ -210,18 +205,26 @@
// running
updateBooster();
- // Ensure that we have prepared the target for animation before
- // we trigger any size changes, so it can swap surfaces
- // in to appropriate modes, or do as it wishes otherwise.
- if (!mSkipAnimationStart) {
+ // Ensure that we have prepared the target for animation before we trigger any size
+ // changes, so it can swap surfaces in to appropriate modes, or do as it wishes
+ // otherwise.
+ if (mPrevSchedulePipModeChangedState == NO_PIP_MODE_CHANGED_CALLBACKS) {
mTarget.onAnimationStart(mSchedulePipModeChangedState ==
- SCHEDULE_PIP_MODE_CHANGED_ON_START);
+ SCHEDULE_PIP_MODE_CHANGED_ON_START, false /* forceUpdate */);
// When starting an animation from fullscreen, pause here and wait for the
// windows-drawn signal before we start the rest of the transition down into PiP.
if (mMoveFromFullscreen) {
pause();
}
+ } else if (mPrevSchedulePipModeChangedState == SCHEDULE_PIP_MODE_CHANGED_ON_END &&
+ mSchedulePipModeChangedState == SCHEDULE_PIP_MODE_CHANGED_ON_START) {
+ // We are replacing a running animation into PiP, but since it hasn't completed, the
+ // client will not currently receive any picture-in-picture mode change callbacks.
+ // However, we still need to report to them that they are leaving PiP, so this will
+ // force an update via a mode changed callback.
+ mTarget.onAnimationStart(true /* schedulePipModeChangedCallback */,
+ true /* forceUpdate */);
}
// Immediately update the task bounds if they have to become larger, but preserve
@@ -388,6 +391,8 @@
boolean moveFromFullscreen, boolean moveToFullscreen) {
final BoundsAnimator existing = mRunningAnimations.get(target);
final boolean replacing = existing != null;
+ @SchedulePipModeChangedState int prevSchedulePipModeChangedState =
+ NO_PIP_MODE_CHANGED_CALLBACKS;
if (DEBUG) Slog.d(TAG, "animateBounds: target=" + target + " from=" + from + " to=" + to
+ " schedulePipModeChangedState=" + schedulePipModeChangedState
@@ -403,6 +408,9 @@
return existing;
}
+ // Save the previous state
+ prevSchedulePipModeChangedState = existing.mSchedulePipModeChangedState;
+
// Update the PiP callback states if we are replacing the animation
if (existing.mSchedulePipModeChangedState == SCHEDULE_PIP_MODE_CHANGED_ON_START) {
if (schedulePipModeChangedState == SCHEDULE_PIP_MODE_CHANGED_ON_START) {
@@ -428,7 +436,8 @@
existing.cancel();
}
final BoundsAnimator animator = new BoundsAnimator(target, from, to,
- schedulePipModeChangedState, moveFromFullscreen, moveToFullscreen, replacing);
+ schedulePipModeChangedState, prevSchedulePipModeChangedState,
+ moveFromFullscreen, moveToFullscreen);
mRunningAnimations.put(target, animator);
animator.setFloatValues(0f, 1f);
animator.setDuration((animationDuration != -1 ? animationDuration
diff --git a/services/core/java/com/android/server/wm/BoundsAnimationTarget.java b/services/core/java/com/android/server/wm/BoundsAnimationTarget.java
index 8b1bf7b..647a2d6 100644
--- a/services/core/java/com/android/server/wm/BoundsAnimationTarget.java
+++ b/services/core/java/com/android/server/wm/BoundsAnimationTarget.java
@@ -31,7 +31,7 @@
* @param schedulePipModeChangedCallback whether or not to schedule the PiP mode changed
* callbacks
*/
- void onAnimationStart(boolean schedulePipModeChangedCallback);
+ void onAnimationStart(boolean schedulePipModeChangedCallback, boolean forceUpdate);
/**
* Sets the size of the target (without any intermediate steps, like scheduling animation)
diff --git a/services/core/java/com/android/server/wm/PinnedStackWindowController.java b/services/core/java/com/android/server/wm/PinnedStackWindowController.java
index 989e8f2..fc0baad 100644
--- a/services/core/java/com/android/server/wm/PinnedStackWindowController.java
+++ b/services/core/java/com/android/server/wm/PinnedStackWindowController.java
@@ -202,10 +202,12 @@
*/
/** Calls directly into activity manager so window manager lock shouldn't held. */
- public void updatePictureInPictureModeForPinnedStackAnimation(Rect targetStackBounds) {
+ public void updatePictureInPictureModeForPinnedStackAnimation(Rect targetStackBounds,
+ boolean forceUpdate) {
if (mListener != null) {
PinnedStackWindowListener listener = (PinnedStackWindowListener) mListener;
- listener.updatePictureInPictureModeForPinnedStackAnimation(targetStackBounds);
+ listener.updatePictureInPictureModeForPinnedStackAnimation(targetStackBounds,
+ forceUpdate);
}
}
}
diff --git a/services/core/java/com/android/server/wm/PinnedStackWindowListener.java b/services/core/java/com/android/server/wm/PinnedStackWindowListener.java
index 12b9c1f..33e8a60 100644
--- a/services/core/java/com/android/server/wm/PinnedStackWindowListener.java
+++ b/services/core/java/com/android/server/wm/PinnedStackWindowListener.java
@@ -28,5 +28,6 @@
* Called when the stack container pinned stack animation will change the picture-in-picture
* mode. This is a direct call into ActivityManager.
*/
- default void updatePictureInPictureModeForPinnedStackAnimation(Rect targetStackBounds) {}
+ default void updatePictureInPictureModeForPinnedStackAnimation(Rect targetStackBounds,
+ boolean forceUpdate) {}
}
diff --git a/services/core/java/com/android/server/wm/TaskStack.java b/services/core/java/com/android/server/wm/TaskStack.java
index 39878cc..55151a1 100644
--- a/services/core/java/com/android/server/wm/TaskStack.java
+++ b/services/core/java/com/android/server/wm/TaskStack.java
@@ -1498,7 +1498,7 @@
}
@Override // AnimatesBounds
- public void onAnimationStart(boolean schedulePipModeChangedCallback) {
+ public void onAnimationStart(boolean schedulePipModeChangedCallback, boolean forceUpdate) {
// Hold the lock since this is called from the BoundsAnimator running on the UiThread
synchronized (mService.mWindowMap) {
mBoundsAnimatingRequested = false;
@@ -1523,9 +1523,11 @@
final PinnedStackWindowController controller =
(PinnedStackWindowController) getController();
if (schedulePipModeChangedCallback && controller != null) {
- // We need to schedule the PiP mode change after the animation down, so use the
- // final bounds
- controller.updatePictureInPictureModeForPinnedStackAnimation(null);
+ // We need to schedule the PiP mode change before the animation up. It is possible
+ // in this case for the animation down to not have been completed, so always
+ // force-schedule and update to the client to ensure that it is notified that it
+ // is no longer in picture-in-picture mode
+ controller.updatePictureInPictureModeForPinnedStackAnimation(null, forceUpdate);
}
}
}
@@ -1553,7 +1555,7 @@
// We need to schedule the PiP mode change after the animation down, so use the
// final bounds
controller.updatePictureInPictureModeForPinnedStackAnimation(
- mBoundsAnimationTarget);
+ mBoundsAnimationTarget, false /* forceUpdate */);
}
if (finalStackSize != null) {
diff --git a/services/tests/servicestests/src/com/android/server/wm/BoundsAnimationControllerTests.java b/services/tests/servicestests/src/com/android/server/wm/BoundsAnimationControllerTests.java
index 9d32496..0081214 100644
--- a/services/tests/servicestests/src/com/android/server/wm/BoundsAnimationControllerTests.java
+++ b/services/tests/servicestests/src/com/android/server/wm/BoundsAnimationControllerTests.java
@@ -126,6 +126,7 @@
boolean mMovedToFullscreen;
boolean mAnimationStarted;
boolean mSchedulePipModeChangedOnStart;
+ boolean mForcePipModeChangedCallback;
boolean mAnimationEnded;
Rect mAnimationEndFinalStackBounds;
boolean mSchedulePipModeChangedOnEnd;
@@ -140,6 +141,7 @@
mAnimationStarted = false;
mAnimationEnded = false;
mAnimationEndFinalStackBounds = null;
+ mForcePipModeChangedCallback = false;
mSchedulePipModeChangedOnStart = false;
mSchedulePipModeChangedOnEnd = false;
mStackBounds = from;
@@ -148,10 +150,11 @@
}
@Override
- public void onAnimationStart(boolean schedulePipModeChangedCallback) {
+ public void onAnimationStart(boolean schedulePipModeChangedCallback, boolean forceUpdate) {
mAwaitingAnimationStart = false;
mAnimationStarted = true;
mSchedulePipModeChangedOnStart = schedulePipModeChangedCallback;
+ mForcePipModeChangedCallback = forceUpdate;
}
@Override
@@ -232,7 +235,7 @@
return this;
}
- BoundsAnimationDriver restart(Rect to) {
+ BoundsAnimationDriver restart(Rect to, boolean expectStartedAndPipModeChangedCallback) {
if (mAnimator == null) {
throw new IllegalArgumentException("Call start() to start a new animation");
}
@@ -251,8 +254,15 @@
assertSame(oldAnimator, mAnimator);
}
- // No animation start for replacing animation
- assertTrue(!mTarget.mAnimationStarted);
+ if (expectStartedAndPipModeChangedCallback) {
+ // Replacing animation with pending pip mode changed callback, ensure we update
+ assertTrue(mTarget.mAnimationStarted);
+ assertTrue(mTarget.mSchedulePipModeChangedOnStart);
+ assertTrue(mTarget.mForcePipModeChangedCallback);
+ } else {
+ // No animation start for replacing animation
+ assertTrue(!mTarget.mAnimationStarted);
+ }
mTarget.mAnimationStarted = true;
return this;
}
@@ -467,7 +477,7 @@
mDriver.start(BOUNDS_FULL, BOUNDS_FLOATING)
.expectStarted(!SCHEDULE_PIP_MODE_CHANGED)
.update(0.25f)
- .restart(BOUNDS_FLOATING)
+ .restart(BOUNDS_FLOATING, false /* expectStartedAndPipModeChangedCallback */)
.end()
.expectEnded(SCHEDULE_PIP_MODE_CHANGED, !MOVE_TO_FULLSCREEN);
}
@@ -478,7 +488,8 @@
mDriver.start(BOUNDS_FULL, BOUNDS_FLOATING)
.expectStarted(!SCHEDULE_PIP_MODE_CHANGED)
.update(0.25f)
- .restart(BOUNDS_SMALLER_FLOATING)
+ .restart(BOUNDS_SMALLER_FLOATING,
+ false /* expectStartedAndPipModeChangedCallback */)
.end()
.expectEnded(SCHEDULE_PIP_MODE_CHANGED, !MOVE_TO_FULLSCREEN);
}
@@ -486,10 +497,12 @@
@UiThreadTest
@Test
public void testFullscreenToFloatingCancelFromAnimationToFullscreenBounds() throws Exception {
+ // When animating from fullscreen and the animation is interruped, we expect the animation
+ // start callback to be made, with a forced pip mode change callback
mDriver.start(BOUNDS_FULL, BOUNDS_FLOATING)
.expectStarted(!SCHEDULE_PIP_MODE_CHANGED)
.update(0.25f)
- .restart(BOUNDS_FULL)
+ .restart(BOUNDS_FULL, true /* expectStartedAndPipModeChangedCallback */)
.end()
.expectEnded(!SCHEDULE_PIP_MODE_CHANGED, MOVE_TO_FULLSCREEN);
}
@@ -512,7 +525,7 @@
mDriver.start(BOUNDS_FLOATING, BOUNDS_FULL)
.expectStarted(SCHEDULE_PIP_MODE_CHANGED)
.update(0.25f)
- .restart(BOUNDS_FULL)
+ .restart(BOUNDS_FULL, false /* expectStartedAndPipModeChangedCallback */)
.end()
.expectEnded(!SCHEDULE_PIP_MODE_CHANGED, MOVE_TO_FULLSCREEN);
}
@@ -523,7 +536,8 @@
mDriver.start(BOUNDS_FLOATING, BOUNDS_FULL)
.expectStarted(SCHEDULE_PIP_MODE_CHANGED)
.update(0.25f)
- .restart(BOUNDS_SMALLER_FLOATING)
+ .restart(BOUNDS_SMALLER_FLOATING,
+ false /* expectStartedAndPipModeChangedCallback */)
.end()
.expectEnded(SCHEDULE_PIP_MODE_CHANGED, !MOVE_TO_FULLSCREEN);
}