From 026d59513162f10ab33cbf345d06bb2e675d6520 Mon Sep 17 00:00:00 2001 From: Evan Rosky Date: Wed, 31 Aug 2022 15:07:24 -0700 Subject: [PATCH] Don't abort transition for activity re-ordering. ActivityStarter will reorder activities and change their visibilities but still report DELIVERED_TO_TOP. This means we need to do some secondary checks for whether the top changed. This adds some additional tracking so that we don't abort the transition if activity visibility does change. Bug: 243812111 Test: wmtest ActivityStarterTests Change-Id: I14e42b55cc95b9331d1dbc1afb5da1457acedf9d --- .../android/server/wm/ActivityStarter.java | 21 +++++++++-- .../core/java/com/android/server/wm/Task.java | 23 ++++++++---- .../android/server/wm/WindowContainer.java | 5 +++ .../server/wm/ActivityStarterTests.java | 35 +++++++++++++++++++ .../src/com/android/server/wm/TaskTests.java | 5 +-- 5 files changed, 77 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java index fc3320e76c2af..7e6ea941d66c1 100644 --- a/services/core/java/com/android/server/wm/ActivityStarter.java +++ b/services/core/java/com/android/server/wm/ActivityStarter.java @@ -203,6 +203,10 @@ class ActivityStarter { private TaskFragment mAddingToTaskFragment; @VisibleForTesting boolean mAddingToTask; + // Activity that was moved to the top of its task in situations where activity-order changes + // due to launch flags (eg. REORDER_TO_TOP). + @VisibleForTesting + ActivityRecord mMovedToTopActivity; private ActivityInfo mNewTaskInfo; private Intent mNewTaskIntent; @@ -1763,7 +1767,9 @@ class ActivityStarter { // The activity is started new rather than just brought forward, so record it as an // existence change. transitionController.collectExistenceChange(started); - } else if (result == START_DELIVERED_TO_TOP && newTransition != null) { + } else if (result == START_DELIVERED_TO_TOP && newTransition != null + // An activity has changed order/visibility so this isn't just deliver-to-top + && mMovedToTopActivity == null) { // We just delivered to top, so there isn't an actual transition here. if (!forceTransientTransition) { newTransition.abort(); @@ -2343,10 +2349,15 @@ class ActivityStarter { // In this situation we want to remove all activities from the task up to the one // being started. In most cases this means we are resetting the task to its initial // state. + int[] finishCount = new int[1]; final ActivityRecord clearTop = targetTask.performClearTop(mStartActivity, - mLaunchFlags); + mLaunchFlags, finishCount); if (clearTop != null && !clearTop.finishing) { + if (finishCount[0] > 0) { + // Only record if actually moved to top. + mMovedToTopActivity = clearTop; + } if (clearTop.isRootOfTask()) { // Activity aliases may mean we use different intents for the top activity, // so make sure the task now has the identity of the new intent. @@ -2383,7 +2394,11 @@ class ActivityStarter { mStartActivity.mUserId); if (act != null) { final Task task = act.getTask(); - task.moveActivityToFrontLocked(act); + boolean actuallyMoved = task.moveActivityToFrontLocked(act); + if (actuallyMoved) { + // Only record if the activity actually moved. + mMovedToTopActivity = act; + } act.updateOptionsLocked(mOptions); deliverNewIntent(act, intentGrants); act.getTaskFragment().clearLastPausedActivity(); diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java index 9186eb2abea66..436cc4d4cbe9c 100644 --- a/services/core/java/com/android/server/wm/Task.java +++ b/services/core/java/com/android/server/wm/Task.java @@ -1399,13 +1399,15 @@ class Task extends TaskFragment { /** * Reorder the history task so that the passed activity is brought to the front. + * @return whether it was actually moved (vs already being top). */ - final void moveActivityToFrontLocked(ActivityRecord newTop) { + final boolean moveActivityToFrontLocked(ActivityRecord newTop) { ProtoLog.i(WM_DEBUG_ADD_REMOVE, "Removing and adding activity %s to root task at top " + "callers=%s", newTop, Debug.getCallers(4)); - + int origDist = getDistanceFromTop(newTop); positionChildAtTop(newTop); updateEffectiveIntent(); + return getDistanceFromTop(newTop) != origDist; } @Override @@ -1613,14 +1615,14 @@ class Task extends TaskFragment { } } - ActivityRecord performClearTop(ActivityRecord newR, int launchFlags) { + ActivityRecord performClearTop(ActivityRecord newR, int launchFlags, int[] finishCount) { // The task should be preserved for putting new activity in case the last activity is // finished if it is normal launch mode and not single top ("clear-task-top"). mReuseTask = true; mTaskSupervisor.beginDeferResume(); final ActivityRecord result; try { - result = clearTopActivities(newR, launchFlags); + result = clearTopActivities(newR, launchFlags, finishCount); } finally { mTaskSupervisor.endDeferResume(); mReuseTask = false; @@ -1636,14 +1638,19 @@ class Task extends TaskFragment { * activities on top of it and return the instance. * * @param newR Description of the new activity being started. + * @param finishCount 1-element array that will be populated with the number of activities + * that have been finished. * @return Returns the existing activity in the task that performs the clear-top operation, * or {@code null} if none was found. */ - private ActivityRecord clearTopActivities(ActivityRecord newR, int launchFlags) { + private ActivityRecord clearTopActivities(ActivityRecord newR, int launchFlags, + int[] finishCount) { final ActivityRecord r = findActivityInHistory(newR.mActivityComponent, newR.mUserId); if (r == null) return null; - final PooledPredicate f = PooledLambda.obtainPredicate(Task::finishActivityAbove, + final PooledPredicate f = PooledLambda.obtainPredicate( + (ActivityRecord ar, ActivityRecord boundaryActivity) -> + finishActivityAbove(ar, boundaryActivity, finishCount), PooledLambda.__(ActivityRecord.class), r); forAllActivities(f); f.recycle(); @@ -1661,7 +1668,8 @@ class Task extends TaskFragment { return r; } - private static boolean finishActivityAbove(ActivityRecord r, ActivityRecord boundaryActivity) { + private static boolean finishActivityAbove(ActivityRecord r, ActivityRecord boundaryActivity, + @NonNull int[] finishCount) { // Stop operation once we reach the boundary activity. if (r == boundaryActivity) return true; @@ -1672,6 +1680,7 @@ class Task extends TaskFragment { // TODO: Why is this updating the boundary activity vs. the current activity??? boundaryActivity.updateOptionsLocked(opts); } + finishCount[0] += 1; r.finishIfPossible("clear-task-stack", false /* oomAdj */); } diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java index 92e52de2c01f9..7a84b3ec6723e 100644 --- a/services/core/java/com/android/server/wm/WindowContainer.java +++ b/services/core/java/com/android/server/wm/WindowContainer.java @@ -1834,6 +1834,11 @@ class WindowContainer extends ConfigurationContainer< return null; } + int getDistanceFromTop(WindowContainer child) { + int idx = mChildren.indexOf(child); + return idx < 0 ? -1 : mChildren.size() - 1 - idx; + } + private ActivityRecord processGetActivityWithBoundary(Predicate callback, WindowContainer boundary, boolean includeBoundary, boolean traverseTopToBottom, boolean[] boundaryFound, WindowContainer wc) { diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java index c78bc59612d45..00be7ed5bc6c2 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java @@ -35,6 +35,7 @@ import static android.app.WindowConfiguration.WINDOWING_MODE_PINNED; import static android.content.Intent.FLAG_ACTIVITY_CLEAR_TASK; import static android.content.Intent.FLAG_ACTIVITY_LAUNCH_ADJACENT; import static android.content.Intent.FLAG_ACTIVITY_NEW_TASK; +import static android.content.Intent.FLAG_ACTIVITY_REORDER_TO_FRONT; import static android.content.Intent.FLAG_ACTIVITY_RESET_TASK_IF_NEEDED; import static android.content.Intent.FLAG_ACTIVITY_SINGLE_TOP; import static android.content.pm.ActivityInfo.FLAG_ALLOW_UNTRUSTED_ACTIVITY_EMBEDDING; @@ -66,6 +67,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; @@ -1402,6 +1404,39 @@ public class ActivityStarterTests extends WindowTestsBase { canEmbedActivity(taskFragment, starting, task)); } + @Test + public void testRecordActivityMovementBeforeDeliverToTop() { + final Task task = new TaskBuilder(mAtm.mTaskSupervisor).build(); + final ActivityRecord activityBot = new ActivityBuilder(mAtm).setTask(task).build(); + final ActivityRecord activityTop = new ActivityBuilder(mAtm).setTask(task).build(); + + activityBot.setVisible(false); + activityBot.mVisibleRequested = false; + + assertTrue(activityTop.isVisible()); + assertTrue(activityTop.mVisibleRequested); + + final ActivityStarter starter = prepareStarter(FLAG_ACTIVITY_REORDER_TO_FRONT + | FLAG_ACTIVITY_NEW_TASK, false /* mockGetRootTask */); + starter.mStartActivity = activityBot; + task.inRecents = true; + starter.setInTask(task); + starter.getIntent().setComponent(activityBot.mActivityComponent); + final int result = starter.setReason("testRecordActivityMovement").execute(); + + assertEquals(START_DELIVERED_TO_TOP, result); + assertNotNull(starter.mMovedToTopActivity); + + final ActivityStarter starter2 = prepareStarter(FLAG_ACTIVITY_REORDER_TO_FRONT + | FLAG_ACTIVITY_NEW_TASK, false /* mockGetRootTask */); + starter2.setInTask(task); + starter2.getIntent().setComponent(activityBot.mActivityComponent); + final int result2 = starter2.setReason("testRecordActivityMovement").execute(); + + assertEquals(START_DELIVERED_TO_TOP, result2); + assertNull(starter2.mMovedToTopActivity); + } + private static void startActivityInner(ActivityStarter starter, ActivityRecord target, ActivityRecord source, ActivityOptions options, Task inTask, TaskFragment inTaskFragment) { diff --git a/services/tests/wmtests/src/com/android/server/wm/TaskTests.java b/services/tests/wmtests/src/com/android/server/wm/TaskTests.java index f3227799f9133..9480ae81aede2 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TaskTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/TaskTests.java @@ -263,7 +263,8 @@ public class TaskTests extends WindowTestsBase { // Detach from process so the activities can be removed from hierarchy when finishing. activity1.detachFromProcess(); activity2.detachFromProcess(); - assertTrue(task.performClearTop(activity1, 0 /* launchFlags */).finishing); + int[] finishCount = new int[1]; + assertTrue(task.performClearTop(activity1, 0 /* launchFlags */, finishCount).finishing); assertFalse(task.hasChild()); // In real case, the task should be preserved for adding new activity. assertTrue(task.isAttached()); @@ -277,7 +278,7 @@ public class TaskTests extends WindowTestsBase { doReturn(true).when(activityB).shouldBeVisibleUnchecked(); doReturn(true).when(activityC).shouldBeVisibleUnchecked(); activityA.getConfiguration().densityDpi += 100; - assertTrue(task.performClearTop(activityA, 0 /* launchFlags */).finishing); + assertTrue(task.performClearTop(activityA, 0 /* launchFlags */, finishCount).finishing); // The bottom activity should destroy directly without relaunch for config change. assertEquals(ActivityRecord.State.DESTROYING, activityA.getState()); verify(activityA, never()).startRelaunching();