From ac611a12493c3b1ffa60a3a2dfd6a0e1a700ca40 Mon Sep 17 00:00:00 2001 From: Hongwei Wang Date: Tue, 26 Jul 2022 17:14:51 -0700 Subject: [PATCH] Ensure isInPictureInPicture result in onPause We use to set Activity#mIsInPictureInPictureMode to be true when app requests Activity#enterPictureInPictureMode and therefore when they receive the onPause callback, Activity#isInPictureInPictureMode can be true. With the new auto-enter-pip API, this code path is no more relevant and here we add extra info to PauseActivityItem to set the same flag when WM puts the Activity into PiP mode due to auto-enter. Video: http://recall/-/aaaaaabFQoRHlzixHdtY/yXi24xi6TgYGhoqB31Oov Bug: 239618513 Test: follow the reproduce steps in bug, see video Test: atest PinnedStackTests Change-Id: I81ece7d3aad8eb868d95f106f6825612e0741a19 --- core/java/android/app/Activity.java | 3 ++- core/java/android/app/ActivityThread.java | 11 +++++++++-- .../android/app/ClientTransactionHandler.java | 4 ++-- .../servertransaction/PauseActivityItem.java | 19 ++++++++++++++----- .../TransactionExecutor.java | 3 ++- .../servertransaction/ObjectPoolTests.java | 6 +++--- .../TransactionParcelTests.java | 3 ++- .../activity/ActivityThreadClientTest.java | 4 ++-- .../com/android/server/wm/ActivityRecord.java | 3 ++- .../server/wm/ActivityTaskManagerService.java | 2 +- .../com/android/server/wm/TaskFragment.java | 7 ++++--- 11 files changed, 43 insertions(+), 22 deletions(-) diff --git a/core/java/android/app/Activity.java b/core/java/android/app/Activity.java index 4ea0c32d3b369..7c1f9c80eec11 100644 --- a/core/java/android/app/Activity.java +++ b/core/java/android/app/Activity.java @@ -980,7 +980,8 @@ public class Activity extends ContextThemeWrapper boolean mEnterAnimationComplete; private boolean mIsInMultiWindowMode; - private boolean mIsInPictureInPictureMode; + /** @hide */ + boolean mIsInPictureInPictureMode; private boolean mShouldDockBigOverlays; diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index f384fa9e6a0bd..a70a1a8d51de9 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -4162,7 +4162,8 @@ public final class ActivityThread extends ClientTransactionHandler private void schedulePauseWithUserLeavingHint(ActivityClientRecord r) { final ClientTransaction transaction = ClientTransaction.obtain(this.mAppThread, r.token); transaction.setLifecycleStateRequest(PauseActivityItem.obtain(r.activity.isFinishing(), - /* userLeaving */ true, r.activity.mConfigChangeFlags, /* dontReport */ false)); + /* userLeaving */ true, r.activity.mConfigChangeFlags, /* dontReport */ false, + /* autoEnteringPip */ false)); executeTransaction(transaction); } @@ -4952,12 +4953,18 @@ public final class ActivityThread extends ClientTransactionHandler @Override public void handlePauseActivity(ActivityClientRecord r, boolean finished, boolean userLeaving, - int configChanges, PendingTransactionActions pendingActions, String reason) { + int configChanges, boolean autoEnteringPip, PendingTransactionActions pendingActions, + String reason) { if (userLeaving) { performUserLeavingActivity(r); } r.activity.mConfigChangeFlags |= configChanges; + if (autoEnteringPip) { + // Set mIsInPictureInPictureMode earlier in case of auto-enter-pip, see also + // {@link Activity#enterPictureInPictureMode(PictureInPictureParams)}. + r.activity.mIsInPictureInPictureMode = true; + } performPauseActivity(r, finished, reason, pendingActions); // Make sure any pending writes are now committed. diff --git a/core/java/android/app/ClientTransactionHandler.java b/core/java/android/app/ClientTransactionHandler.java index 65e6ab7a21376..a7566fdaae64a 100644 --- a/core/java/android/app/ClientTransactionHandler.java +++ b/core/java/android/app/ClientTransactionHandler.java @@ -97,8 +97,8 @@ public abstract class ClientTransactionHandler { /** Pause the activity. */ public abstract void handlePauseActivity(@NonNull ActivityClientRecord r, boolean finished, - boolean userLeaving, int configChanges, PendingTransactionActions pendingActions, - String reason); + boolean userLeaving, int configChanges, boolean autoEnteringPip, + PendingTransactionActions pendingActions, String reason); /** * Resume the activity. diff --git a/core/java/android/app/servertransaction/PauseActivityItem.java b/core/java/android/app/servertransaction/PauseActivityItem.java index 813e0f93a1f71..965e761ebfb3b 100644 --- a/core/java/android/app/servertransaction/PauseActivityItem.java +++ b/core/java/android/app/servertransaction/PauseActivityItem.java @@ -39,13 +39,14 @@ public class PauseActivityItem extends ActivityLifecycleItem { private boolean mUserLeaving; private int mConfigChanges; private boolean mDontReport; + private boolean mAutoEnteringPip; @Override public void execute(ClientTransactionHandler client, ActivityClientRecord r, PendingTransactionActions pendingActions) { Trace.traceBegin(TRACE_TAG_ACTIVITY_MANAGER, "activityPause"); - client.handlePauseActivity(r, mFinished, mUserLeaving, mConfigChanges, pendingActions, - "PAUSE_ACTIVITY_ITEM"); + client.handlePauseActivity(r, mFinished, mUserLeaving, mConfigChanges, mAutoEnteringPip, + pendingActions, "PAUSE_ACTIVITY_ITEM"); Trace.traceEnd(TRACE_TAG_ACTIVITY_MANAGER); } @@ -71,7 +72,7 @@ public class PauseActivityItem extends ActivityLifecycleItem { /** Obtain an instance initialized with provided params. */ public static PauseActivityItem obtain(boolean finished, boolean userLeaving, int configChanges, - boolean dontReport) { + boolean dontReport, boolean autoEnteringPip) { PauseActivityItem instance = ObjectPool.obtain(PauseActivityItem.class); if (instance == null) { instance = new PauseActivityItem(); @@ -80,6 +81,7 @@ public class PauseActivityItem extends ActivityLifecycleItem { instance.mUserLeaving = userLeaving; instance.mConfigChanges = configChanges; instance.mDontReport = dontReport; + instance.mAutoEnteringPip = autoEnteringPip; return instance; } @@ -94,6 +96,7 @@ public class PauseActivityItem extends ActivityLifecycleItem { instance.mUserLeaving = false; instance.mConfigChanges = 0; instance.mDontReport = true; + instance.mAutoEnteringPip = false; return instance; } @@ -105,6 +108,7 @@ public class PauseActivityItem extends ActivityLifecycleItem { mUserLeaving = false; mConfigChanges = 0; mDontReport = false; + mAutoEnteringPip = false; ObjectPool.recycle(this); } @@ -117,6 +121,7 @@ public class PauseActivityItem extends ActivityLifecycleItem { dest.writeBoolean(mUserLeaving); dest.writeInt(mConfigChanges); dest.writeBoolean(mDontReport); + dest.writeBoolean(mAutoEnteringPip); } /** Read from Parcel. */ @@ -125,6 +130,7 @@ public class PauseActivityItem extends ActivityLifecycleItem { mUserLeaving = in.readBoolean(); mConfigChanges = in.readInt(); mDontReport = in.readBoolean(); + mAutoEnteringPip = in.readBoolean(); } public static final @NonNull Creator CREATOR = @@ -148,7 +154,8 @@ public class PauseActivityItem extends ActivityLifecycleItem { } final PauseActivityItem other = (PauseActivityItem) o; return mFinished == other.mFinished && mUserLeaving == other.mUserLeaving - && mConfigChanges == other.mConfigChanges && mDontReport == other.mDontReport; + && mConfigChanges == other.mConfigChanges && mDontReport == other.mDontReport + && mAutoEnteringPip == other.mAutoEnteringPip; } @Override @@ -158,12 +165,14 @@ public class PauseActivityItem extends ActivityLifecycleItem { result = 31 * result + (mUserLeaving ? 1 : 0); result = 31 * result + mConfigChanges; result = 31 * result + (mDontReport ? 1 : 0); + result = 31 * result + (mAutoEnteringPip ? 1 : 0); return result; } @Override public String toString() { return "PauseActivityItem{finished=" + mFinished + ",userLeaving=" + mUserLeaving - + ",configChanges=" + mConfigChanges + ",dontReport=" + mDontReport + "}"; + + ",configChanges=" + mConfigChanges + ",dontReport=" + mDontReport + + ",autoEnteringPip=" + mAutoEnteringPip + "}"; } } diff --git a/core/java/android/app/servertransaction/TransactionExecutor.java b/core/java/android/app/servertransaction/TransactionExecutor.java index 25ff8a78a0c82..de1d38a64163f 100644 --- a/core/java/android/app/servertransaction/TransactionExecutor.java +++ b/core/java/android/app/servertransaction/TransactionExecutor.java @@ -227,7 +227,8 @@ public class TransactionExecutor { break; case ON_PAUSE: mTransactionHandler.handlePauseActivity(r, false /* finished */, - false /* userLeaving */, 0 /* configChanges */, mPendingActions, + false /* userLeaving */, 0 /* configChanges */, + false /* autoEnteringPip */, mPendingActions, "LIFECYCLER_PAUSE_ACTIVITY"); break; case ON_STOP: diff --git a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java index 50639be57f22c..942e1cf3eed59 100644 --- a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java @@ -223,15 +223,15 @@ public class ObjectPoolTests { @Test public void testRecyclePauseActivityItemItem() { - PauseActivityItem emptyItem = PauseActivityItem.obtain(false, false, 0, false); - PauseActivityItem item = PauseActivityItem.obtain(true, true, 5, true); + PauseActivityItem emptyItem = PauseActivityItem.obtain(false, false, 0, false, false); + PauseActivityItem item = PauseActivityItem.obtain(true, true, 5, true, true); assertNotSame(item, emptyItem); assertFalse(item.equals(emptyItem)); item.recycle(); assertEquals(item, emptyItem); - PauseActivityItem item2 = PauseActivityItem.obtain(true, false, 5, true); + PauseActivityItem item2 = PauseActivityItem.obtain(true, false, 5, true, true); assertSame(item, item2); assertFalse(item2.equals(emptyItem)); } diff --git a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java index 0eca0a8cb1a7f..c868963c4d02a 100644 --- a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java @@ -235,7 +235,8 @@ public class TransactionParcelTests { public void testPause() { // Write to parcel PauseActivityItem item = PauseActivityItem.obtain(true /* finished */, - true /* userLeaving */, 135 /* configChanges */, true /* dontReport */); + true /* userLeaving */, 135 /* configChanges */, true /* dontReport */, + true /* autoEnteringPip */); writeAndPrepareForReading(item); // Read from parcel and assert diff --git a/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java b/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java index 47f70ddf2d427..ad72d49d2d6d0 100644 --- a/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java +++ b/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java @@ -299,8 +299,8 @@ public class ActivityThreadClientTest { private void pauseActivity(ActivityClientRecord r) { mThread.handlePauseActivity(r, false /* finished */, - false /* userLeaving */, 0 /* configChanges */, null /* pendingActions */, - "test"); + false /* userLeaving */, 0 /* configChanges */, false /* autoEnteringPip */, + null /* pendingActions */, "test"); } private void stopActivity(ActivityClientRecord r) { diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index 3a2c9c831cb38..e1a03d3ddefbd 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -5877,7 +5877,8 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A try { mAtmService.getLifecycleManager().scheduleTransaction(app.getThread(), token, PauseActivityItem.obtain(finishing, false /* userLeaving */, - configChangeFlags, false /* dontReport */)); + configChangeFlags, false /* dontReport */, + false /* autoEnteringPip */)); } catch (Exception e) { Slog.w(TAG, "Exception thrown sending pause: " + intent.getComponent(), e); } diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index 73237189da163..b1ba10177b811 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -3560,7 +3560,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { // Continue the pausing process after entering pip. if (r.isState(PAUSING)) { r.getTask().schedulePauseActivity(r, false /* userLeaving */, - false /* pauseImmediately */, "auto-pip"); + false /* pauseImmediately */, true /* autoEnteringPip */, "auto-pip"); } } }; diff --git a/services/core/java/com/android/server/wm/TaskFragment.java b/services/core/java/com/android/server/wm/TaskFragment.java index 5f85a14010141..0a86f739e7e1d 100644 --- a/services/core/java/com/android/server/wm/TaskFragment.java +++ b/services/core/java/com/android/server/wm/TaskFragment.java @@ -1621,7 +1621,8 @@ class TaskFragment extends WindowContainer { ProtoLog.d(WM_DEBUG_STATES, "Auto-PIP allowed, entering PIP mode " + "directly: %s, didAutoPip: %b", prev, didAutoPip); } else { - schedulePauseActivity(prev, userLeaving, pauseImmediately, reason); + schedulePauseActivity(prev, userLeaving, pauseImmediately, + false /* autoEnteringPip */, reason); } } else { mPausingActivity = null; @@ -1675,7 +1676,7 @@ class TaskFragment extends WindowContainer { } void schedulePauseActivity(ActivityRecord prev, boolean userLeaving, - boolean pauseImmediately, String reason) { + boolean pauseImmediately, boolean autoEnteringPip, String reason) { ProtoLog.v(WM_DEBUG_STATES, "Enqueueing pending pause: %s", prev); try { EventLogTags.writeWmPauseActivity(prev.mUserId, System.identityHashCode(prev), @@ -1683,7 +1684,7 @@ class TaskFragment extends WindowContainer { mAtmService.getLifecycleManager().scheduleTransaction(prev.app.getThread(), prev.token, PauseActivityItem.obtain(prev.finishing, userLeaving, - prev.configChangeFlags, pauseImmediately)); + prev.configChangeFlags, pauseImmediately, autoEnteringPip)); } catch (Exception e) { // Ignore exception, if process died other code will cleanup. Slog.w(TAG, "Exception thrown during pause", e);