Ensure TaskOrganizer works for newly created pinned stack
Steps to reproduce the bug:
- Restart the device
- Open Duo app and make a call
- When in call, try entering PiP
Duo application contains multiple activities and we create a new stack
from its top activity when moving it to pinned mode. This newly created
stack does not carry on the existing PictureInPictureParams, nor it is
marked as visible. Therefore, no onTaskAppeared would be sent to the
task organizer.
Also in this change:
- Removed aspectRatio and sourceHintBounds used to be passed into
RootWindowContainer#moveActivityToPinnedStack, they are used in
moveActivityToPinnedStack and one should refer to the
PictureInPictureParams set on ActivityRecord
- Added a null check for existing token when onTaskInfoChanged is
invoked in PipTaskOrganizer, it should be a fatal error that
onTaskInfoChanged is called ahead of onTaskAppeared
Bug: 152933995
Test: manually enter/exit PiP mode for Duo app
Test: atest RootActivityContainerTests
Test: atest ActivityRecordTests
Change-Id: Ifa9ad8768ba47ce043b8dd86cadc729931edcb14
diff --git a/packages/SystemUI/src/com/android/systemui/pip/PipTaskOrganizer.java b/packages/SystemUI/src/com/android/systemui/pip/PipTaskOrganizer.java
index a95d6b7..f89a01e 100644
--- a/packages/SystemUI/src/com/android/systemui/pip/PipTaskOrganizer.java
+++ b/packages/SystemUI/src/com/android/systemui/pip/PipTaskOrganizer.java
@@ -126,7 +126,7 @@
};
@SuppressWarnings("unchecked")
- private Handler.Callback mUpdateCallbacks = (msg) -> {
+ private final Handler.Callback mUpdateCallbacks = (msg) -> {
SomeArgs args = (SomeArgs) msg.obj;
Consumer<Rect> updateBoundsCallback = (Consumer<Rect>) args.arg1;
switch (msg.what) {
@@ -282,7 +282,7 @@
*/
@Override
public void onTaskVanished(ActivityManager.RunningTaskInfo info) {
- WindowContainerToken token = info.token;
+ final WindowContainerToken token = info.token;
Objects.requireNonNull(token, "Requires valid WindowContainerToken");
if (token.asBinder() != mToken.asBinder()) {
Log.wtf(TAG, "Unrecognized token: " + token);
@@ -297,6 +297,7 @@
@Override
public void onTaskInfoChanged(ActivityManager.RunningTaskInfo info) {
+ Objects.requireNonNull(mToken, "onTaskInfoChanged requires valid existing mToken");
final PictureInPictureParams newParams = info.pictureInPictureParams;
if (!shouldUpdateDestinationBounds(newParams)) {
Log.d(TAG, "Ignored onTaskInfoChanged with PiP param: " + newParams);
@@ -375,7 +376,7 @@
@PipAnimationController.TransitionDirection int direction, int durationMs,
Consumer<Rect> updateBoundsCallback) {
if (!mInPip) {
- // Ignore animation when we are no longer in PIP
+ // can be initiated in other component, ignore if we are no longer in PIP
return;
}
SomeArgs args = SomeArgs.obtain();
@@ -427,6 +428,10 @@
private void scheduleFinishResizePip(SurfaceControl.Transaction tx,
Rect destinationBounds, @PipAnimationController.TransitionDirection int direction,
Consumer<Rect> updateBoundsCallback) {
+ if (!mInPip) {
+ // can be initiated in other component, ignore if we are no longer in PIP
+ return;
+ }
SomeArgs args = SomeArgs.obtain();
args.arg1 = updateBoundsCallback;
args.arg2 = tx;
@@ -441,7 +446,7 @@
public void scheduleOffsetPip(Rect originalBounds, int offset, int duration,
Consumer<Rect> updateBoundsCallback) {
if (!mInPip) {
- // Ignore offsets when we are no longer in PIP
+ // can be initiated in other component, ignore if we are no longer in PIP
return;
}
SomeArgs args = SomeArgs.obtain();
diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java
index ed9b019..0b33cbd 100644
--- a/services/core/java/com/android/server/wm/ActivityRecord.java
+++ b/services/core/java/com/android/server/wm/ActivityRecord.java
@@ -1298,6 +1298,19 @@
if (newTask != null && isState(RESUMED)) {
newTask.setResumedActivity(this, "onParentChanged");
}
+
+ if (stack != null && stack.topRunningActivity() == this) {
+ // carry over the PictureInPictureParams to the parent stack without calling
+ // TaskOrganizerController#dispatchTaskInfoChanged.
+ // this is to ensure the stack holding up-to-dated pinned stack information
+ // when activity is re-parented to enter pip mode, see also
+ // RootWindowContainer#moveActivityToPinnedStack
+ stack.mPictureInPictureParams.copyOnlySet(pictureInPictureArgs);
+ // make ensure the TaskOrganizer still works after re-parenting
+ if (firstWindowDrawn) {
+ stack.setHasBeenVisible(true);
+ }
+ }
}
private void updateColorTransform() {
diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
index c253cd2..08683ca 100644
--- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
+++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
@@ -4095,11 +4095,8 @@
r.setPictureInPictureParams(params);
final float aspectRatio = r.pictureInPictureArgs.getAspectRatio();
final List<RemoteAction> actions = r.pictureInPictureArgs.getActions();
- // Adjust the source bounds by the insets for the transition down
- final Rect sourceBounds = new Rect(
- r.pictureInPictureArgs.getSourceRectHint());
mRootWindowContainer.moveActivityToPinnedStack(
- r, sourceBounds, aspectRatio, "enterPictureInPictureMode");
+ r, "enterPictureInPictureMode");
final ActivityStack stack = r.getRootTask();
stack.setPictureInPictureAspectRatio(aspectRatio);
stack.setPictureInPictureActions(actions);
diff --git a/services/core/java/com/android/server/wm/RootWindowContainer.java b/services/core/java/com/android/server/wm/RootWindowContainer.java
index 1a70de7..e8f7ba5 100644
--- a/services/core/java/com/android/server/wm/RootWindowContainer.java
+++ b/services/core/java/com/android/server/wm/RootWindowContainer.java
@@ -2147,13 +2147,11 @@
return false;
}
- moveActivityToPinnedStack(r, null /* sourceBounds */, 0f /* aspectRatio */,
- "moveTopActivityToPinnedStack");
+ moveActivityToPinnedStack(r, "moveTopActivityToPinnedStack");
return true;
}
- void moveActivityToPinnedStack(ActivityRecord r, Rect sourceHintBounds, float aspectRatio,
- String reason) {
+ void moveActivityToPinnedStack(ActivityRecord r, String reason) {
mService.deferWindowLayout();
final TaskDisplayArea taskDisplayArea = r.getDisplayArea();
@@ -2176,17 +2174,19 @@
final ActivityStack stack;
if (singleActivity) {
stack = r.getRootTask();
- stack.setWindowingMode(WINDOWING_MODE_PINNED);
} else {
// In the case of multiple activities, we will create a new task for it and then
// move the PIP activity into the task.
- stack = taskDisplayArea.createStack(WINDOWING_MODE_PINNED, r.getActivityType(),
+ stack = taskDisplayArea.createStack(WINDOWING_MODE_UNDEFINED, r.getActivityType(),
ON_TOP, r.info, r.intent, false /* createdByOrganizer */);
// There are multiple activities in the task and moving the top activity should
// reveal/leave the other activities in their original task.
- r.reparent(stack, MAX_VALUE, "moveActivityToStack");
+ // On the other hand, ActivityRecord#onParentChanged takes care of setting the
+ // up-to-dated pinned stack information on this newly created stack.
+ r.reparent(stack, MAX_VALUE, reason);
}
+ stack.setWindowingMode(WINDOWING_MODE_PINNED);
// Reset the state that indicates it can enter PiP while pausing after we've moved it
// to the pinned stack
diff --git a/services/tests/wmtests/src/com/android/server/wm/RootActivityContainerTests.java b/services/tests/wmtests/src/com/android/server/wm/RootActivityContainerTests.java
index 7613655..3c90515 100644
--- a/services/tests/wmtests/src/com/android/server/wm/RootActivityContainerTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/RootActivityContainerTests.java
@@ -58,7 +58,6 @@
import android.content.pm.ApplicationInfo;
import android.content.pm.ResolveInfo;
import android.content.res.Resources;
-import android.graphics.Rect;
import android.platform.test.annotations.Presubmit;
import android.util.Pair;
@@ -126,9 +125,7 @@
ensureStackPlacement(mFullscreenStack, firstActivity, secondActivity);
// Move first activity to pinned stack.
- final Rect sourceBounds = new Rect();
- mRootWindowContainer.moveActivityToPinnedStack(firstActivity, sourceBounds,
- 0f /*aspectRatio*/, "initialMove");
+ mRootWindowContainer.moveActivityToPinnedStack(firstActivity, "initialMove");
final TaskDisplayArea taskDisplayArea = mFullscreenStack.getDisplayArea();
ActivityStack pinnedStack = taskDisplayArea.getRootPinnedTask();
@@ -137,8 +134,7 @@
ensureStackPlacement(mFullscreenStack, secondActivity);
// Move second activity to pinned stack.
- mRootWindowContainer.moveActivityToPinnedStack(secondActivity, sourceBounds,
- 0f /*aspectRatio*/, "secondMove");
+ mRootWindowContainer.moveActivityToPinnedStack(secondActivity, "secondMove");
// Need to get stacks again as a new instance might have been created.
pinnedStack = taskDisplayArea.getRootPinnedTask();