From 079221fdc83e5a1d7442b3e281f0e4ca44acf7fb Mon Sep 17 00:00:00 2001 From: Winson Chung Date: Wed, 13 Dec 2017 17:43:34 -0800 Subject: [PATCH] Fix regression in task trimming - This change does not affect the trimming of inactive or visible tasks. The previous logic was too aggressive in clearing tasks that were being moved in the recent task list (only in the case when a new task is being added). In particular if the two tasks share affinities, we should clean up the old task, but not clear it in replacing the task in the recents list. Test: com.android.server.am.RecentTasksTest Test: android.server.am.ActivityManagerDisplayTests Test: #testContentDestroyOnDisplayRemoved Bug: 70636181 Change-Id: If6275ff1d9d8f842ce1122e1229b46fae92c43b0 Signed-off-by: Winson Chung --- .../com/android/server/am/RecentTasks.java | 13 ++- .../android/server/am/RecentTasksTest.java | 80 ++++++++++++++++--- 2 files changed, 73 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/am/RecentTasks.java b/services/core/java/com/android/server/am/RecentTasks.java index abb296e9c8c33..2de84ab265ffd 100644 --- a/services/core/java/com/android/server/am/RecentTasks.java +++ b/services/core/java/com/android/server/am/RecentTasks.java @@ -662,7 +662,7 @@ class RecentTasks { * task to be trimmed as a result of that add. */ private boolean canAddTaskWithoutTrim(TaskRecord task) { - return findTrimIndexForAddTask(task) == -1; + return findRemoveIndexForAddTask(task) == -1; } /** @@ -896,7 +896,7 @@ class RecentTasks { } if (DEBUG_RECENTS) Slog.d(TAG_RECENTS, "addRecent: trimming tasks for " + task); - trimForAddTask(task); + removeForAddTask(task); task.inRecents = true; if (!isAffiliated || needAffiliationFix) { @@ -1175,8 +1175,8 @@ class RecentTasks { * If needed, remove oldest existing entries in recents that are for the same kind * of task as the given one. */ - private void trimForAddTask(TaskRecord task) { - final int removeIndex = findTrimIndexForAddTask(task); + private void removeForAddTask(TaskRecord task) { + final int removeIndex = findRemoveIndexForAddTask(task); if (removeIndex == -1) { // Nothing to trim return; @@ -1187,7 +1187,7 @@ class RecentTasks { // callbacks here. final TaskRecord removedTask = mTasks.remove(removeIndex); if (removedTask != task) { - notifyTaskRemoved(removedTask, TRIMMED); + notifyTaskRemoved(removedTask, !TRIMMED); if (DEBUG_RECENTS_TRIM_TASKS) Slog.d(TAG, "Trimming task=" + removedTask + " for addition of task=" + task); } @@ -1198,7 +1198,7 @@ class RecentTasks { * Find the task that would be removed if the given {@param task} is added to the recent tasks * list (if any). */ - private int findTrimIndexForAddTask(TaskRecord task) { + private int findRemoveIndexForAddTask(TaskRecord task) { int recentsCount = mTasks.size(); final Intent intent = task.intent; final boolean document = intent != null && intent.isDocument(); @@ -1241,7 +1241,6 @@ class RecentTasks { // don't need to trim it. continue; } else if (maxRecents > 0) { - // Otherwise only trim if we are over our max recents for this task --maxRecents; if (!sameIntent || multiTasksAllowed) { // We don't want to trim if we are not over the max allowed entries and diff --git a/services/tests/servicestests/src/com/android/server/am/RecentTasksTest.java b/services/tests/servicestests/src/com/android/server/am/RecentTasksTest.java index 0d03863a08049..f38404436e698 100644 --- a/services/tests/servicestests/src/com/android/server/am/RecentTasksTest.java +++ b/services/tests/servicestests/src/com/android/server/am/RecentTasksTest.java @@ -22,6 +22,7 @@ import static android.app.WindowConfiguration.ACTIVITY_TYPE_UNDEFINED; import static android.app.WindowConfiguration.WINDOWING_MODE_FULLSCREEN; import static android.app.WindowConfiguration.WINDOWING_MODE_UNDEFINED; import static android.content.Intent.FLAG_ACTIVITY_EXCLUDE_FROM_RECENTS; +import static android.content.Intent.FLAG_ACTIVITY_MULTIPLE_TASK; import static android.content.Intent.FLAG_ACTIVITY_NEW_DOCUMENT; import static android.view.Display.DEFAULT_DISPLAY; @@ -36,6 +37,7 @@ import static org.mockito.Mockito.spy; import static java.lang.Integer.MAX_VALUE; +import android.app.ActivityManager; import android.app.ActivityManager.RecentTaskInfo; import android.app.ActivityManager.RunningTaskInfo; import android.content.ComponentName; @@ -181,18 +183,6 @@ public class RecentTasksTest extends ActivityTestsBase { assertTrue(mCallbacksRecorder.removed.contains(mTasks.get(1))); mCallbacksRecorder.clear(); - // Add a task which will trigger the trimming of another - TaskRecord documentTask1 = createDocumentTask(".DocumentTask1"); - documentTask1.maxRecents = 1; - TaskRecord documentTask2 = createDocumentTask(".DocumentTask1"); - mRecentTasks.add(documentTask1); - mRecentTasks.add(documentTask2); - assertTrue(mCallbacksRecorder.added.contains(documentTask1)); - assertTrue(mCallbacksRecorder.added.contains(documentTask2)); - assertTrue(mCallbacksRecorder.trimmed.contains(documentTask1)); - assertTrue(mCallbacksRecorder.removed.contains(documentTask1)); - mCallbacksRecorder.clear(); - // Remove the callback, ensure we don't get any calls mRecentTasks.unregisterCallback(mCallbacksRecorder); mRecentTasks.add(mTasks.get(0)); @@ -202,6 +192,65 @@ public class RecentTasksTest extends ActivityTestsBase { assertTrue(mCallbacksRecorder.removed.isEmpty()); } + @Test + public void testAddTasksNoMultiple_expectNoTrim() throws Exception { + // Add same non-multiple-task document tasks will remove the task (to re-add it) but not + // trim it + TaskRecord documentTask1 = createDocumentTask(".DocumentTask1"); + TaskRecord documentTask2 = createDocumentTask(".DocumentTask1"); + mRecentTasks.add(documentTask1); + mRecentTasks.add(documentTask2); + assertTrue(mCallbacksRecorder.added.contains(documentTask1)); + assertTrue(mCallbacksRecorder.added.contains(documentTask2)); + assertTrue(mCallbacksRecorder.trimmed.isEmpty()); + assertTrue(mCallbacksRecorder.removed.contains(documentTask1)); + } + + @Test + public void testAddTasksMaxTaskRecents_expectNoTrim() throws Exception { + // Add a task hitting max-recents for that app will remove the task (to add the next one) + // but not trim it + TaskRecord documentTask1 = createDocumentTask(".DocumentTask1"); + TaskRecord documentTask2 = createDocumentTask(".DocumentTask1"); + documentTask1.maxRecents = 1; + documentTask2.maxRecents = 1; + mRecentTasks.add(documentTask1); + mRecentTasks.add(documentTask2); + assertTrue(mCallbacksRecorder.added.contains(documentTask1)); + assertTrue(mCallbacksRecorder.added.contains(documentTask2)); + assertTrue(mCallbacksRecorder.trimmed.isEmpty()); + assertTrue(mCallbacksRecorder.removed.contains(documentTask1)); + } + + @Test + public void testAddTasksSameTask_expectNoTrim() throws Exception { + // Add a task that is already in the task list does not trigger any callbacks, it just + // moves in the list + TaskRecord documentTask1 = createDocumentTask(".DocumentTask1"); + mRecentTasks.add(documentTask1); + mRecentTasks.add(documentTask1); + assertTrue(mCallbacksRecorder.added.size() == 1); + assertTrue(mCallbacksRecorder.added.contains(documentTask1)); + assertTrue(mCallbacksRecorder.trimmed.isEmpty()); + assertTrue(mCallbacksRecorder.removed.isEmpty()); + } + + @Test + public void testAddTasksMultipleTasks_expectNoTrim() throws Exception { + // Add same multiple-task document tasks does not trim the first tasks + TaskRecord documentTask1 = createDocumentTask(".DocumentTask1", + FLAG_ACTIVITY_MULTIPLE_TASK); + TaskRecord documentTask2 = createDocumentTask(".DocumentTask1", + FLAG_ACTIVITY_MULTIPLE_TASK); + mRecentTasks.add(documentTask1); + mRecentTasks.add(documentTask2); + assertTrue(mCallbacksRecorder.added.size() == 2); + assertTrue(mCallbacksRecorder.added.contains(documentTask1)); + assertTrue(mCallbacksRecorder.added.contains(documentTask2)); + assertTrue(mCallbacksRecorder.trimmed.isEmpty()); + assertTrue(mCallbacksRecorder.removed.isEmpty()); + } + @Test public void testUsersTasks() throws Exception { mRecentTasks.setOnlyTestVisibleRange(); @@ -549,10 +598,15 @@ public class RecentTasksTest extends ActivityTestsBase { } private TaskRecord createDocumentTask(String className) { + return createDocumentTask(className, 0); + } + + private TaskRecord createDocumentTask(String className, int flags) { TaskRecord task = createTaskBuilder(className) - .setFlags(FLAG_ACTIVITY_NEW_DOCUMENT) + .setFlags(FLAG_ACTIVITY_NEW_DOCUMENT | flags) .build(); task.affinity = null; + task.maxRecents = ActivityManager.getMaxAppRecentsLimitStatic(); return task; }