From 42a44d79840f6f7fe847f2fd23cd3d10c86ee724 Mon Sep 17 00:00:00 2001 From: Jeremy Sim Date: Fri, 15 Dec 2023 17:59:47 +0900 Subject: [PATCH] Fixes two bugs with findLastActiveTasksAndRunCallback() This CL changes findLastActiveTasksAndRunCallback to return a fixed array rather than an ArrayList. 1) This fixes a (minor) crash with app pairs, where we performed get(0) on an empty array when no running tasks were found. 2) This also fixes a live bug where app pairs would launch apps in the wrong order if app 1 was not found in running tasks, but app 2 was. The function should be more robust now, preserving the indexing of the input keys. Fixes: 316053131 Test: Clear all tasks from Overview, launch an app pair, launch succeeds Flag: ACONFIG com.android.wm.shell.enable_app_pairs DEVELOPMENT Change-Id: I51d8ba823a2ec57e9ecfeede956e0afce1d653f4 --- .../taskbar/TaskbarActivityContext.java | 2 +- .../taskbar/TaskbarUIController.java | 4 +-- .../uioverrides/QuickstepLauncher.java | 2 +- .../quickstep/util/AppPairsController.java | 4 +-- .../util/SplitSelectStateController.java | 25 +++++++++++-------- .../util/SplitSelectStateControllerTest.kt | 20 +++++++-------- 6 files changed, 30 insertions(+), 27 deletions(-) diff --git a/quickstep/src/com/android/launcher3/taskbar/TaskbarActivityContext.java b/quickstep/src/com/android/launcher3/taskbar/TaskbarActivityContext.java index 38ee4ac9ba..8d2c0f6be6 100644 --- a/quickstep/src/com/android/launcher3/taskbar/TaskbarActivityContext.java +++ b/quickstep/src/com/android/launcher3/taskbar/TaskbarActivityContext.java @@ -1125,7 +1125,7 @@ public class TaskbarActivityContext extends BaseTaskbarContext { componentKeys, findExactPairMatch, foundTasks -> { - @Nullable Task foundTask = foundTasks.get(0); + @Nullable Task foundTask = foundTasks[0]; if (foundTask != null) { TaskView foundTaskView = recents.getTaskViewByTaskId(foundTask.key.id); if (foundTaskView != null diff --git a/quickstep/src/com/android/launcher3/taskbar/TaskbarUIController.java b/quickstep/src/com/android/launcher3/taskbar/TaskbarUIController.java index df2a43b170..7edf0d3208 100644 --- a/quickstep/src/com/android/launcher3/taskbar/TaskbarUIController.java +++ b/quickstep/src/com/android/launcher3/taskbar/TaskbarUIController.java @@ -219,7 +219,7 @@ public class TaskbarUIController { Collections.singletonList(splitSelectSource.itemInfo.getComponentKey()), false /* findExactPairMatch */, foundTasks -> { - @Nullable Task foundTask = foundTasks.get(0); + @Nullable Task foundTask = foundTasks[0]; splitSelectSource.alreadyRunningTaskId = foundTask == null ? INVALID_TASK_ID : foundTask.key.id; @@ -238,7 +238,7 @@ public class TaskbarUIController { Collections.singletonList(info.getComponentKey()), false /* findExactPairMatch */, foundTasks -> { - @Nullable Task foundTask = foundTasks.get(0); + @Nullable Task foundTask = foundTasks[0]; if (foundTask != null) { TaskView foundTaskView = recents.getTaskViewByTaskId(foundTask.key.id); // TODO (b/266482558): This additional null check is needed because there diff --git a/quickstep/src/com/android/launcher3/uioverrides/QuickstepLauncher.java b/quickstep/src/com/android/launcher3/uioverrides/QuickstepLauncher.java index 9438e00b43..a065387e2f 100644 --- a/quickstep/src/com/android/launcher3/uioverrides/QuickstepLauncher.java +++ b/quickstep/src/com/android/launcher3/uioverrides/QuickstepLauncher.java @@ -644,7 +644,7 @@ public class QuickstepLauncher extends Launcher { Collections.singletonList(splitSelectSource.itemInfo.getComponentKey()), false /* findExactPairMatch */, foundTasks -> { - @Nullable Task foundTask = foundTasks.get(0); + @Nullable Task foundTask = foundTasks[0]; boolean taskWasFound = foundTask != null; splitSelectSource.alreadyRunningTaskId = taskWasFound ? foundTask.key.id diff --git a/quickstep/src/com/android/quickstep/util/AppPairsController.java b/quickstep/src/com/android/quickstep/util/AppPairsController.java index 3ca2531cd4..0f3c029a3b 100644 --- a/quickstep/src/com/android/quickstep/util/AppPairsController.java +++ b/quickstep/src/com/android/quickstep/util/AppPairsController.java @@ -130,7 +130,7 @@ public class AppPairsController { Arrays.asList(app1Key, app2Key), false /* findExactPairMatch */, foundTasks -> { - @Nullable Task foundTask1 = foundTasks.get(0); + @Nullable Task foundTask1 = foundTasks[0]; Intent task1Intent; int task1Id; if (foundTask1 != null) { @@ -147,7 +147,7 @@ public class AppPairsController { LAUNCHER_APP_PAIR_LAUNCH, task1Id); - @Nullable Task foundTask2 = foundTasks.get(1); + @Nullable Task foundTask2 = foundTasks[1]; if (foundTask2 != null) { mSplitSelectStateController.setSecondTask(foundTask2); } else { diff --git a/quickstep/src/com/android/quickstep/util/SplitSelectStateController.java b/quickstep/src/com/android/quickstep/util/SplitSelectStateController.java index d5899e47f4..a9fa3375f0 100644 --- a/quickstep/src/com/android/quickstep/util/SplitSelectStateController.java +++ b/quickstep/src/com/android/quickstep/util/SplitSelectStateController.java @@ -105,7 +105,7 @@ import com.android.wm.shell.splitscreen.ISplitSelectListener; import java.io.PrintWriter; import java.util.ArrayList; -import java.util.Collections; +import java.util.Arrays; import java.util.List; import java.util.function.Consumer; @@ -225,14 +225,14 @@ public class SplitSelectStateController { * tasks (i.e. searching for a running pair of tasks.) */ public void findLastActiveTasksAndRunCallback(@Nullable List componentKeys, - boolean findExactPairMatch, Consumer> callback) { + boolean findExactPairMatch, Consumer callback) { mRecentTasksModel.getTasks(taskGroups -> { if (componentKeys == null || componentKeys.isEmpty()) { - callback.accept(Collections.emptyList()); + callback.accept(new Task[]{}); return; } - List lastActiveTasks = new ArrayList<>(); + Task[] lastActiveTasks = new Task[componentKeys.size()]; if (findExactPairMatch) { // Loop through tasks in reverse, since they are ordered with most-recent tasks last @@ -240,32 +240,35 @@ public class SplitSelectStateController { GroupTask groupTask = taskGroups.get(i); if (isInstanceOfAppPair( groupTask, componentKeys.get(0), componentKeys.get(1))) { - lastActiveTasks.add(groupTask.task1); + lastActiveTasks[0] = groupTask.task1; break; } } } else { // For each key we are looking for, add to lastActiveTasks with the corresponding // Task (or do nothing if not found). - for (ComponentKey key : componentKeys) { + for (int i = 0; i < componentKeys.size(); i++) { + ComponentKey key = componentKeys.get(i); Task lastActiveTask = null; // Loop through tasks in reverse, since they are ordered with recent tasks last - for (int i = taskGroups.size() - 1; i >= 0; i--) { - GroupTask groupTask = taskGroups.get(i); + for (int j = taskGroups.size() - 1; j >= 0; j--) { + GroupTask groupTask = taskGroups.get(j); Task task1 = groupTask.task1; // Don't add duplicate Tasks - if (isInstanceOfComponent(task1, key) && !lastActiveTasks.contains(task1)) { + if (isInstanceOfComponent(task1, key) + && !Arrays.asList(lastActiveTasks).contains(task1)) { lastActiveTask = task1; break; } Task task2 = groupTask.task2; - if (isInstanceOfComponent(task2, key) && !lastActiveTasks.contains(task2)) { + if (isInstanceOfComponent(task2, key) + && !Arrays.asList(lastActiveTasks).contains(task2)) { lastActiveTask = task2; break; } } - lastActiveTasks.add(lastActiveTask); + lastActiveTasks[i] = lastActiveTask; } } diff --git a/quickstep/tests/src/com/android/quickstep/util/SplitSelectStateControllerTest.kt b/quickstep/tests/src/com/android/quickstep/util/SplitSelectStateControllerTest.kt index f41ac4809b..1e39a3480f 100644 --- a/quickstep/tests/src/com/android/quickstep/util/SplitSelectStateControllerTest.kt +++ b/quickstep/tests/src/com/android/quickstep/util/SplitSelectStateControllerTest.kt @@ -107,7 +107,7 @@ class SplitSelectStateControllerTest { // Assertions happen in the callback we get from what we pass into // #findLastActiveTasksAndRunCallback val taskConsumer = - Consumer> { assertNull("No tasks should have matched", it[0] /*task*/) } + Consumer> { assertNull("No tasks should have matched", it[0] /*task*/) } // Capture callback from recentsModel#getTasks() val consumer = @@ -148,7 +148,7 @@ class SplitSelectStateControllerTest { // Assertions happen in the callback we get from what we pass into // #findLastActiveTasksAndRunCallback val taskConsumer = - Consumer> { + Consumer> { assertEquals( "ComponentName package mismatched", it[0].key.baseIntent.component?.packageName, @@ -201,7 +201,7 @@ class SplitSelectStateControllerTest { // Assertions happen in the callback we get from what we pass into // #findLastActiveTasksAndRunCallback val taskConsumer = - Consumer> { assertNull("No tasks should have matched", it[0] /*task*/) } + Consumer> { assertNull("No tasks should have matched", it[0] /*task*/) } // Capture callback from recentsModel#getTasks() val consumer = @@ -244,7 +244,7 @@ class SplitSelectStateControllerTest { // Assertions happen in the callback we get from what we pass into // #findLastActiveTasksAndRunCallback val taskConsumer = - Consumer> { + Consumer> { assertEquals( "ComponentName package mismatched", it[0].key.baseIntent.component?.packageName, @@ -298,7 +298,7 @@ class SplitSelectStateControllerTest { // Assertions happen in the callback we get from what we pass into // #findLastActiveTasksAndRunCallback val taskConsumer = - Consumer> { + Consumer> { assertEquals( "ComponentName package mismatched", it[0].key.baseIntent.component?.packageName, @@ -350,7 +350,7 @@ class SplitSelectStateControllerTest { // Assertions happen in the callback we get from what we pass into // #findLastActiveTasksAndRunCallback val taskConsumer = - Consumer> { + Consumer> { assertEquals("Expected array length 2", 2, it.size) assertNull("No tasks should have matched", it[0] /*task*/) assertEquals( @@ -403,7 +403,7 @@ class SplitSelectStateControllerTest { // Assertions happen in the callback we get from what we pass into // #findLastActiveTasksAndRunCallback val taskConsumer = - Consumer> { + Consumer> { assertEquals("Expected array length 2", 2, it.size) assertEquals( "ComponentName package mismatched", @@ -459,7 +459,7 @@ class SplitSelectStateControllerTest { // Assertions happen in the callback we get from what we pass into // #findLastActiveTasksAndRunCallback val taskConsumer = - Consumer> { + Consumer> { assertEquals("Expected array length 2", 2, it.size) assertEquals( "ComponentName package mismatched", @@ -532,8 +532,8 @@ class SplitSelectStateControllerTest { // Assertions happen in the callback we get from what we pass into // #findLastActiveTasksAndRunCallback val taskConsumer = - Consumer> { - assertEquals("Expected array length 1", 1, it.size) + Consumer> { + assertEquals("Expected array length 2", 2, it.size) assertEquals("Found wrong task", it[0], groupTask2.task1) }