From c590a62c80acf432578d5d1a99010e730f9bee94 Mon Sep 17 00:00:00 2001 From: Ahmed Fakhry Date: Tue, 28 Jan 2025 18:20:26 +0000 Subject: [PATCH 1/2] Remove `GroupTask.mSplitBounds` See go/refactor-group-task for details. This CL removes `mSplitBounds` from `GroupTask` and adds `splitBounds` to `SplitTask`. This makes its usages more intentional. Bug: 388593902 Test: m Flag: EXEMPT pure refactor with no behavior change. Change-Id: I9a3e8ee2e96d46f9eb79a58ffcf5efc257615aac --- .../taskbar/KeyboardQuickSwitchView.java | 2 +- .../uioverrides/QuickstepLauncher.java | 8 ++----- .../com/android/quickstep/util/DesktopTask.kt | 2 +- .../com/android/quickstep/util/GroupTask.kt | 24 +++++++------------ .../android/quickstep/views/RecentsView.java | 2 +- 5 files changed, 13 insertions(+), 25 deletions(-) diff --git a/quickstep/src/com/android/launcher3/taskbar/KeyboardQuickSwitchView.java b/quickstep/src/com/android/launcher3/taskbar/KeyboardQuickSwitchView.java index 73f9bea668..4581119c72 100644 --- a/quickstep/src/com/android/launcher3/taskbar/KeyboardQuickSwitchView.java +++ b/quickstep/src/com/android/launcher3/taskbar/KeyboardQuickSwitchView.java @@ -275,7 +275,7 @@ public class KeyboardQuickSwitchView extends ConstraintLayout { task2, updateTasks ? mViewCallbacks::updateThumbnailInBackground : null, updateTasks ? mViewCallbacks::updateIconInBackground : null, - groupTask.mSplitBounds); + groupTask instanceof SplitTask splitTask ? splitTask.getSplitBounds() : null); previousTaskView = currentTaskView; } diff --git a/quickstep/src/com/android/launcher3/uioverrides/QuickstepLauncher.java b/quickstep/src/com/android/launcher3/uioverrides/QuickstepLauncher.java index c880aa9412..690dec40e6 100644 --- a/quickstep/src/com/android/launcher3/uioverrides/QuickstepLauncher.java +++ b/quickstep/src/com/android/launcher3/uioverrides/QuickstepLauncher.java @@ -65,7 +65,6 @@ import static com.android.launcher3.util.Executors.UI_HELPER_EXECUTOR; import static com.android.quickstep.util.AnimUtils.completeRunnableListCallback; import static com.android.quickstep.util.SplitAnimationTimings.TABLET_HOME_TO_SPLIT; import static com.android.systemui.shared.system.ActivityManagerWrapper.CLOSE_SYSTEM_WINDOWS_REASON_HOME_KEY; -import static com.android.wm.shell.shared.split.SplitScreenConstants.SNAP_TO_2_50_50; import android.animation.Animator; import android.animation.AnimatorListenerAdapter; @@ -1390,16 +1389,13 @@ public class QuickstepLauncher extends Launcher implements RecentsViewContainer, */ public void launchSplitTasks( @NonNull SplitTask splitTask, @Nullable RemoteTransition remoteTransition) { - mSplitSelectStateController.launchExistingSplitPair( - null /* launchingTaskView */, + mSplitSelectStateController.launchExistingSplitPair(null /* launchingTaskView */, splitTask.getTopLeftTask().key.id, splitTask.getBottomRightTask().key.id, SplitConfigurationOptions.STAGE_POSITION_TOP_OR_LEFT, /* callback= */ success -> mSplitSelectStateController.resetState(), /* freezeTaskList= */ false, - splitTask.mSplitBounds == null - ? SNAP_TO_2_50_50 - : splitTask.mSplitBounds.snapPosition, + splitTask.getSplitBounds().snapPosition, remoteTransition); } diff --git a/quickstep/src/com/android/quickstep/util/DesktopTask.kt b/quickstep/src/com/android/quickstep/util/DesktopTask.kt index 066918571b..5d61dd4775 100644 --- a/quickstep/src/com/android/quickstep/util/DesktopTask.kt +++ b/quickstep/src/com/android/quickstep/util/DesktopTask.kt @@ -23,7 +23,7 @@ import com.android.systemui.shared.recents.model.Task * tasks list. */ class DesktopTask(override val tasks: List) : - GroupTask(tasks[0], null, null, TaskViewType.DESKTOP) { + GroupTask(tasks[0], task2 = null, TaskViewType.DESKTOP) { override fun containsTask(taskId: Int) = tasks.any { it.key.id == taskId } diff --git a/quickstep/src/com/android/quickstep/util/GroupTask.kt b/quickstep/src/com/android/quickstep/util/GroupTask.kt index 8237d2e3f2..9ab963399c 100644 --- a/quickstep/src/com/android/quickstep/util/GroupTask.kt +++ b/quickstep/src/com/android/quickstep/util/GroupTask.kt @@ -30,19 +30,12 @@ abstract class GroupTask constructor( @Deprecated("Prefer using `getTasks()` instead") @JvmField val task1: Task, @Deprecated("Prefer using `getTasks()` instead") @JvmField val task2: Task?, - @JvmField val mSplitBounds: SplitConfigurationOptions.SplitBounds?, @JvmField val taskViewType: TaskViewType, ) { protected constructor( task1: Task, task2: Task?, - splitBounds: SplitConfigurationOptions.SplitBounds?, - ) : this( - task1, - task2, - splitBounds, - if (task2 != null) TaskViewType.GROUPED else TaskViewType.SINGLE, - ) + ) : this(task1, task2, if (task2 != null) TaskViewType.GROUPED else TaskViewType.SINGLE) open fun containsTask(taskId: Int) = task1.key.id == taskId || (task2 != null && task2.key.id == taskId) @@ -83,8 +76,7 @@ constructor( } /** A [Task] container that must contain exactly one task in the recent tasks list. */ -class SingleTask(task: Task) : - GroupTask(task, task2 = null, mSplitBounds = null, TaskViewType.SINGLE) { +class SingleTask(task: Task) : GroupTask(task, task2 = null, TaskViewType.SINGLE) { val task: Task get() = task1 @@ -104,25 +96,25 @@ class SingleTask(task: Task) : * A [Task] container that must contain exactly two tasks and split bounds to represent an app-pair * in the recent tasks list. */ -class SplitTask(task1: Task, task2: Task, splitBounds: SplitConfigurationOptions.SplitBounds) : - GroupTask(task1, task2, splitBounds, TaskViewType.GROUPED) { +class SplitTask(task1: Task, task2: Task, val splitBounds: SplitConfigurationOptions.SplitBounds) : + GroupTask(task1, task2, TaskViewType.GROUPED) { val topLeftTask: Task - get() = if (mSplitBounds!!.leftTopTaskId == task1.key.id) task1!! else task2!! + get() = if (splitBounds.leftTopTaskId == task1.key.id) task1!! else task2!! val bottomRightTask: Task get() = if (topLeftTask == task1) task2!! else task1!! - override fun copy() = SplitTask(task1, task2!!, mSplitBounds!!) + override fun copy() = SplitTask(task1, task2!!, splitBounds) override fun toString() = "type=$taskViewType task1=$task1 task2=$task2" override fun equals(o: Any?): Boolean { if (this === o) return true if (o !is SplitTask) return false - if (mSplitBounds!! != o.mSplitBounds!!) return false + if (splitBounds != o.splitBounds) return false return super.equals(o) } - override fun hashCode() = Objects.hash(super.hashCode(), mSplitBounds) + override fun hashCode() = Objects.hash(super.hashCode(), splitBounds) } diff --git a/quickstep/src/com/android/quickstep/views/RecentsView.java b/quickstep/src/com/android/quickstep/views/RecentsView.java index c6db576177..5e59a06166 100644 --- a/quickstep/src/com/android/quickstep/views/RecentsView.java +++ b/quickstep/src/com/android/quickstep/views/RecentsView.java @@ -1988,7 +1988,7 @@ public abstract class RecentsView< var splitTask = (SplitTask) groupTask; groupedTaskView.bind(splitTask.getTopLeftTask(), splitTask.getBottomRightTask(), mOrientationState, - mTaskOverlayFactory, splitTask.mSplitBounds); + mTaskOverlayFactory, splitTask.getSplitBounds()); } else if (taskView instanceof DesktopTaskView desktopTaskView) { // Minimized tasks should not be shown in Overview List nonMinimizedTasks = From 129a3a00976898ba37d85b897f296d41b8530d2c Mon Sep 17 00:00:00 2001 From: Ahmed Fakhry Date: Tue, 28 Jan 2025 22:11:18 +0000 Subject: [PATCH 2/2] Finally remove `task1` and `task2` from `GroupTask` See go/refactor-group-task for details. This is the final CL in this refactor, where `task1` and `task2` are finally removed, and `DesktopTask` is allowed to have empty tasks. Bug: 388593902 Test: m Flag: EXEMPT pure refactor with no behavior change. Change-Id: I8af39b58138f11030981311efc2e95a77cef125b --- .../launcher3/taskbar/TaskbarView.java | 6 +-- .../com/android/quickstep/util/DesktopTask.kt | 9 +--- .../com/android/quickstep/util/GroupTask.kt | 45 +++++-------------- 3 files changed, 16 insertions(+), 44 deletions(-) diff --git a/quickstep/src/com/android/launcher3/taskbar/TaskbarView.java b/quickstep/src/com/android/launcher3/taskbar/TaskbarView.java index c7f33e94e0..fd327c21d6 100644 --- a/quickstep/src/com/android/launcher3/taskbar/TaskbarView.java +++ b/quickstep/src/com/android/launcher3/taskbar/TaskbarView.java @@ -26,8 +26,6 @@ import static com.android.launcher3.LauncherSettings.Favorites.ITEM_TYPE_FOLDER; import static com.android.launcher3.config.FeatureFlags.enableTaskbarPinning; import static com.android.launcher3.icons.IconNormalizer.ICON_VISIBLE_AREA_FACTOR; -import static java.util.function.Predicate.not; - import android.content.Context; import android.content.res.Resources; import android.graphics.Canvas; @@ -420,7 +418,7 @@ public class TaskbarView extends FrameLayout implements FolderIcon.FolderIconPar .filter(Objects::nonNull) .toArray(ItemInfo[]::new); // TODO(b/343289567 and b/316004172): support app pairs and desktop mode. - recentTasks = recentTasks.stream().filter(not(GroupTask::supportsMultipleTasks)).toList(); + recentTasks = recentTasks.stream().filter(it -> it instanceof SingleTask).toList(); if (taskbarRecentsLayoutTransition()) { updateItemsWithLayoutTransition(hotseatItemInfos, recentTasks); @@ -671,7 +669,7 @@ public class TaskbarView extends FrameLayout implements FolderIcon.FolderIconPar // Replace any Recent views with the appropriate type if it's not already that type. final int expectedLayoutResId; boolean isCollection = false; - if (task.supportsMultipleTasks()) { + if (!(task instanceof SingleTask)) { if (task.taskViewType == TaskViewType.DESKTOP) { // TODO(b/316004172): use Desktop tile layout. expectedLayoutResId = -1; diff --git a/quickstep/src/com/android/quickstep/util/DesktopTask.kt b/quickstep/src/com/android/quickstep/util/DesktopTask.kt index 5d61dd4775..53ea022a23 100644 --- a/quickstep/src/com/android/quickstep/util/DesktopTask.kt +++ b/quickstep/src/com/android/quickstep/util/DesktopTask.kt @@ -20,14 +20,9 @@ import com.android.systemui.shared.recents.model.Task /** * A [Task] container that can contain N number of tasks that are part of the desktop in recent - * tasks list. + * tasks list. Note that desktops can be empty with no tasks in them. */ -class DesktopTask(override val tasks: List) : - GroupTask(tasks[0], task2 = null, TaskViewType.DESKTOP) { - - override fun containsTask(taskId: Int) = tasks.any { it.key.id == taskId } - - override fun supportsMultipleTasks() = true +class DesktopTask(tasks: List) : GroupTask(tasks, TaskViewType.DESKTOP) { override fun copy() = DesktopTask(tasks) diff --git a/quickstep/src/com/android/quickstep/util/GroupTask.kt b/quickstep/src/com/android/quickstep/util/GroupTask.kt index 9ab963399c..49c37dce71 100644 --- a/quickstep/src/com/android/quickstep/util/GroupTask.kt +++ b/quickstep/src/com/android/quickstep/util/GroupTask.kt @@ -15,7 +15,6 @@ */ package com.android.quickstep.util -import androidx.annotation.VisibleForTesting import com.android.launcher3.util.SplitConfigurationOptions import com.android.quickstep.views.TaskViewType import com.android.systemui.shared.recents.model.Task @@ -25,20 +24,8 @@ import java.util.Objects * An abstract class for creating [Task] containers that can be [SingleTask]s, [SplitTask]s, or * [DesktopTask]s in the recent tasks list. */ -abstract class GroupTask -@VisibleForTesting(otherwise = VisibleForTesting.PROTECTED) -constructor( - @Deprecated("Prefer using `getTasks()` instead") @JvmField val task1: Task, - @Deprecated("Prefer using `getTasks()` instead") @JvmField val task2: Task?, - @JvmField val taskViewType: TaskViewType, -) { - protected constructor( - task1: Task, - task2: Task?, - ) : this(task1, task2, if (task2 != null) TaskViewType.GROUPED else TaskViewType.SINGLE) - - open fun containsTask(taskId: Int) = - task1.key.id == taskId || (task2 != null && task2.key.id == taskId) +abstract class GroupTask(val tasks: List, @JvmField val taskViewType: TaskViewType) { + fun containsTask(taskId: Int) = tasks.any { it.key.id == taskId } /** * Returns true if a task in this group has a package name that matches the given `packageName`. @@ -54,18 +41,9 @@ constructor( fun isEmpty() = tasks.isEmpty() - /** Returns whether this task supports multiple tasks or not. */ - open fun supportsMultipleTasks() = taskViewType == TaskViewType.GROUPED - - /** Returns a List of all the Tasks in this GroupTask */ - open val tasks: List - get() = listOfNotNull(task1, task2) - /** Creates a copy of this instance */ abstract fun copy(): GroupTask - override fun toString() = "type=$taskViewType task1=$task1 task2=$task2" - override fun equals(o: Any?): Boolean { if (this === o) return true if (o !is GroupTask) return false @@ -76,14 +54,14 @@ constructor( } /** A [Task] container that must contain exactly one task in the recent tasks list. */ -class SingleTask(task: Task) : GroupTask(task, task2 = null, TaskViewType.SINGLE) { +class SingleTask(task: Task) : GroupTask(listOf(task), TaskViewType.SINGLE) { val task: Task - get() = task1 + get() = tasks[0] - override fun copy() = SingleTask(task1) + override fun copy() = SingleTask(task) - override fun toString() = "type=$taskViewType task=$task1" + override fun toString() = "type=$taskViewType task=$task" override fun equals(o: Any?): Boolean { if (this === o) return true @@ -97,17 +75,18 @@ class SingleTask(task: Task) : GroupTask(task, task2 = null, TaskViewType.SINGLE * in the recent tasks list. */ class SplitTask(task1: Task, task2: Task, val splitBounds: SplitConfigurationOptions.SplitBounds) : - GroupTask(task1, task2, TaskViewType.GROUPED) { + GroupTask(listOf(task1, task2), TaskViewType.GROUPED) { val topLeftTask: Task - get() = if (splitBounds.leftTopTaskId == task1.key.id) task1!! else task2!! + get() = if (splitBounds.leftTopTaskId == tasks[0].key.id) tasks[0] else tasks[1] val bottomRightTask: Task - get() = if (topLeftTask == task1) task2!! else task1!! + get() = if (topLeftTask == tasks[0]) tasks[1] else tasks[0] - override fun copy() = SplitTask(task1, task2!!, splitBounds) + override fun copy() = SplitTask(tasks[0], tasks[1], splitBounds) - override fun toString() = "type=$taskViewType task1=$task1 task2=$task2" + override fun toString() = + "type=$taskViewType topLeftTask=$topLeftTask bottomRightTask=$bottomRightTask" override fun equals(o: Any?): Boolean { if (this === o) return true