Commit 978d4285 authored by Lijin Shen's avatar Lijin Shen Committed by Commit Bot

Fix crash when multiple bookmarks are being dragged

This CL tries to fix:
1. when one bookmark is being dragged while another dragging animation
is on the progress, getAdapterPosition may return -1.
2. calling notifyDataSetChanged when recyclerview is computing or
scrolling will also throw an error.

Bug: 1002856
Change-Id: I2c22f7ad58c68245cc675b6cccbeb644bd1fbc5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1799469
Commit-Queue: Lijin Shen <lazzzis@google.com>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696612}
parent 99b51a52
...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.widget.dragreorder; ...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.widget.dragreorder;
import android.content.Context; import android.content.Context;
import android.content.res.Resources; import android.content.res.Resources;
import android.graphics.Color; import android.graphics.Color;
import android.support.annotation.Nullable;
import android.support.v4.graphics.ColorUtils; import android.support.v4.graphics.ColorUtils;
import android.support.v4.view.ViewCompat; import android.support.v4.view.ViewCompat;
import android.support.v7.widget.RecyclerView; import android.support.v7.widget.RecyclerView;
...@@ -50,9 +51,15 @@ public abstract class DragReorderableListAdapter<T> extends RecyclerView.Adapter ...@@ -50,9 +51,15 @@ public abstract class DragReorderableListAdapter<T> extends RecyclerView.Adapter
* A callback for touch actions on drag-reorderable lists. * A callback for touch actions on drag-reorderable lists.
*/ */
private class DragTouchCallback extends ItemTouchHelper.Callback { private class DragTouchCallback extends ItemTouchHelper.Callback {
// The view that is being dragged now; null means no view is being dragged now;
private @Nullable ViewHolder mBeingDragged;
@Override @Override
public int getMovementFlags(RecyclerView recyclerView, ViewHolder viewHolder) { public int getMovementFlags(RecyclerView recyclerView, ViewHolder viewHolder) {
if (isActivelyDraggable(viewHolder)) { // this method may be called multiple times until the view is dropped
// ensure there is only one bookmark being dragged
if ((mBeingDragged == viewHolder || mBeingDragged == null)
&& isActivelyDraggable(viewHolder)) {
return makeMovementFlags( return makeMovementFlags(
ItemTouchHelper.UP | ItemTouchHelper.DOWN, 0 /* swipe flags */); ItemTouchHelper.UP | ItemTouchHelper.DOWN, 0 /* swipe flags */);
} }
...@@ -64,7 +71,6 @@ public abstract class DragReorderableListAdapter<T> extends RecyclerView.Adapter ...@@ -64,7 +71,6 @@ public abstract class DragReorderableListAdapter<T> extends RecyclerView.Adapter
int from = current.getAdapterPosition(); int from = current.getAdapterPosition();
int to = target.getAdapterPosition(); int to = target.getAdapterPosition();
if (from == to) return false; if (from == to) return false;
Collections.swap(mElements, from, to); Collections.swap(mElements, from, to);
notifyItemMoved(from, to); notifyItemMoved(from, to);
return true; return true;
...@@ -74,7 +80,9 @@ public abstract class DragReorderableListAdapter<T> extends RecyclerView.Adapter ...@@ -74,7 +80,9 @@ public abstract class DragReorderableListAdapter<T> extends RecyclerView.Adapter
public void onSelectedChanged(ViewHolder viewHolder, int actionState) { public void onSelectedChanged(ViewHolder viewHolder, int actionState) {
super.onSelectedChanged(viewHolder, actionState); super.onSelectedChanged(viewHolder, actionState);
if (actionState == ItemTouchHelper.ACTION_STATE_DRAG) { // similar to getMovementFlags, this method may be called multiple times
if (actionState == ItemTouchHelper.ACTION_STATE_DRAG && mBeingDragged != viewHolder) {
mBeingDragged = viewHolder;
mStart = viewHolder.getAdapterPosition(); mStart = viewHolder.getAdapterPosition();
onDragStateChange(true); onDragStateChange(true);
updateVisualState(true, viewHolder); updateVisualState(true, viewHolder);
...@@ -88,6 +96,8 @@ public abstract class DragReorderableListAdapter<T> extends RecyclerView.Adapter ...@@ -88,6 +96,8 @@ public abstract class DragReorderableListAdapter<T> extends RecyclerView.Adapter
// Commit the position change for the dragged item when it's dropped. // Commit the position change for the dragged item when it's dropped.
setOrder(mElements); setOrder(mElements);
} }
// the row has been dropped, even though it is possible at same row
mBeingDragged = null;
onDragStateChange(false); onDragStateChange(false);
updateVisualState(false, viewHolder); updateVisualState(false, viewHolder);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment