diff options
6 files changed, 189 insertions, 52 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 4df2d7d82f6a..6f9a4ff8188a 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java @@ -965,10 +965,16 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen @VisibleForTesting @GuardedBy("mLock") void onActivityDestroyed(@NonNull Activity activity) { + if (!activity.isFinishing()) { + // onDestroyed is triggered without finishing. This happens when the activity is + // relaunched. In this case, we don't want to cleanup the record. + return; + } // Remove any pending appeared activity, as the server won't send finished activity to the // organizer. + final IBinder activityToken = activity.getActivityToken(); for (int i = mTaskContainers.size() - 1; i >= 0; i--) { - mTaskContainers.valueAt(i).onActivityDestroyed(activity); + mTaskContainers.valueAt(i).onActivityDestroyed(activityToken); } // We didn't trigger the callback if there were any pending appeared activities, so check // again after the pending is removed. @@ -1170,16 +1176,33 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen * Returns a container that this activity is registered with. An activity can only belong to one * container, or no container at all. */ + @GuardedBy("mLock") @Nullable TaskFragmentContainer getContainerWithActivity(@NonNull Activity activity) { - final IBinder activityToken = activity.getActivityToken(); + return getContainerWithActivity(activity.getActivityToken()); + } + + @GuardedBy("mLock") + @Nullable + TaskFragmentContainer getContainerWithActivity(@NonNull IBinder activityToken) { + // Check pending appeared activity first because there can be a delay for the server + // update. + for (int i = mTaskContainers.size() - 1; i >= 0; i--) { + final List<TaskFragmentContainer> containers = mTaskContainers.valueAt(i).mContainers; + for (int j = containers.size() - 1; j >= 0; j--) { + final TaskFragmentContainer container = containers.get(j); + if (container.hasPendingAppearedActivity(activityToken)) { + return container; + } + } + } + + // Check appeared activity if there is no such pending appeared activity. for (int i = mTaskContainers.size() - 1; i >= 0; i--) { final List<TaskFragmentContainer> containers = mTaskContainers.valueAt(i).mContainers; - // Traverse from top to bottom in case an activity is added to top pending, and hasn't - // received update from server yet. for (int j = containers.size() - 1; j >= 0; j--) { final TaskFragmentContainer container = containers.get(j); - if (container.hasActivity(activityToken)) { + if (container.hasAppearedActivity(activityToken)) { return container; } } diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java index dba5a7a1cf3c..03f4dc9c1167 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java @@ -166,16 +166,16 @@ class TaskContainer { } /** Called when the activity is destroyed. */ - void onActivityDestroyed(@NonNull Activity activity) { + void onActivityDestroyed(@NonNull IBinder activityToken) { for (TaskFragmentContainer container : mContainers) { - container.onActivityDestroyed(activity); + container.onActivityDestroyed(activityToken); } } /** Removes the pending appeared activity from all TaskFragments in this Task. */ - void cleanupPendingAppearedActivity(@NonNull Activity pendingAppearedActivity) { + void cleanupPendingAppearedActivity(@NonNull IBinder activityToken) { for (TaskFragmentContainer container : mContainers) { - container.removePendingAppearedActivity(pendingAppearedActivity); + container.removePendingAppearedActivity(activityToken); } } 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 71b884018bdb..e31792ad718f 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java @@ -43,6 +43,9 @@ import java.util.List; * Client-side container for a stack of activities. Corresponds to an instance of TaskFragment * on the server side. */ +// Suppress GuardedBy warning because all the TaskFragmentContainers are stored in +// SplitController.mTaskContainers which is guarded. +@SuppressWarnings("GuardedBy") class TaskFragmentContainer { private static final int APPEAR_EMPTY_TIMEOUT_MS = 3000; @@ -66,11 +69,11 @@ class TaskFragmentContainer { TaskFragmentInfo mInfo; /** - * Activities that are being reparented or being started to this container, but haven't been - * added to {@link #mInfo} yet. + * Activity tokens that are being reparented or being started to this container, but haven't + * been added to {@link #mInfo} yet. */ @VisibleForTesting - final ArrayList<Activity> mPendingAppearedActivities = new ArrayList<>(); + final ArrayList<IBinder> mPendingAppearedActivities = new ArrayList<>(); /** * When this container is created for an {@link Intent} to start within, we store that Intent @@ -84,8 +87,11 @@ class TaskFragmentContainer { private final List<TaskFragmentContainer> mContainersToFinishOnExit = new ArrayList<>(); - /** Individual associated activities in different containers that should be finished on exit. */ - private final List<Activity> mActivitiesToFinishOnExit = new ArrayList<>(); + /** + * Individual associated activity tokens in different containers that should be finished on + * exit. + */ + private final List<IBinder> mActivitiesToFinishOnExit = new ArrayList<>(); /** Indicates whether the container was cleaned up after the last activity was removed. */ private boolean mIsFinished; @@ -158,8 +164,9 @@ class TaskFragmentContainer { // in this intermediate state. // Place those on top of the list since they will be on the top after reported from the // server. - for (Activity activity : mPendingAppearedActivities) { - if (!activity.isFinishing()) { + for (IBinder token : mPendingAppearedActivities) { + final Activity activity = mController.getActivity(token); + if (activity != null && !activity.isFinishing()) { allActivities.add(activity); } } @@ -203,55 +210,58 @@ class TaskFragmentContainer { /** Adds the activity that will be reparented to this container. */ void addPendingAppearedActivity(@NonNull Activity pendingAppearedActivity) { - if (hasActivity(pendingAppearedActivity.getActivityToken())) { + final IBinder activityToken = pendingAppearedActivity.getActivityToken(); + if (hasActivity(activityToken)) { return; } - // Remove the pending activity from other TaskFragments. - mTaskContainer.cleanupPendingAppearedActivity(pendingAppearedActivity); - mPendingAppearedActivities.add(pendingAppearedActivity); - updateActivityClientRecordTaskFragmentToken(pendingAppearedActivity); + // Remove the pending activity from other TaskFragments in case the activity is reparented + // again before the server update. + mTaskContainer.cleanupPendingAppearedActivity(activityToken); + mPendingAppearedActivities.add(activityToken); + updateActivityClientRecordTaskFragmentToken(activityToken); } /** * Updates the {@link ActivityThread.ActivityClientRecord#mTaskFragmentToken} for the * activity. This makes sure the token is up-to-date if the activity is relaunched later. */ - private void updateActivityClientRecordTaskFragmentToken(@NonNull Activity activity) { + private void updateActivityClientRecordTaskFragmentToken(@NonNull IBinder activityToken) { final ActivityThread.ActivityClientRecord record = ActivityThread - .currentActivityThread().getActivityClient(activity.getActivityToken()); + .currentActivityThread().getActivityClient(activityToken); if (record != null) { record.mTaskFragmentToken = mToken; } } - void removePendingAppearedActivity(@NonNull Activity pendingAppearedActivity) { - mPendingAppearedActivities.remove(pendingAppearedActivity); + void removePendingAppearedActivity(@NonNull IBinder activityToken) { + mPendingAppearedActivities.remove(activityToken); } void clearPendingAppearedActivities() { - final List<Activity> cleanupActivities = new ArrayList<>(mPendingAppearedActivities); + final List<IBinder> cleanupActivities = new ArrayList<>(mPendingAppearedActivities); // Clear mPendingAppearedActivities so that #getContainerWithActivity won't return the // current TaskFragment. mPendingAppearedActivities.clear(); mPendingAppearedIntent = null; // For removed pending activities, we need to update the them to their previous containers. - for (Activity activity : cleanupActivities) { + for (IBinder activityToken : cleanupActivities) { final TaskFragmentContainer curContainer = mController.getContainerWithActivity( - activity); + activityToken); if (curContainer != null) { - curContainer.updateActivityClientRecordTaskFragmentToken(activity); + curContainer.updateActivityClientRecordTaskFragmentToken(activityToken); } } } /** Called when the activity is destroyed. */ - void onActivityDestroyed(@NonNull Activity activity) { - removePendingAppearedActivity(activity); + void onActivityDestroyed(@NonNull IBinder activityToken) { + removePendingAppearedActivity(activityToken); if (mInfo != null) { // Remove the activity now because there can be a delay before the server callback. - mInfo.getActivities().remove(activity.getActivityToken()); + mInfo.getActivities().remove(activityToken); } + mActivitiesToFinishOnExit.remove(activityToken); } @Nullable @@ -275,16 +285,24 @@ class TaskFragmentContainer { mPendingAppearedIntent = null; } - boolean hasActivity(@NonNull IBinder token) { - if (mInfo != null && mInfo.getActivities().contains(token)) { - return true; - } - for (Activity activity : mPendingAppearedActivities) { - if (activity.getActivityToken().equals(token)) { - return true; - } - } - return false; + boolean hasActivity(@NonNull IBinder activityToken) { + // Instead of using (hasAppearedActivity() || hasPendingAppearedActivity), we want to make + // sure the controller considers this container as the one containing the activity. + // This is needed when the activity is added as pending appeared activity to one + // TaskFragment while it is also an appeared activity in another. + return mController.getContainerWithActivity(activityToken) == this; + } + + /** Whether this activity has appeared in the TaskFragment on the server side. */ + boolean hasAppearedActivity(@NonNull IBinder activityToken) { + return mInfo != null && mInfo.getActivities().contains(activityToken); + } + + /** + * Whether we are waiting for this activity to appear in the TaskFragment on the server side. + */ + boolean hasPendingAppearedActivity(@NonNull IBinder activityToken) { + return mPendingAppearedActivities.contains(activityToken); } int getRunningActivityCount() { @@ -342,8 +360,8 @@ class TaskFragmentContainer { // Cleanup activities that were being re-parented List<IBinder> infoActivities = mInfo.getActivities(); for (int i = mPendingAppearedActivities.size() - 1; i >= 0; --i) { - final Activity activity = mPendingAppearedActivities.get(i); - if (infoActivities.contains(activity.getActivityToken())) { + final IBinder activityToken = mPendingAppearedActivities.get(i); + if (infoActivities.contains(activityToken)) { mPendingAppearedActivities.remove(i); } } @@ -392,7 +410,7 @@ class TaskFragmentContainer { if (mIsFinished) { return; } - mActivitiesToFinishOnExit.add(activityToFinish); + mActivitiesToFinishOnExit.add(activityToFinish.getActivityToken()); } /** @@ -402,7 +420,7 @@ class TaskFragmentContainer { if (mIsFinished) { return; } - mActivitiesToFinishOnExit.remove(activityToRemove); + mActivitiesToFinishOnExit.remove(activityToRemove.getActivityToken()); } /** Removes all dependencies that should be finished when this container is finished. */ @@ -470,8 +488,9 @@ class TaskFragmentContainer { mContainersToFinishOnExit.clear(); // Finish associated activities - for (Activity activity : mActivitiesToFinishOnExit) { - if (activity.isFinishing() + for (IBinder activityToken : mActivitiesToFinishOnExit) { + final Activity activity = mController.getActivity(activityToken); + if (activity == null || activity.isFinishing() || controller.shouldRetainAssociatedActivity(this, activity)) { continue; } @@ -540,7 +559,8 @@ class TaskFragmentContainer { } int maxMinWidth = mInfo.getMinimumWidth(); int maxMinHeight = mInfo.getMinimumHeight(); - for (Activity activity : mPendingAppearedActivities) { + for (IBinder activityToken : mPendingAppearedActivities) { + final Activity activity = mController.getActivity(activityToken); final Size minDimensions = SplitPresenter.getMinDimensions(activity); if (minDimensions == null) { continue; diff --git a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/EmbeddingTestUtils.java b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/EmbeddingTestUtils.java index 92011af27619..bc03e4ec303d 100644 --- a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/EmbeddingTestUtils.java +++ b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/EmbeddingTestUtils.java @@ -21,6 +21,7 @@ import static android.view.Display.DEFAULT_DISPLAY; import static androidx.window.extensions.embedding.SplitRule.FINISH_ALWAYS; import static androidx.window.extensions.embedding.SplitRule.FINISH_NEVER; +import static org.junit.Assert.assertFalse; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -45,6 +46,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +// Suppress GuardedBy warning on unit tests +@SuppressWarnings("GuardedBy") public class EmbeddingTestUtils { static final Rect TASK_BOUNDS = new Rect(0, 0, 600, 1200); static final int TASK_ID = 10; @@ -191,6 +194,15 @@ public class EmbeddingTestUtils { return new TaskContainer(TASK_ID, activity); } + static TaskContainer createTestTaskContainer(@NonNull SplitController controller) { + final TaskContainer taskContainer = createTestTaskContainer(); + final int taskId = taskContainer.getTaskId(); + // Should not call to create TaskContainer with the same task id twice. + assertFalse(controller.mTaskContainers.contains(taskId)); + controller.mTaskContainers.put(taskId, taskContainer); + return taskContainer; + } + static WindowLayoutInfo createWindowLayoutInfo() { final FoldingFeature foldingFeature = new FoldingFeature( new Rect( diff --git a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java index 8c1b87a650e0..221c7640ef0b 100644 --- a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java +++ b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java @@ -242,6 +242,14 @@ public class SplitControllerTest { assertTrue(tf.hasActivity(mActivity.getActivityToken())); + // When the activity is not finishing, do not clear the record. + doReturn(false).when(mActivity).isFinishing(); + mSplitController.onActivityDestroyed(mActivity); + + assertTrue(tf.hasActivity(mActivity.getActivityToken())); + + // Clear the record when the activity is finishing and destroyed. + doReturn(true).when(mActivity).isFinishing(); mSplitController.onActivityDestroyed(mActivity); assertFalse(tf.hasActivity(mActivity.getActivityToken())); diff --git a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java index d43c471fb8ae..99f56b4ceb17 100644 --- a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java +++ b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java @@ -161,7 +161,8 @@ public class TaskFragmentContainerTest { final TaskFragmentContainer pendingActivityContainer = new TaskFragmentContainer(mActivity, null /* pendingAppearedIntent */, taskContainer, mController); - assertTrue(pendingActivityContainer.mPendingAppearedActivities.contains(mActivity)); + assertTrue(pendingActivityContainer.mPendingAppearedActivities.contains( + mActivity.getActivityToken())); final TaskFragmentInfo info0 = createMockTaskFragmentInfo(pendingActivityContainer, mActivity); @@ -317,7 +318,7 @@ public class TaskFragmentContainerTest { @Test public void testOnActivityDestroyed() { - final TaskContainer taskContainer = createTestTaskContainer(); + final TaskContainer taskContainer = createTestTaskContainer(mController); final TaskFragmentContainer container = new TaskFragmentContainer(null /* activity */, mIntent, taskContainer, mController); container.addPendingAppearedActivity(mActivity); @@ -328,7 +329,7 @@ public class TaskFragmentContainerTest { assertTrue(container.hasActivity(mActivity.getActivityToken())); - taskContainer.onActivityDestroyed(mActivity); + taskContainer.onActivityDestroyed(mActivity.getActivityToken()); // It should not contain the destroyed Activity. assertFalse(container.hasActivity(mActivity.getActivityToken())); @@ -398,6 +399,79 @@ public class TaskFragmentContainerTest { assertFalse(taskContainer.isInIntermediateState()); } + @Test + public void testHasAppearedActivity() { + final TaskContainer taskContainer = createTestTaskContainer(); + final TaskFragmentContainer container = new TaskFragmentContainer(null /* activity */, + mIntent, taskContainer, mController); + container.addPendingAppearedActivity(mActivity); + + assertFalse(container.hasAppearedActivity(mActivity.getActivityToken())); + + final List<IBinder> activities = new ArrayList<>(); + activities.add(mActivity.getActivityToken()); + doReturn(activities).when(mInfo).getActivities(); + container.setInfo(mTransaction, mInfo); + + assertTrue(container.hasAppearedActivity(mActivity.getActivityToken())); + } + + @Test + public void testHasPendingAppearedActivity() { + final TaskContainer taskContainer = createTestTaskContainer(); + final TaskFragmentContainer container = new TaskFragmentContainer(null /* activity */, + mIntent, taskContainer, mController); + container.addPendingAppearedActivity(mActivity); + + assertTrue(container.hasPendingAppearedActivity(mActivity.getActivityToken())); + + final List<IBinder> activities = new ArrayList<>(); + activities.add(mActivity.getActivityToken()); + doReturn(activities).when(mInfo).getActivities(); + container.setInfo(mTransaction, mInfo); + + assertFalse(container.hasPendingAppearedActivity(mActivity.getActivityToken())); + } + + @Test + public void testHasActivity() { + final TaskContainer taskContainer = createTestTaskContainer(mController); + final TaskFragmentContainer container1 = new TaskFragmentContainer(null /* activity */, + mIntent, taskContainer, mController); + final TaskFragmentContainer container2 = new TaskFragmentContainer(null /* activity */, + mIntent, taskContainer, mController); + + // Activity is pending appeared on container2. + container2.addPendingAppearedActivity(mActivity); + + assertFalse(container1.hasActivity(mActivity.getActivityToken())); + assertTrue(container2.hasActivity(mActivity.getActivityToken())); + + // Activity is pending appeared on container1 (removed from container2). + container1.addPendingAppearedActivity(mActivity); + + assertTrue(container1.hasActivity(mActivity.getActivityToken())); + assertFalse(container2.hasActivity(mActivity.getActivityToken())); + + final List<IBinder> activities = new ArrayList<>(); + activities.add(mActivity.getActivityToken()); + doReturn(activities).when(mInfo).getActivities(); + + // Although Activity is appeared on container2, we prioritize pending appeared record on + // container1. + container2.setInfo(mTransaction, mInfo); + + assertTrue(container1.hasActivity(mActivity.getActivityToken())); + assertFalse(container2.hasActivity(mActivity.getActivityToken())); + + // When the pending appeared record is removed from container1, we respect the appeared + // record in container2. + container1.removePendingAppearedActivity(mActivity.getActivityToken()); + + assertFalse(container1.hasActivity(mActivity.getActivityToken())); + assertTrue(container2.hasActivity(mActivity.getActivityToken())); + } + /** Creates a mock activity in the organizer process. */ private Activity createMockActivity() { final Activity activity = mock(Activity.class); |