From 332036a29e65e1f80c8115077c3c832be7ddcdc5 Mon Sep 17 00:00:00 2001 From: Louis Chang Date: Wed, 28 Jul 2021 15:46:25 +0800 Subject: [PATCH] Sending TaskFragment token to client for the launching activity Activity#onCreate is called before receiving the TaskFragment appeared event in the client process. So, the client split controller created another split and resulted in unexpected behaviors. Bug: 194860679 Test: start activity to side Test: wm presubmit Change-Id: Iac34bd39376940c5c0224c8687ad9858a4190e20 --- core/java/android/app/ActivityThread.java | 6 ++- .../servertransaction/LaunchActivityItem.java | 20 ++++++--- .../servertransaction/ObjectPoolTests.java | 3 +- .../app/servertransaction/TestUtils.java | 8 +++- .../TransactionParcelTests.java | 1 + .../activity/ActivityThreadClientTest.java | 2 +- .../extensions/organizer/SplitController.java | 22 ++++++++- .../organizer/TaskFragmentContainer.java | 45 ++++++++++++------- .../server/wm/ActivityTaskSupervisor.java | 6 ++- .../com/android/server/wm/TaskFragment.java | 4 ++ 10 files changed, 87 insertions(+), 30 deletions(-) diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index 49836773fb82c..7d70441c1cd24 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -530,6 +530,8 @@ public final class ActivityThread extends ClientTransactionHandler // A reusable token for other purposes, e.g. content capture, translation. It shouldn't be // used without security checks public IBinder shareableActivityToken; + // The token of the TaskFragment that embedded this activity. + @Nullable public IBinder mTaskFragmentToken; int ident; @UnsupportedAppUsage Intent intent; @@ -623,7 +625,8 @@ public final class ActivityThread extends ClientTransactionHandler List pendingNewIntents, ActivityOptions activityOptions, boolean isForward, ProfilerInfo profilerInfo, ClientTransactionHandler client, IBinder assistToken, FixedRotationAdjustments fixedRotationAdjustments, - IBinder shareableActivityToken, boolean launchedFromBubble) { + IBinder shareableActivityToken, boolean launchedFromBubble, + IBinder taskFragmentToken) { this.token = token; this.assistToken = assistToken; this.shareableActivityToken = shareableActivityToken; @@ -645,6 +648,7 @@ public final class ActivityThread extends ClientTransactionHandler mActivityOptions = activityOptions; mPendingFixedRotationAdjustments = fixedRotationAdjustments; mLaunchedFromBubble = launchedFromBubble; + mTaskFragmentToken = taskFragmentToken; init(); } diff --git a/core/java/android/app/servertransaction/LaunchActivityItem.java b/core/java/android/app/servertransaction/LaunchActivityItem.java index 34e4fcdb9140e..37cbccb897358 100644 --- a/core/java/android/app/servertransaction/LaunchActivityItem.java +++ b/core/java/android/app/servertransaction/LaunchActivityItem.java @@ -73,6 +73,7 @@ public class LaunchActivityItem extends ClientTransactionItem { private IBinder mAssistToken; private IBinder mShareableActivityToken; private boolean mLaunchedFromBubble; + private IBinder mTaskFragmentToken; /** * It is only non-null if the process is the first time to launch activity. It is only an * optimization for quick look up of the interface so the field is ignored for comparison. @@ -86,7 +87,7 @@ public class LaunchActivityItem extends ClientTransactionItem { mOverrideConfig, mCompatInfo, mReferrer, mVoiceInteractor, mState, mPersistentState, mPendingResults, mPendingNewIntents, mActivityOptions, mIsForward, mProfilerInfo, client, mAssistToken, mFixedRotationAdjustments, mShareableActivityToken, - mLaunchedFromBubble); + mLaunchedFromBubble, mTaskFragmentToken); client.addLaunchingActivity(token, r); client.updateProcessState(mProcState, false); client.updatePendingConfiguration(mCurConfig); @@ -124,7 +125,7 @@ public class LaunchActivityItem extends ClientTransactionItem { boolean isForward, ProfilerInfo profilerInfo, IBinder assistToken, IActivityClientController activityClientController, FixedRotationAdjustments fixedRotationAdjustments, IBinder shareableActivityToken, - boolean launchedFromBubble) { + boolean launchedFromBubble, IBinder taskFragmentToken) { LaunchActivityItem instance = ObjectPool.obtain(LaunchActivityItem.class); if (instance == null) { instance = new LaunchActivityItem(); @@ -133,7 +134,7 @@ public class LaunchActivityItem extends ClientTransactionItem { voiceInteractor, procState, state, persistentState, pendingResults, pendingNewIntents, activityOptions, isForward, profilerInfo, assistToken, activityClientController, fixedRotationAdjustments, shareableActivityToken, - launchedFromBubble); + launchedFromBubble, taskFragmentToken); return instance; } @@ -141,7 +142,7 @@ public class LaunchActivityItem extends ClientTransactionItem { @Override public void recycle() { setValues(this, null, 0, null, null, null, null, null, null, 0, null, null, null, null, - null, false, null, null, null, null, null, false); + null, false, null, null, null, null, null, false, null); ObjectPool.recycle(this); } @@ -172,6 +173,7 @@ public class LaunchActivityItem extends ClientTransactionItem { dest.writeTypedObject(mFixedRotationAdjustments, flags); dest.writeStrongBinder(mShareableActivityToken); dest.writeBoolean(mLaunchedFromBubble); + dest.writeStrongBinder(mTaskFragmentToken); } /** Read from Parcel. */ @@ -190,7 +192,8 @@ public class LaunchActivityItem extends ClientTransactionItem { in.readStrongBinder(), IActivityClientController.Stub.asInterface(in.readStrongBinder()), in.readTypedObject(FixedRotationAdjustments.CREATOR), in.readStrongBinder(), - in.readBoolean()); + in.readBoolean(), + in.readStrongBinder()); } public static final @NonNull Creator CREATOR = @@ -229,7 +232,8 @@ public class LaunchActivityItem extends ClientTransactionItem { && Objects.equals(mProfilerInfo, other.mProfilerInfo) && Objects.equals(mAssistToken, other.mAssistToken) && Objects.equals(mFixedRotationAdjustments, other.mFixedRotationAdjustments) - && Objects.equals(mShareableActivityToken, other.mShareableActivityToken); + && Objects.equals(mShareableActivityToken, other.mShareableActivityToken) + && Objects.equals(mTaskFragmentToken, other.mTaskFragmentToken); } @Override @@ -252,6 +256,7 @@ public class LaunchActivityItem extends ClientTransactionItem { result = 31 * result + Objects.hashCode(mAssistToken); result = 31 * result + Objects.hashCode(mFixedRotationAdjustments); result = 31 * result + Objects.hashCode(mShareableActivityToken); + result = 31 * result + Objects.hashCode(mTaskFragmentToken); return result; } @@ -301,7 +306,7 @@ public class LaunchActivityItem extends ClientTransactionItem { ActivityOptions activityOptions, boolean isForward, ProfilerInfo profilerInfo, IBinder assistToken, IActivityClientController activityClientController, FixedRotationAdjustments fixedRotationAdjustments, IBinder shareableActivityToken, - boolean launchedFromBubble) { + boolean launchedFromBubble, IBinder taskFragmentToken) { instance.mIntent = intent; instance.mIdent = ident; instance.mInfo = info; @@ -323,5 +328,6 @@ public class LaunchActivityItem extends ClientTransactionItem { instance.mFixedRotationAdjustments = fixedRotationAdjustments; instance.mShareableActivityToken = shareableActivityToken; instance.mLaunchedFromBubble = launchedFromBubble; + instance.mTaskFragmentToken = taskFragmentToken; } } diff --git a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java index 9915e3852b8de..50639be57f22c 100644 --- a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java @@ -156,7 +156,8 @@ public class ObjectPoolTests { .setProcState(procState).setState(bundle).setPersistentState(persistableBundle) .setPendingResults(resultInfoList()).setPendingNewIntents(referrerIntentList()) .setIsForward(true).setAssistToken(assistToken) - .setShareableActivityToken(shareableActivityToken).build(); + .setShareableActivityToken(shareableActivityToken) + .setTaskFragmentToken(new Binder()).build(); LaunchActivityItem emptyItem = new LaunchActivityItemBuilder().build(); LaunchActivityItem item = itemSupplier.get(); diff --git a/core/tests/coretests/src/android/app/servertransaction/TestUtils.java b/core/tests/coretests/src/android/app/servertransaction/TestUtils.java index 75da0bfba5812..1173c9210ed5e 100644 --- a/core/tests/coretests/src/android/app/servertransaction/TestUtils.java +++ b/core/tests/coretests/src/android/app/servertransaction/TestUtils.java @@ -112,6 +112,7 @@ class TestUtils { private IBinder mShareableActivityToken; private FixedRotationAdjustments mFixedRotationAdjustments; private boolean mLaunchedFromBubble; + private IBinder mTaskFragmentToken; LaunchActivityItemBuilder setIntent(Intent intent) { mIntent = intent; @@ -213,13 +214,18 @@ class TestUtils { return this; } + LaunchActivityItemBuilder setTaskFragmentToken(IBinder taskFragmentToken) { + mTaskFragmentToken = taskFragmentToken; + return this; + } + LaunchActivityItem build() { return LaunchActivityItem.obtain(mIntent, mIdent, mInfo, mCurConfig, mOverrideConfig, mCompatInfo, mReferrer, mVoiceInteractor, mProcState, mState, mPersistentState, mPendingResults, mPendingNewIntents, mActivityOptions, mIsForward, mProfilerInfo, mAssistToken, null /* activityClientController */, mFixedRotationAdjustments, - mShareableActivityToken, mLaunchedFromBubble); + mShareableActivityToken, mLaunchedFromBubble, mTaskFragmentToken); } } } diff --git a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java index df0c64c810c66..98c9afd2eb6bf 100644 --- a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java @@ -209,6 +209,7 @@ public class TransactionParcelTests { .setPendingNewIntents(referrerIntentList()).setIsForward(true) .setAssistToken(new Binder()).setFixedRotationAdjustments(fixedRotationAdjustments) .setShareableActivityToken(new Binder()) + .setTaskFragmentToken(new Binder()) .build(); writeAndPrepareForReading(item); diff --git a/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java b/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java index 269d8424a78f3..516a5d288c0f2 100644 --- a/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java +++ b/core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java @@ -299,7 +299,7 @@ public class ActivityThreadClientTest { null /* activityOptions */, true /* isForward */, null /* profilerInfo */, mThread /* client */, null /* asssitToken */, null /* fixedRotationAdjustments */, null /* shareableActivityToken */, - false /* launchedFromBubble */); + false /* launchedFromBubble */, null /* taskfragmentToken */); } @Override diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/organizer/SplitController.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/organizer/SplitController.java index 407c43d9226aa..e43c5bf71c050 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/organizer/SplitController.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/organizer/SplitController.java @@ -155,7 +155,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen final List splitRules = getSplitRules(); final TaskFragmentContainer currentContainer = getContainerWithActivity( - launchedActivity.getActivityToken()); + launchedActivity.getActivityToken(), launchedActivity); // Check if the activity is configured to always be expanded. if (shouldExpand(componentName, splitRules)) { @@ -247,9 +247,29 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen */ @Nullable TaskFragmentContainer getContainerWithActivity(@NonNull IBinder activityToken) { + return getContainerWithActivity(activityToken, null /* activityToAdd */); + } + + /** + * This method can only be called from {@link #onActivityCreated(Activity)}, use + * {@link #getContainerWithActivity(IBinder) } otherwise. + * + * Returns a container that this activity is registered with. The activity could be created + * before the container appeared, adding the activity to the container if so. + */ + @Nullable + private TaskFragmentContainer getContainerWithActivity(@NonNull IBinder activityToken, + Activity activityToAdd) { + final IBinder taskFragmentToken = ActivityThread.currentActivityThread().getActivityClient( + activityToken).mTaskFragmentToken; for (TaskFragmentContainer container : mContainers) { if (container.hasActivity(activityToken)) { return container; + } else if (container.getTaskFragmentToken().equals(taskFragmentToken)) { + if (activityToAdd != null) { + container.addPendingAppearedActivity(activityToAdd); + } + return container; } } diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/organizer/TaskFragmentContainer.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/organizer/TaskFragmentContainer.java index 368adeffce316..f47dc245d2b07 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/organizer/TaskFragmentContainer.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/organizer/TaskFragmentContainer.java @@ -45,10 +45,10 @@ class TaskFragmentContainer { private TaskFragmentInfo mInfo; /** - * Activity that is being reparented to this container, but haven't been added to {@link #mInfo} - * yet. + * Activities that are being reparented or being started to this container, but haven't been + * added to {@link #mInfo} yet. */ - private Activity mReparentingActivity; + private final ArrayList mPendingAppearedActivities = new ArrayList<>(); /** Containers that are dependent on this one and should be completely destroyed on exit. */ private final List mContainersToFinishOnExit = @@ -71,7 +71,9 @@ class TaskFragmentContainer { */ TaskFragmentContainer(@Nullable Activity activity) { mToken = new Binder("TaskFragmentContainer"); - mReparentingActivity = activity; + if (activity != null) { + addPendingAppearedActivity(activity); + } } /** @@ -89,8 +91,8 @@ class TaskFragmentContainer { // fragment info update with it placed in this container. We still want to apply rules // in this intermediate state. List allActivities = new ArrayList<>(); - if (mReparentingActivity != null) { - allActivities.add(mReparentingActivity); + if (!mPendingAppearedActivities.isEmpty()) { + allActivities.addAll(mPendingAppearedActivities); } // Add activities reported from the server. if (mInfo == null) { @@ -106,12 +108,20 @@ class TaskFragmentContainer { return allActivities; } + void addPendingAppearedActivity(@NonNull Activity pendingAppearedActivity) { + mPendingAppearedActivities.add(pendingAppearedActivity); + } + boolean hasActivity(@NonNull IBinder token) { if (mInfo != null && mInfo.getActivities().contains(token)) { return true; } - return mReparentingActivity != null - && mReparentingActivity.getActivityToken().equals(token); + for (Activity activity : mPendingAppearedActivities) { + if (activity.getActivityToken().equals(token)) { + return true; + } + } + return false; } @Nullable @@ -121,14 +131,15 @@ class TaskFragmentContainer { void setInfo(@Nullable TaskFragmentInfo info) { mInfo = info; - if (mInfo == null || mReparentingActivity == null) { + if (mInfo == null || mPendingAppearedActivities.isEmpty()) { return; } // Cleanup activities that were being re-parented - for (IBinder activityToken : mInfo.getActivities()) { - if (mReparentingActivity.getActivityToken().equals(activityToken)) { - mReparentingActivity = null; - break; + List infoActivities = mInfo.getActivities(); + for (int i = mPendingAppearedActivities.size() - 1; i >= 0; --i) { + final Activity activity = mPendingAppearedActivities.get(i); + if (infoActivities.contains(activity.getActivityToken())) { + mPendingAppearedActivities.remove(i); } } } @@ -147,7 +158,7 @@ class TaskFragmentContainer { } boolean isEmpty() { - return mReparentingActivity == null && (mInfo == null || mInfo.isEmpty()); + return mPendingAppearedActivities.isEmpty() && (mInfo == null || mInfo.isEmpty()); } /** @@ -196,10 +207,10 @@ class TaskFragmentContainer { mActivitiesToFinishOnExit.clear(); // Finish activities that were being re-parented to this container. - if (mReparentingActivity != null) { - mReparentingActivity.finish(); - mReparentingActivity = null; + for (Activity activity : mPendingAppearedActivities) { + activity.finish(); } + mPendingAppearedActivities.clear(); } boolean isFinished() { diff --git a/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java b/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java index cb9c0372c3ad2..fd64cf828f155 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java +++ b/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java @@ -841,6 +841,10 @@ public class ActivityTaskSupervisor implements RecentTasks.Callbacks { proc.getThread(), r.appToken); final boolean isTransitionForward = r.isTransitionForward(); + IBinder fragmentToken = null; + if (r.getTaskFragment().getTaskFragmentOrganizerPid() == r.getPid()) { + fragmentToken = r.getTaskFragment().getFragmentToken(); + } clientTransaction.addCallback(LaunchActivityItem.obtain(new Intent(r.intent), System.identityHashCode(r), r.info, // TODO: Have this take the merged configuration instead of separate global @@ -852,7 +856,7 @@ public class ActivityTaskSupervisor implements RecentTasks.Callbacks { r.takeOptions(), isTransitionForward, proc.createProfilerInfoIfNeeded(), r.assistToken, activityClientController, r.createFixedRotationAdjustmentsIfNeeded(), r.shareableActivityToken, - r.getLaunchedFromBubble())); + r.getLaunchedFromBubble(), fragmentToken)); // Set desired final state. final ActivityLifecycleItem lifecycleItem; diff --git a/services/core/java/com/android/server/wm/TaskFragment.java b/services/core/java/com/android/server/wm/TaskFragment.java index 999df307cbf05..d7a6a2c624eb1 100644 --- a/services/core/java/com/android/server/wm/TaskFragment.java +++ b/services/core/java/com/android/server/wm/TaskFragment.java @@ -1902,6 +1902,10 @@ class TaskFragment extends WindowContainer { } } + int getTaskFragmentOrganizerPid() { + return mTaskFragmentOrganizerPid; + } + /** * Returns a {@link TaskFragmentInfo} with information from this TaskFragment. Should not be * called from {@link Task}.