From 77f01524bd6a031a8fdb8089fa96aeda12f14c8d Mon Sep 17 00:00:00 2001 From: Kevin Date: Wed, 17 Apr 2019 12:04:35 -0700 Subject: [PATCH] Fix NPE and make getTask return Optional Fix an NPE when attempting to update the thumbnail in IconRecentsView. If the task item is binding the loading UI, it has no task so the app will crash, so we just need to ensure the task is actually present first. While we're here, also change the API to return an Optional and make a tighter contract, making it harder to make this mistake in the future. Bug: 130746661 Fix: 130746661 Test: Go from app => overview on recents Go Change-Id: I1402fcd2e58bdeb703c7dcc1b9dcf0d258808b3d --- .../com/android/quickstep/TaskActionController.java | 11 ++++++----- .../src/com/android/quickstep/TaskAdapter.java | 5 +++-- .../src/com/android/quickstep/TaskHolder.java | 7 ++++--- .../com/android/quickstep/views/IconRecentsView.java | 7 +++++-- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/go/quickstep/src/com/android/quickstep/TaskActionController.java b/go/quickstep/src/com/android/quickstep/TaskActionController.java index 71bee9181d..40195eb4c9 100644 --- a/go/quickstep/src/com/android/quickstep/TaskActionController.java +++ b/go/quickstep/src/com/android/quickstep/TaskActionController.java @@ -42,7 +42,7 @@ public final class TaskActionController { * @param viewHolder the task view holder to launch */ public void launchTask(TaskHolder viewHolder) { - if (viewHolder.getTask() == null) { + if (!viewHolder.getTask().isPresent()) { return; } TaskItemView itemView = (TaskItemView) (viewHolder.itemView); @@ -53,8 +53,9 @@ public final class TaskActionController { int height = v.getMeasuredHeight(); ActivityOptions opts = ActivityOptions.makeClipRevealAnimation(v, left, top, width, height); - ActivityManagerWrapper.getInstance().startActivityFromRecentsAsync(viewHolder.getTask().key, - opts, null /* resultCallback */, null /* resultCallbackHandler */); + ActivityManagerWrapper.getInstance().startActivityFromRecentsAsync( + viewHolder.getTask().get().key, opts, null /* resultCallback */, + null /* resultCallbackHandler */); } /** @@ -63,11 +64,11 @@ public final class TaskActionController { * @param viewHolder the task view holder to remove */ public void removeTask(TaskHolder viewHolder) { - if (viewHolder.getTask() == null) { + if (!viewHolder.getTask().isPresent()) { return; } int position = viewHolder.getAdapterPosition(); - Task task = viewHolder.getTask(); + Task task = viewHolder.getTask().get(); ActivityManagerWrapper.getInstance().removeTask(task.key.id); mLoader.removeTask(task); mAdapter.notifyItemRemoved(position); diff --git a/go/quickstep/src/com/android/quickstep/TaskAdapter.java b/go/quickstep/src/com/android/quickstep/TaskAdapter.java index 7e7f278297..18d5877aa2 100644 --- a/go/quickstep/src/com/android/quickstep/TaskAdapter.java +++ b/go/quickstep/src/com/android/quickstep/TaskAdapter.java @@ -28,6 +28,7 @@ import com.android.systemui.shared.recents.model.Task; import java.util.List; import java.util.Objects; +import java.util.Optional; /** * Recycler view adapter that dynamically inflates and binds {@link TaskHolder} instances with the @@ -109,13 +110,13 @@ public final class TaskAdapter extends Adapter { holder.bindTask(task, willAnimate /* willAnimate */); mLoader.loadTaskIconAndLabel(task, () -> { // Ensure holder still has the same task. - if (Objects.equals(task, holder.getTask())) { + if (Objects.equals(Optional.of(task), holder.getTask())) { holder.getTaskItemView().setIcon(task.icon); holder.getTaskItemView().setLabel(task.titleDescription); } }); mLoader.loadTaskThumbnail(task, () -> { - if (Objects.equals(task, holder.getTask())) { + if (Objects.equals(Optional.of(task), holder.getTask())) { holder.getTaskItemView().setThumbnail(task.thumbnail.thumbnail); } }); diff --git a/go/quickstep/src/com/android/quickstep/TaskHolder.java b/go/quickstep/src/com/android/quickstep/TaskHolder.java index a3fa5c1f2b..5755df4f4a 100644 --- a/go/quickstep/src/com/android/quickstep/TaskHolder.java +++ b/go/quickstep/src/com/android/quickstep/TaskHolder.java @@ -18,12 +18,13 @@ package com.android.quickstep; import android.graphics.Bitmap; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import androidx.recyclerview.widget.RecyclerView.ViewHolder; import com.android.quickstep.views.TaskItemView; import com.android.systemui.shared.recents.model.Task; +import java.util.Optional; + /** * A recycler view holder that holds the task view and binds {@link Task} content (app title, icon, * etc.) to the view. @@ -81,7 +82,7 @@ public final class TaskHolder extends ViewHolder { * * @return the current task */ - public @Nullable Task getTask() { - return mTask; + public Optional getTask() { + return Optional.ofNullable(mTask); } } diff --git a/go/quickstep/src/com/android/quickstep/views/IconRecentsView.java b/go/quickstep/src/com/android/quickstep/views/IconRecentsView.java index 9688fdfdaa..a0efce456a 100644 --- a/go/quickstep/src/com/android/quickstep/views/IconRecentsView.java +++ b/go/quickstep/src/com/android/quickstep/views/IconRecentsView.java @@ -61,6 +61,8 @@ import com.android.quickstep.TaskListLoader; import com.android.quickstep.TaskSwipeCallback; import com.android.systemui.shared.recents.model.Task; +import java.util.Optional; + /** * Root view for the icon recents view. Acts as the main interface to the rest of the Launcher code * base. @@ -120,8 +122,9 @@ public final class IconRecentsView extends FrameLayout { TaskItemView[] itemViews = getTaskViews(); for (TaskItemView taskView : itemViews) { TaskHolder taskHolder = (TaskHolder) mTaskRecyclerView.getChildViewHolder(taskView); - Task task = taskHolder.getTask(); - if (taskHolder.getTask().key.id == taskId) { + Optional optTask = taskHolder.getTask(); + if (optTask.filter(task -> task.key.id == taskId).isPresent()) { + Task task = optTask.get(); // Update thumbnail on the task. task.thumbnail = thumbnailData; taskView.setThumbnail(thumbnailData.thumbnail);