From d6cf8c85ac14a2e6bcc797e31a39a0cdab030a85 Mon Sep 17 00:00:00 2001 From: Shivam Agrawal Date: Wed, 27 Oct 2021 16:12:00 -0700 Subject: [PATCH 1/4] Update toString methods for various Embedding... ...Reference Implementation classes Bug: b/204193051 Test: manual Change-Id: I79e94feb002dadc2219ccc7604843a3c61b91057 --- .../java/android/window/TaskFragmentInfo.java | 1 + .../extensions/embedding/SplitContainer.java | 9 +++++ .../embedding/TaskFragmentContainer.java | 39 +++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/core/java/android/window/TaskFragmentInfo.java b/core/java/android/window/TaskFragmentInfo.java index 165dcdf3a8368..a118f9a8188f8 100644 --- a/core/java/android/window/TaskFragmentInfo.java +++ b/core/java/android/window/TaskFragmentInfo.java @@ -213,6 +213,7 @@ public final class TaskFragmentInfo implements Parcelable { + " isEmpty=" + mIsEmpty + " runningActivityCount=" + mRunningActivityCount + " isVisible=" + mIsVisible + + " activities=" + mActivities + " positionInParent=" + mPositionInParent + " isTaskClearedForReuse=" + mIsTaskClearedForReuse + "}"; diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitContainer.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitContainer.java index 06e7d14574179..1e9fda6599d5a 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitContainer.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitContainer.java @@ -83,4 +83,13 @@ class SplitContainer { && ((SplitPairRule) splitRule).shouldFinishSecondaryWithPrimary(); return shouldFinishSecondaryWithPrimary || isPlaceholderContainer; } + + @Override + public String toString() { + return "SplitContainer{" + + " primaryContainer=" + mPrimaryContainer + + " secondaryContainer=" + mSecondaryContainer + + " splitRule=" + mSplitRule + + "}"; + } } diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java index 80d9c2c1719ce..6805fde685b09 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java @@ -27,6 +27,7 @@ import android.window.TaskFragmentInfo; import android.window.WindowContainerTransaction; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; /** @@ -267,4 +268,42 @@ class TaskFragmentContainer { mLastRequestedBounds.set(bounds); } } + + @Override + public String toString() { + return toString(true /* includeContainersToFinishOnExit */); + } + + /** + * @return string for this TaskFragmentContainer and includes containers to finish on exit + * based on {@code includeContainersToFinishOnExit}. If containers to finish on exit are always + * included in the string, then calling {@link #toString()} on a container that mutually + * finishes with another container would cause a stack overflow. + */ + private String toString(boolean includeContainersToFinishOnExit) { + return "TaskFragmentContainer{" + + " token=" + mToken + + " info=" + mInfo + + " topNonFinishingActivity=" + getTopNonFinishingActivity() + + " pendingAppearedActivities=" + mPendingAppearedActivities + + (includeContainersToFinishOnExit ? " containersToFinishOnExit=" + + containersToFinishOnExitToString() : "") + + " activitiesToFinishOnExit=" + mActivitiesToFinishOnExit + + " isFinished=" + mIsFinished + + " lastRequestedBounds=" + mLastRequestedBounds + + "}"; + } + + private String containersToFinishOnExitToString() { + StringBuilder sb = new StringBuilder("["); + Iterator containerIterator = mContainersToFinishOnExit.iterator(); + while (containerIterator.hasNext()) { + sb.append(containerIterator.next().toString( + false /* includeContainersToFinishOnExit */)); + if (containerIterator.hasNext()) { + sb.append(", "); + } + } + return sb.append("]").toString(); + } } From 9c906fbc942bbddba7fe3bc1c6e905281712a118 Mon Sep 17 00:00:00 2001 From: Shivam Agrawal Date: Wed, 27 Oct 2021 16:20:14 -0700 Subject: [PATCH 2/4] Include Activities That Have Not Been Assigned... ...to a Process Yet in TaskFragmentInfo TaskFragmentInfo might be reported before an activity has been assigned to a process, which would prevent the activity from being reported in the TaskFragmentInfo activities array because the activity process id does not match the TaskFragmentOrganizer process id. However, the activity is still reported in the running activity count and makes the TaskFragment non-empty. This discrepency results in unnecessary split info callbacks because the change in TaskFragmentInfo#isEmpty or runningActivityCount triggers an onTaskFragmentInfoChanged callback in SplitController, which triggers a split info callback. However, this split info callback will soon be stale information because once the activity has been attached to a process, the activity lifecycle listener in SplitController will send a split info callback. This CL uses a combination of uid and process name to check that the activity belongs to the TaskFragmentOrganizer process instead of the process id. uid and process name are set during activity creation whereas pid is set afterwards, which causes the discrepency. Bug: b/204193051 b/205240942 b/204721225 b/204405410 Test: atest CtsWindowManagerJetpackTestCases:ActivityEmbeddingLaunchTests Change-Id: Icfa28325da0278d2fb70e1111ffd39412b370bdb --- .../com/android/server/wm/TaskFragment.java | 22 +++++++++---------- .../server/wm/WindowOrganizerController.java | 4 ++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/services/core/java/com/android/server/wm/TaskFragment.java b/services/core/java/com/android/server/wm/TaskFragment.java index f08ebba7839ed..44c7d09d8a537 100644 --- a/services/core/java/com/android/server/wm/TaskFragment.java +++ b/services/core/java/com/android/server/wm/TaskFragment.java @@ -31,6 +31,7 @@ import static android.content.pm.ActivityInfo.FLAG_RESUME_WHILE_PAUSING; import static android.content.res.Configuration.ORIENTATION_LANDSCAPE; import static android.content.res.Configuration.ORIENTATION_PORTRAIT; import static android.content.res.Configuration.ORIENTATION_UNDEFINED; +import static android.os.Process.INVALID_UID; import static android.os.UserHandle.USER_NULL; import static android.view.Display.INVALID_DISPLAY; import static android.view.WindowManager.TRANSIT_CLOSE; @@ -221,6 +222,8 @@ class TaskFragment extends WindowContainer { /** Organizer that organizing this TaskFragment. */ @Nullable private ITaskFragmentOrganizer mTaskFragmentOrganizer; + private int mTaskFragmentOrganizerUid = INVALID_UID; + private @Nullable String mTaskFragmentOrganizerProcessName; /** Client assigned unique token for this TaskFragment if this is created by an organizer. */ @Nullable @@ -233,13 +236,6 @@ class TaskFragment extends WindowContainer { */ private boolean mDelayLastActivityRemoval; - /** - * The PID of the organizer that created this TaskFragment. It should be the same as the PID - * of {@link android.window.TaskFragmentCreationParams#getOwnerToken()}. - * {@link ActivityRecord#INVALID_PID} if this is not an organizer-created TaskFragment. - */ - private int mTaskFragmentOrganizerPid = ActivityRecord.INVALID_PID; - final Point mLastSurfaceSize = new Point(); private final Rect mTmpInsets = new Rect(); @@ -338,9 +334,11 @@ class TaskFragment extends WindowContainer { mDelayLastActivityRemoval = false; } - void setTaskFragmentOrganizer(TaskFragmentOrganizerToken organizer, int pid) { + void setTaskFragmentOrganizer(@NonNull TaskFragmentOrganizerToken organizer, int uid, + @NonNull String processName) { mTaskFragmentOrganizer = ITaskFragmentOrganizer.Stub.asInterface(organizer.asBinder()); - mTaskFragmentOrganizerPid = pid; + mTaskFragmentOrganizerUid = uid; + mTaskFragmentOrganizerProcessName = processName; } /** Whether this TaskFragment is organized by the given {@code organizer}. */ @@ -2178,9 +2176,11 @@ class TaskFragment extends WindowContainer { List childActivities = new ArrayList<>(); for (int i = 0; i < getChildCount(); i++) { WindowContainer wc = getChildAt(i); - if (mTaskFragmentOrganizerPid != ActivityRecord.INVALID_PID + if (mTaskFragmentOrganizerUid != INVALID_UID && wc.asActivityRecord() != null - && wc.asActivityRecord().getPid() == mTaskFragmentOrganizerPid) { + && wc.asActivityRecord().info.processName.equals( + mTaskFragmentOrganizerProcessName) + && wc.asActivityRecord().getUid() == mTaskFragmentOrganizerUid) { // Only includes Activities that belong to the organizer process for security. childActivities.add(wc.asActivityRecord().appToken); } diff --git a/services/core/java/com/android/server/wm/WindowOrganizerController.java b/services/core/java/com/android/server/wm/WindowOrganizerController.java index 43a4f977e73aa..5dd01a5b46ec5 100644 --- a/services/core/java/com/android/server/wm/WindowOrganizerController.java +++ b/services/core/java/com/android/server/wm/WindowOrganizerController.java @@ -1205,8 +1205,8 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub creationParams.getFragmentToken(), true /* createdByOrganizer */); // Set task fragment organizer immediately, since it might have to be notified about further // actions. - taskFragment.setTaskFragmentOrganizer( - creationParams.getOrganizer(), ownerActivity.getPid()); + taskFragment.setTaskFragmentOrganizer(creationParams.getOrganizer(), + ownerActivity.getUid(), ownerActivity.info.processName); ownerActivity.getTask().addChild(taskFragment, POSITION_TOP); taskFragment.setWindowingMode(creationParams.getWindowingMode()); taskFragment.setBounds(creationParams.getInitialBounds()); From 5cb70ee9d46047cba342804d4f82936e4bbff516 Mon Sep 17 00:00:00 2001 From: Shivam Agrawal Date: Fri, 29 Oct 2021 11:36:49 -0700 Subject: [PATCH 3/4] Do not send split info update when TaskFragmentContainer ...in a split has no activities If a TaskFragmentContainer in a SplitContainer has no activities, then that means that either the entire split is going to be removed or the empty TaskFragmentContainer is about to get a running activity. This CL prevents a split info update from sent in this case because the info will soon be stale by another update. Bug: b/204193051 Test: atest CtsWindowManagerJetpackTestCases:ActivityEmbeddingLaunchTests Change-Id: I4b6f684b8d8e9a9b3fefd54b399d8a7d83c6cfb0 --- .../window/extensions/embedding/SplitController.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java index 20515e71a91ba..9014102d3f552 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java @@ -497,7 +497,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen return; } List currentSplitStates = getActiveSplitStates(); - if (mLastReportedSplitStates.equals(currentSplitStates)) { + if (currentSplitStates == null || mLastReportedSplitStates.equals(currentSplitStates)) { return; } mLastReportedSplitStates.clear(); @@ -506,15 +506,19 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen } /** - * Returns a list of descriptors for currently active split states. + * @return a list of descriptors for currently active split states. If the value returned is + * null, that indicates that the active split states are in an intermediate state and should + * not be reported. */ + @Nullable private List getActiveSplitStates() { List splitStates = new ArrayList<>(); for (SplitContainer container : mSplitContainers) { if (container.getPrimaryContainer().isEmpty() || container.getSecondaryContainer().isEmpty()) { - // Skipping containers that do not have any activities to report. - continue; + // We are in an intermediate state because either the split container is about to be + // removed or the primary or secondary container are about to receive an activity. + return null; } ActivityStack primaryContainer = container.getPrimaryContainer().toActivityStack(); ActivityStack secondaryContainer = container.getSecondaryContainer().toActivityStack(); From dbcffad428304eecf3847962af11b497969bd16a Mon Sep 17 00:00:00 2001 From: Shivam Agrawal Date: Tue, 2 Nov 2021 14:49:00 -0700 Subject: [PATCH 4/4] Adds Test that verifies an Activity is Still Reported... ...in the TaskFragmentInfo even if the Activity has not yet been assigned to a process. Bug: b/204193051 Test: atest WmTests:TaskFragmentTest#testActivityStillReported_NotYetAssignedToProcess Change-Id: I907e7e0a44b4d21740b68c47f15dad4602548f6e --- .../TaskFragmentOrganizerControllerTest.java | 18 ++++++++++----- .../android/server/wm/TaskFragmentTest.java | 23 +++++++++++++++++++ .../android/server/wm/WindowTestsBase.java | 6 ++++- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java index d475c46eed0c3..786c3eae63d0b 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java @@ -254,7 +254,8 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase { }); // Allow transaction to change a TaskFragment created by the organizer. - mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* pid */); + mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */, + "Test:TaskFragmentOrganizer" /* processName */); mAtm.getWindowOrganizerController().applyTransaction(mTransaction); } @@ -276,7 +277,8 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase { }); // Allow transaction to change a TaskFragment created by the organizer. - mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* pid */); + mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */, + "Test:TaskFragmentOrganizer" /* processName */); mAtm.getWindowOrganizerController().applyTransaction(mTransaction); } @@ -301,7 +303,8 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase { }); // Allow transaction to change a TaskFragment created by the organizer. - mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* pid */); + mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */, + "Test:TaskFragmentOrganizer" /* processName */); clearInvocations(mAtm.mRootWindowContainer); mAtm.getWindowOrganizerController().applyTransaction(mTransaction); @@ -337,8 +340,10 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase { }); // Allow transaction to change a TaskFragment created by the organizer. - mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* pid */); - taskFragment2.setTaskFragmentOrganizer(mOrganizerToken, 10 /* pid */); + mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */, + "Test:TaskFragmentOrganizer" /* processName */); + taskFragment2.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */, + "Test:TaskFragmentOrganizer" /* processName */); clearInvocations(mAtm.mRootWindowContainer); mAtm.getWindowOrganizerController().applyTransaction(mTransaction); @@ -391,7 +396,8 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase { }); // Allow transaction to change a TaskFragment created by the organizer. - mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* pid */); + mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */, + "Test:TaskFragmentOrganizer" /* processName */); clearInvocations(mAtm.mRootWindowContainer); mAtm.getWindowOrganizerController().applyTransaction(mTransaction); diff --git a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentTest.java b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentTest.java index cb209abf6aa9a..f1c4d835322ff 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentTest.java @@ -20,12 +20,15 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify; +import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.clearInvocations; import android.graphics.Rect; +import android.os.Binder; import android.platform.test.annotations.Presubmit; import android.view.SurfaceControl; import android.window.ITaskFragmentOrganizer; +import android.window.TaskFragmentInfo; import android.window.TaskFragmentOrganizer; import androidx.test.filters.MediumTest; @@ -64,6 +67,7 @@ public class TaskFragmentTest extends WindowTestsBase { mTaskFragment = new TaskFragmentBuilder(mAtm) .setCreateParentTask() .setOrganizer(mOrganizer) + .setFragmentToken(new Binder()) .build(); mLeash = mTaskFragment.getSurfaceControl(); spyOn(mTaskFragment); @@ -102,4 +106,23 @@ public class TaskFragmentTest extends WindowTestsBase { verify(mTransaction).setPosition(mLeash, 500, 500); verify(mTransaction).setWindowCrop(mLeash, 500, 500); } + + /** + * Tests that when a {@link TaskFragmentInfo} is generated from a {@link TaskFragment}, an + * activity that has not yet been attached to a process because it is being initialized but + * belongs to the TaskFragmentOrganizer process is still reported in the TaskFragmentInfo. + */ + @Test + public void testActivityStillReported_NotYetAssignedToProcess() { + mTaskFragment.addChild(new ActivityBuilder(mAtm).setUid(DEFAULT_TASK_FRAGMENT_ORGANIZER_UID) + .setProcessName(DEFAULT_TASK_FRAGMENT_ORGANIZER_PROCESS_NAME).build()); + final ActivityRecord activity = mTaskFragment.getTopMostActivity(); + // Remove the process to simulate an activity that has not yet been attached to a process + activity.app = null; + final TaskFragmentInfo info = activity.getTaskFragment().getTaskFragmentInfo(); + assertEquals(1, info.getRunningActivityCount()); + assertEquals(1, info.getActivities().size()); + assertEquals(false, info.isEmpty()); + assertEquals(activity.token, info.getActivities().get(0)); + } } diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java b/services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java index 115f8a3fecf43..40a1440e7f8e1 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java @@ -127,6 +127,9 @@ class WindowTestsBase extends SystemServiceTestsBase { // Default package name static final String DEFAULT_COMPONENT_PACKAGE_NAME = "com.foo"; + static final int DEFAULT_TASK_FRAGMENT_ORGANIZER_UID = 10000; + static final String DEFAULT_TASK_FRAGMENT_ORGANIZER_PROCESS_NAME = "Test:TaskFragmentOrganizer"; + // Default base activity name private static final String DEFAULT_COMPONENT_CLASS_NAME = ".BarActivity"; @@ -1227,7 +1230,8 @@ class WindowTestsBase extends SystemServiceTestsBase { } if (mOrganizer != null) { taskFragment.setTaskFragmentOrganizer( - mOrganizer.getOrganizerToken(), 10000 /* pid */); + mOrganizer.getOrganizerToken(), DEFAULT_TASK_FRAGMENT_ORGANIZER_UID, + DEFAULT_TASK_FRAGMENT_ORGANIZER_PROCESS_NAME); } return taskFragment; }