Enable Dragging for GTS

This change enables dragging to reorganize tabs in grid tab switcher.
It is disabled for groups for now.

Bug: 960102
Change-Id: I7148ee2b44133d1847df54c71f5712c55fd10cda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1597341
Commit-Queue: Yue Zhang <yuezhanggg@google.com>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658428}

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediator.java

@@ -34,6 +34,7 @@ import org.chromium.chrome.browser.tabmodel.TabModelSelector;
 import org.chromium.chrome.browser.tabmodel.TabModelUtils;
 import org.chromium.chrome.browser.tabmodel.TabSelectionType;
 import org.chromium.chrome.browser.tasks.tab_groups.TabGroupUtils;
+import org.chromium.chrome.browser.util.FeatureUtilities;
 import org.chromium.components.feature_engagement.FeatureConstants;
 import org.chromium.content_public.browser.NavigationHandle;
 import org.chromium.ui.modelutil.PropertyModel;
@@ -316,6 +317,13 @@ class TabListMediator {
                 if (mModel.indexFromId(tab.getId()) == TabModel.INVALID_TAB_INDEX) return;
                 mModel.removeAt(mModel.indexFromId(tab.getId()));
             }
+
+            @Override
+            public void didMoveTab(Tab tab, int newIndex, int curIndex) {
+                if (mModel.indexFromId(tab.getId()) == TabModel.INVALID_TAB_INDEX) return;
+                if (newIndex >= mModel.size() || curIndex >= mModel.size()) return;
+                mModel.move(curIndex, newIndex);
+            }
         };
 
         mTabModelSelector.getTabModelFilterProvider().addTabModelFilterObserver(mTabModelObserver);
@@ -434,11 +442,31 @@ class TabListMediator {
      * @return The callback that hosts the logic for swipe and drag related actions.
      */
     ItemTouchHelper.SimpleCallback getItemTouchHelperCallback(final float swipeToDismissThreshold) {
-        return new ItemTouchHelper.SimpleCallback(0, ItemTouchHelper.START | ItemTouchHelper.END) {
+        return new ItemTouchHelper.SimpleCallback(0, 0) {
+            @Override
+            public int getMovementFlags(
+                    RecyclerView recyclerView, RecyclerView.ViewHolder viewHolder) {
+                final int dragFlags = !FeatureUtilities.isTabGroupsAndroidEnabled()
+                        ? ItemTouchHelper.START | ItemTouchHelper.END | ItemTouchHelper.UP
+                                | ItemTouchHelper.DOWN
+                        : 0;
+                final int swipeFlags = ItemTouchHelper.START | ItemTouchHelper.END;
+                return makeMovementFlags(dragFlags, swipeFlags);
+            }
+
             @Override
-            public boolean onMove(RecyclerView recyclerView, RecyclerView.ViewHolder viewHolder,
-                    RecyclerView.ViewHolder viewHolder1) {
-                return false;
+            public boolean onMove(RecyclerView recyclerView, RecyclerView.ViewHolder fromViewHolder,
+                    RecyclerView.ViewHolder toViewHolder) {
+                assert fromViewHolder instanceof TabGridViewHolder;
+                assert toViewHolder instanceof TabGridViewHolder;
+
+                int tabId = ((TabGridViewHolder) fromViewHolder).getTabId();
+                int curIndex = mModel.indexFromId(tabId);
+                int distance =
+                        toViewHolder.getAdapterPosition() - fromViewHolder.getAdapterPosition();
+                int newIndex = curIndex + (distance > 0 ? distance + 1 : distance);
+                mTabModelSelector.getCurrentModel().moveTab(tabId, newIndex);
+                return true;
             }
 
             @Override

chrome/android/features/tab_ui/junit/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediatorUnitTest.java

@@ -23,6 +23,7 @@ import static org.mockito.Mockito.when;
 import android.graphics.Color;
 import android.graphics.drawable.ColorDrawable;
 import android.graphics.drawable.Drawable;
+import android.support.v7.widget.RecyclerView;
 import android.view.View;
 
 import org.junit.After;
@@ -68,6 +69,8 @@ public class TabListMediatorUnitTest {
     private static final int TAB1_ID = 456;
     private static final int TAB2_ID = 789;
     private static final int TAB3_ID = 123;
+    private static final int POSITION1 = 0;
+    private static final int POSITION2 = 1;
 
     @Mock
     TabContentManager mTabContentManager;
@@ -81,6 +84,8 @@ public class TabListMediatorUnitTest {
     TabModel mTabModel;
     @Mock
     TabListFaviconProvider mTabListFaviconProvider;
+    @Mock
+    RecyclerView mRecyclerView;
     @Captor
     ArgumentCaptor<TabModelObserver> mTabModelObserverCaptor;
     @Captor
@@ -94,6 +99,8 @@ public class TabListMediatorUnitTest {
     private Tab mTab2;
     private TabListMediator mMediator;
     private TabListModel mModel;
+    private TabGridViewHolder mViewHolder1;
+    private TabGridViewHolder mViewHolder2;
 
     @Before
     public void setUp() {
@@ -104,6 +111,8 @@ public class TabListMediatorUnitTest {
 
         mTab1 = prepareTab(TAB1_ID, TAB1_TITLE);
         mTab2 = prepareTab(TAB2_ID, TAB2_TITLE);
+        mViewHolder1 = prepareViewHolder(TAB1_ID, POSITION1);
+        mViewHolder2 = prepareViewHolder(TAB2_ID, POSITION2);
 
         List<TabModel> tabModelList = new ArrayList<>();
         tabModelList.add(mTabModel);
@@ -127,6 +136,10 @@ public class TabListMediatorUnitTest {
         doNothing()
                 .when(mTabListFaviconProvider)
                 .getFaviconForUrlAsync(anyString(), anyBoolean(), mCallbackCaptor.capture());
+        doReturn(mTab1).when(mTabModelSelector).getTabById(TAB1_ID);
+        doReturn(mTab2).when(mTabModelSelector).getTabById(TAB2_ID);
+        doReturn(POSITION1).when(mTabModel).indexOf(mTab1);
+        doReturn(POSITION2).when(mTabModel).indexOf(mTab2);
 
         mModel = new TabListModel();
         mMediator = new TabListMediator(mModel, mTabModelSelector,
@@ -190,6 +203,15 @@ public class TabListMediatorUnitTest {
         verify(mTabModel).closeTab(eq(mTab2), eq(null), eq(false), eq(false), eq(true));
     }
 
+    @Test
+    public void sendsMoveTabSignalCorrectly() {
+        initAndAssertAllProperties();
+
+        mMediator.getItemTouchHelperCallback(0f).onMove(mRecyclerView, mViewHolder1, mViewHolder2);
+
+        verify(mTabModel).moveTab(eq(TAB1_ID), eq(2));
+    }
+
     @Test
     public void tabClosure() {
         initAndAssertAllProperties();
@@ -335,6 +357,36 @@ public class TabListMediatorUnitTest {
         assertThat(mModel.get(2).get(TabProperties.TITLE), equalTo(TAB3_TITLE));
     }
 
+    @Test
+    public void tabMovement_GTS_Forward() {
+        initAndAssertAllProperties();
+
+        assertThat(mModel.size(), equalTo(2));
+        assertThat(mModel.get(1).get(TabProperties.TAB_ID), equalTo(TAB2_ID));
+        assertThat(mModel.get(1).get(TabProperties.TITLE), equalTo(TAB2_TITLE));
+
+        mTabModelObserverCaptor.getValue().didMoveTab(mTab2, POSITION1, POSITION2);
+
+        assertThat(mModel.size(), equalTo(2));
+        assertThat(mModel.get(0).get(TabProperties.TAB_ID), equalTo(TAB2_ID));
+        assertThat(mModel.get(0).get(TabProperties.TITLE), equalTo(TAB2_TITLE));
+    }
+
+    @Test
+    public void tabMovement_GTS_BackWard() {
+        initAndAssertAllProperties();
+
+        assertThat(mModel.size(), equalTo(2));
+        assertThat(mModel.get(1).get(TabProperties.TAB_ID), equalTo(TAB2_ID));
+        assertThat(mModel.get(1).get(TabProperties.TITLE), equalTo(TAB2_TITLE));
+
+        mTabModelObserverCaptor.getValue().didMoveTab(mTab1, POSITION2, POSITION1);
+
+        assertThat(mModel.size(), equalTo(2));
+        assertThat(mModel.get(0).get(TabProperties.TAB_ID), equalTo(TAB2_ID));
+        assertThat(mModel.get(0).get(TabProperties.TITLE), equalTo(TAB2_TITLE));
+    }
+
     private void initAndAssertAllProperties() {
         List<Tab> tabs = new ArrayList<>();
         for (int i = 0; i < mTabModel.getCount(); i++) {
@@ -384,4 +436,11 @@ public class TabListMediatorUnitTest {
         doReturn(title).when(tab).getTitle();
         return tab;
     }
+
+    private TabGridViewHolder prepareViewHolder(int id, int position) {
+        TabGridViewHolder viewHolder = mock(TabGridViewHolder.class);
+        doReturn(id).when(viewHolder).getTabId();
+        doReturn(position).when(viewHolder).getAdapterPosition();
+        return viewHolder;
+    }
 }

ui/android/BUILD.gn

@@ -376,6 +376,7 @@ junit_binary("ui_junit_tests") {
     "junit/src/org/chromium/ui/drawable/StateListDrawableBuilderTest.java",
     "junit/src/org/chromium/ui/modaldialog/ModalDialogManagerTest.java",
     "junit/src/org/chromium/ui/modelutil/LazyConstructionPropertyMcpTest.java",
+    "junit/src/org/chromium/ui/modelutil/ListModelBaseTest.java",
     "junit/src/org/chromium/ui/modelutil/PropertyModelTest.java",
     "junit/src/org/chromium/ui/modelutil/SimpleListObservableTest.java",
     "junit/src/org/chromium/ui/text/SpanApplierTest.java",

ui/android/java/src/org/chromium/ui/modelutil/ForwardingListObservable.java

@@ -28,4 +28,9 @@ public class ForwardingListObservable<P>
             ListObservable<P> source, int index, int count, @Nullable P payload) {
         notifyItemRangeChanged(index, count, payload);
     }
+
+    @Override
+    public void onItemMoved(ListObservable source, int fromIndex, int toIndex) {
+        notifyItemMoved(fromIndex, toIndex);
+    }
 }

ui/android/java/src/org/chromium/ui/modelutil/ListModelBase.java

@@ -154,4 +154,19 @@ public class ListModelBase<T, P> extends ListObservableImpl<P> implements Simple
     public int indexOf(Object item) {
         return mItems.indexOf(item);
     }
+
+    /**
+     * Moves a single {@code item} from current {@code index} to new {@code index}.
+     * @param curIndex The position of the item before move.
+     * @param newIndex The position of the item after move.
+     */
+    public void move(int curIndex, int newIndex) {
+        T item = mItems.remove(curIndex);
+        if (newIndex == mItems.size()) {
+            mItems.add(item);
+        } else {
+            mItems.add(newIndex, item);
+        }
+        notifyItemMoved(curIndex, newIndex);
+    }
 }

ui/android/java/src/org/chromium/ui/modelutil/ListObservable.java

@@ -62,5 +62,13 @@ public interface ListObservable<P> {
          */
         default void onItemRangeChanged(
                 ListObservable<P> source, int index, int count, @Nullable P payload) {}
+
+        /**
+         * Notifies that item at position {@code curIndex} will be moved to {@code newIndex}.
+         *
+         * @param curIndex Current position of the moved item.
+         * @param newIndex New position of the moved item.
+         */
+        default void onItemMoved(ListObservable source, int curIndex, int newIndex) {}
     }
 }

ui/android/java/src/org/chromium/ui/modelutil/ListObservableImpl.java

@@ -91,4 +91,16 @@ public abstract class ListObservableImpl<P> implements ListObservable<P> {
             observer.onItemRangeChanged(this, index, count, payload);
         }
     }
+
+    /**
+     * Notifies observers that item at position {@code curIndex} will be moved to {@code newIndex}.
+     *
+     * @param curIndex Current position of the moved item.
+     * @param newIndex New position of the moved item.
+     */
+    protected void notifyItemMoved(int curIndex, int newIndex) {
+        for (ListObserver observer : mObservers) {
+            observer.onItemMoved(this, curIndex, newIndex);
+        }
+    }
 }

ui/android/java/src/org/chromium/ui/modelutil/RecyclerViewAdapter.java

@@ -189,4 +189,9 @@ public class RecyclerViewAdapter<VH extends ViewHolder, P>
             ListObservable<P> source, int index, int count, @Nullable P payload) {
         notifyItemRangeChanged(index, count, payload);
     }
+
+    @Override
+    public void onItemMoved(ListObservable source, int curIndex, int newIndex) {
+        notifyItemMoved(curIndex, newIndex);
+    }
 }

ui/android/junit/src/org/chromium/ui/modelutil/ListModelBaseTest.java

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.ui.modelutil;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.robolectric.annotation.Config;
+
+import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.ui.modelutil.ListObservable.ListObserver;
+
+/**
+ * Basic test ensuring the {@link ListModel} notifies listeners properly.
+ * TODO(yuezhanggg) Merge this test with SimpleListObservableTest.
+ */
+@RunWith(BaseRobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class ListModelBaseTest {
+    @Mock
+    private ListObserver<Integer> mObserver;
+
+    private ListModelBase<Integer, Integer> mIntegerList = new ListModelBase<>();
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+        mIntegerList.addObserver(mObserver);
+    }
+
+    @After
+    public void tearDown() {
+        mIntegerList.removeObserver(mObserver);
+    }
+
+    @Test
+    public void testNotifiesSuccessfulInsertions() {
+        // Replacing an empty list with a non-empty one is always an insertion.
+        assertThat(mIntegerList.size(), is(0));
+        mIntegerList.set(new Integer[] {333, 88888888, 22});
+        verify(mObserver).onItemRangeInserted(mIntegerList, 0, 3);
+        assertThat(mIntegerList.size(), is(3));
+        assertThat(mIntegerList.get(1), is(88888888));
+
+        // Adding Items is always an insertion.
+        mIntegerList.add(55555);
+        verify(mObserver).onItemRangeInserted(mIntegerList, 3, 1);
+        assertThat(mIntegerList.size(), is(4));
+        assertThat(mIntegerList.get(3), is(55555));
+    }
+
+    @Test
+    public void testModelNotifiesSuccessfulRemoval() {
+        Integer eightEights = 88888888;
+        mIntegerList.set(new Integer[] {333, eightEights, 22});
+        assertThat(mIntegerList.size(), is(3));
+
+        // Removing any item by instance is always a removal.
+        mIntegerList.remove(eightEights);
+        verify(mObserver).onItemRangeRemoved(mIntegerList, 1, 1);
+
+        // Setting an empty list is a removal of all items.
+        mIntegerList.set(new Integer[] {});
+        verify(mObserver).onItemRangeRemoved(mIntegerList, 0, 2);
+    }
+
+    @Test
+    public void testModelNotifiesSuccessfulMove() {
+        Integer eightEights = 88888888;
+        mIntegerList.set(new Integer[] {333, eightEights, 22});
+        assertThat(mIntegerList.size(), is(3));
+
+        // Moving any item forward is a move.
+        mIntegerList.move(1, 0);
+        verify(mObserver).onItemMoved(mIntegerList, 1, 0);
+        assertThat(mIntegerList.get(0), is(eightEights));
+        assertThat(mIntegerList.get(1), is(333));
+        assertThat(mIntegerList.get(2), is(22));
+        mIntegerList.set(new Integer[] {333, eightEights, 22});
+
+        // Moving any item backward is a move.
+        mIntegerList.move(1, 2);
+        verify(mObserver).onItemMoved(mIntegerList, 1, 2);
+        assertThat(mIntegerList.get(0), is(333));
+        assertThat(mIntegerList.get(1), is(22));
+        assertThat(mIntegerList.get(2), is(eightEights));
+    }
+
+    @Test
+    public void testModelNotifiesReplacedDataAndRemoval() {
+        // The initial setting is an insertion.
+        mIntegerList.set(new Integer[] {333, 88888888, 22});
+        verify(mObserver).onItemRangeInserted(mIntegerList, 0, 3);
+
+        // Setting a smaller number of items is a removal and a change.
+        mIntegerList.set(new Integer[] {4444, 22});
+        verify(mObserver).onItemRangeChanged(mIntegerList, 0, 2, null);
+        verify(mObserver).onItemRangeRemoved(mIntegerList, 2, 1);
+    }
+
+    @Test
+    public void testModelNotifiesReplacedDataAndInsertion() {
+        // The initial setting is an insertion.
+        mIntegerList.set(new Integer[] {1234, 56});
+        verify(mObserver).onItemRangeInserted(mIntegerList, 0, 2);
+
+        // Setting a larger number of items is an insertion and a change.
+        mIntegerList.set(new Integer[] {4444, 22, 1, 666666});
+        verify(mObserver).onItemRangeChanged(mIntegerList, 0, 2, null);
+        verify(mObserver).onItemRangeInserted(mIntegerList, 2, 2);
+
+        // Setting empty data is a removal.
+        mIntegerList.set(new Integer[] {});
+        verify(mObserver).onItemRangeRemoved(mIntegerList, 0, 4);
+
+        // Replacing an empty list with another empty list is a no-op.
+        mIntegerList.set(new Integer[] {});
+        verifyNoMoreInteractions(mObserver);
+    }
+}
\ No newline at end of file