[Android Bulk Tab Edit] Add updated longpress entry point for TabSele…

…ctionEditor

The proposed implementation of the longpress entry point for TabSelectionEditorV2. If a a tab is held and a swipe, move or group/ungroup does not occur, the logic will trigger the longpress action on the held tab, which brings up the tab selection editor.

This is a reland attempt with a selection state fix for https://chromium-review.googlesource.com/c/chromium/src/+/3905682.

In the previous CL, a MOTION_UP click event may be processed if the held tab had no movement, leading to selection state issues as the MOTION_UP event propagates down to the tab switcher recycler view, where consuming it leads to a click that changes selection state. The current CL update will detect if a MOTION_UP event may be consumed if it came as a result from the above conditions and ensure that if those conditions are met, the erroneous click action will be blocked.

Bug: 1343374
Change-Id: I91f137c835d43c9598f592d5502a7bbf4bee4fc8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4047909
Reviewed-by: Mei Liang <meiliang@chromium.org>
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096338}

chrome/android/features/start_surface/javatests/src/org/chromium/chrome/features/start_surface/TabSwitcherAndStartSurfaceLayoutTest.java

@@ -9,6 +9,7 @@
 
 import static androidx.test.espresso.Espresso.onView;
 import static androidx.test.espresso.action.ViewActions.click;
+import static androidx.test.espresso.action.ViewActions.longClick;
 import static androidx.test.espresso.assertion.ViewAssertions.doesNotExist;
 import static androidx.test.espresso.assertion.ViewAssertions.matches;
 import static androidx.test.espresso.contrib.RecyclerViewActions.actionOnItemAtPosition;
@@ -1820,6 +1821,87 @@ public void testUndoGroupClosureInTabSwitcher() {
         verifyTabSwitcherCardCount(cta, 1);
     }
 
+    @Test
+    @EnableFeatures({ChromeFeatureList.TAB_SELECTION_EDITOR_V2})
+    @MediumTest
+    public void testLongPressTab_entryInTabSwitcher_verifyNoSelectionOccurs() {
+        TabUiFeatureUtilities.ENABLE_TAB_SELECTION_EDITOR_V2_LONGPRESS_ENTRY.setForTesting(true);
+
+        final ChromeTabbedActivity cta = mActivityTestRule.getActivity();
+        createTabs(cta, false, 2);
+        enterTabSwitcher(cta);
+        verifyTabSwitcherCardCount(cta, 2);
+
+        // LongPress entry to TabSelectionEditor.
+        onView(tabSwitcherViewMatcher())
+                .perform(RecyclerViewActions.actionOnItemAtPosition(0, longClick()));
+
+        TabSelectionEditorTestingRobot mSelectionEditorRobot = new TabSelectionEditorTestingRobot();
+        mSelectionEditorRobot.resultRobot.verifyTabSelectionEditorIsVisible();
+
+        // Verify no selection action occurred to switch the selected tab in the tab model
+        Criteria.checkThat(
+                mActivityTestRule.getActivity().getCurrentTabModel().index(), Matchers.is(1));
+
+        TabUiFeatureUtilities.ENABLE_TAB_SELECTION_EDITOR_V2_LONGPRESS_ENTRY.setForTesting(false);
+    }
+
+    @Test
+    @EnableFeatures({ChromeFeatureList.TAB_SELECTION_EDITOR_V2})
+    @MediumTest
+    public void testLongPressTabGroup_entryInTabSwitcher() {
+        TabUiFeatureUtilities.ENABLE_TAB_SELECTION_EDITOR_V2_LONGPRESS_ENTRY.setForTesting(true);
+
+        final ChromeTabbedActivity cta = mActivityTestRule.getActivity();
+        createTabs(cta, false, 2);
+        enterTabSwitcher(cta);
+        verifyTabSwitcherCardCount(cta, 2);
+
+        // Create a tab group.
+        mergeAllNormalTabsToAGroup(cta);
+        verifyTabSwitcherCardCount(cta, 1);
+
+        // LongPress entry to TabSelectionEditor.
+        onView(tabSwitcherViewMatcher())
+                .perform(RecyclerViewActions.actionOnItemAtPosition(0, longClick()));
+
+        TabSelectionEditorTestingRobot mSelectionEditorRobot = new TabSelectionEditorTestingRobot();
+        mSelectionEditorRobot.resultRobot.verifyTabSelectionEditorIsVisible();
+
+        TabUiFeatureUtilities.ENABLE_TAB_SELECTION_EDITOR_V2_LONGPRESS_ENTRY.setForTesting(false);
+    }
+
+    @Test
+    @EnableFeatures({ChromeFeatureList.TAB_SELECTION_EDITOR_V2})
+    @MediumTest
+    public void testLongPressTab_verifyPostLongPressClickNoSelectionEditor() {
+        TabUiFeatureUtilities.ENABLE_TAB_SELECTION_EDITOR_V2_LONGPRESS_ENTRY.setForTesting(true);
+
+        final ChromeTabbedActivity cta = mActivityTestRule.getActivity();
+        createTabs(cta, false, 2);
+        enterTabSwitcher(cta);
+        verifyTabSwitcherCardCount(cta, 2);
+
+        // LongPress entry to TabSelectionEditor.
+        onView(tabSwitcherViewMatcher())
+                .perform(RecyclerViewActions.actionOnItemAtPosition(0, longClick()));
+
+        TabSelectionEditorTestingRobot mSelectionEditorRobot = new TabSelectionEditorTestingRobot();
+        mSelectionEditorRobot.resultRobot.verifyTabSelectionEditorIsVisible();
+        mSelectionEditorRobot.actionRobot.clickItemAtAdapterPosition(0);
+        mSelectionEditorRobot.resultRobot.verifyItemSelectedAtAdapterPosition(0);
+        Espresso.pressBack();
+
+        onView(tabSwitcherViewMatcher())
+                .perform(RecyclerViewActions.actionOnItemAtPosition(0, click()));
+        // Check the selected tab in the tab model switches from the second tab to the first to
+        // verify clicking the tab worked.
+        Criteria.checkThat(
+                mActivityTestRule.getActivity().getCurrentTabModel().index(), Matchers.is(0));
+
+        TabUiFeatureUtilities.ENABLE_TAB_SELECTION_EDITOR_V2_LONGPRESS_ENTRY.setForTesting(false);
+    }
+
     private TabSwitcher.TabListDelegate getTabListDelegateFromUIThread() {
         AtomicReference<TabSwitcher.TabListDelegate> tabListDelegate = new AtomicReference<>();
         TestThreadUtils.runOnUiThreadBlocking(

chrome/android/features/tab_ui/java/res/values/dimens.xml

@@ -81,5 +81,6 @@ found in the LICENSE file.
 
     <!-- Dimens for TabSelectionEditorV2 -->
     <dimen name="tab_selection_editor_action_view_padding">14dp</dimen>
+    <dimen name="tab_selection_editor_longpress_entry_threshold">20dp</dimen>
     <dimen name="tab_selection_editor_share_sheet_preview_thumbnail_padding">10dp</dimen>
 </resources>

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

@@ -101,7 +101,7 @@ public class TabGridDialogCoordinator implements TabGridDialogMediator.DialogCon
                     },
                     null, false, gridCardOnClickListenerProvider,
                     mMediator.getTabGridDialogHandler(), TabProperties.UiType.CLOSABLE, null, null,
-                    containerView, false, mComponentName, rootView, null);
+                    containerView, false, mComponentName, rootView, null, mMediator);
             TabListRecyclerView recyclerView = mTabListCoordinator.getContainerView();
 
             TabGroupUiToolbarView toolbarView =

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

@@ -63,7 +63,8 @@
  * for dialog show/hide.
  */
 public class TabGridDialogMediator
-        implements SnackbarManager.SnackbarController, TabGridDialogView.VisibilityListener {
+        implements SnackbarManager.SnackbarController, TabGridDialogView.VisibilityListener,
+                   TabGridItemTouchHelperCallback.OnLongPressTabItemEventListener {
     /**
      * Defines an interface for a {@link TabGridDialogMediator} to control dialog.
      */
@@ -304,15 +305,7 @@ public void initWithNative(
                     mModel.set(TabGridPanelProperties.IS_KEYBOARD_VISIBLE, false);
                 }
                 mModel.set(TabGridPanelProperties.IS_TITLE_TEXT_FOCUSED, false);
-                List<Tab> tabs = getRelatedTabs(mCurrentTabId);
-                // Setup dialog selection editor.
-                setupDialogSelectionEditor();
-                if (mTabSelectionEditorControllerSupplier.get() != null) {
-                    boolean v2Enabled =
-                            TabUiFeatureUtilities.isTabSelectionEditorV2Enabled(mContext);
-                    mTabSelectionEditorControllerSupplier.get().show(tabs,
-                            /*preSelectedTabCount=*/0,
-                            v2Enabled ? mRecyclerViewPositionSupplier.get() : null);
+                if (setupAndShowTabSelectionEditorV2(mCurrentTabId)) {
                     TabUiMetricsHelper.recordSelectionEditorOpenMetrics(
                             TabSelectionEditorOpenMetricGroups.OPEN_FROM_DIALOG, mContext);
                 }
@@ -762,6 +755,26 @@ public void onDismissNoAction(Object actionData) {
         }
     }
 
+    // OnLongPressTabItemEventListener implementation
+    @Override
+    public void onLongPressEvent(int tabId) {
+        if (setupAndShowTabSelectionEditorV2(tabId)) {
+            RecordUserAction.record("TabMultiSelectV2.OpenLongPressInDialog");
+        }
+    }
+
+    private boolean setupAndShowTabSelectionEditorV2(int currentTabId) {
+        if (mTabSelectionEditorControllerSupplier == null) return false;
+
+        List<Tab> tabs = getRelatedTabs(currentTabId);
+        // Setup dialog selection editor.
+        setupDialogSelectionEditor();
+        boolean v2Enabled = TabUiFeatureUtilities.isTabSelectionEditorV2Enabled(mContext);
+        mTabSelectionEditorControllerSupplier.get().show(tabs,
+                /*preSelectedTabCount=*/0, v2Enabled ? mRecyclerViewPositionSupplier.get() : null);
+        return true;
+    }
+
     /**
      * A handler that handles TabGridDialog related changes originated from {@link TabListMediator}
      * and {@link TabGridItemTouchHelperCallback}.

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

@@ -14,6 +14,7 @@
 import android.view.View;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 import androidx.recyclerview.widget.ItemTouchHelper;
 import androidx.recyclerview.widget.RecyclerView;
@@ -43,18 +44,37 @@
  * related actions in grid related layouts.
  */
 public class TabGridItemTouchHelperCallback extends ItemTouchHelper.SimpleCallback {
+    /**
+     * An interface to observe the longpress event triggered on a tab card item.
+     */
+    interface OnLongPressTabItemEventListener {
+        /**
+         * Notify the observers that the longpress event on the tab has triggered.
+         * @param tabId the id of the current tab that is being selected.
+         */
+        void onLongPressEvent(int tabId);
+    }
 
     private final TabListModel mModel;
     private final TabModelSelector mTabModelSelector;
     private final TabListMediator.TabActionListener mTabClosedListener;
     private final String mComponentName;
     private final TabListMediator.TabGridDialogHandler mTabGridDialogHandler;
     private final @TabListMode int mMode;
+    private final OnLongPressTabItemEventListener mOnLongPressTabItemEventListener;
+    private final int mLongPressDpThreshold;
     private float mSwipeToDismissThreshold;
     private float mMergeThreshold;
     private float mUngroupThreshold;
+    // A bool to track whether an action such as swiping, group/ungroup and drag past a certain
+    // threshold was attempted. This can determine if a longpress on the tab is the objective.
+    private boolean mActionAttempted;
+    // A bool to track whether any action that is not a pure longpress hold-no-drag, was started.
+    // This can determine if an unwanted following click from a pure longpress must be blocked.
+    private boolean mActionStarted;
     private boolean mActionsOnAllRelatedTabs;
     private boolean mIsSwipingToDismiss;
+    private boolean mShouldBlockAction;
     private int mDragFlags;
     private int mSelectedTabIndex = TabModel.INVALID_TAB_INDEX;
     private int mHoveredTabIndex = TabModel.INVALID_TAB_INDEX;
@@ -67,7 +87,8 @@ public class TabGridItemTouchHelperCallback extends ItemTouchHelper.SimpleCallba
     public TabGridItemTouchHelperCallback(Context context, TabListModel tabListModel,
             TabModelSelector tabModelSelector, TabActionListener tabClosedListener,
             TabGridDialogHandler tabGridDialogHandler, String componentName,
-            boolean actionsOnAllRelatedTabs, @TabListMode int mode) {
+            boolean actionsOnAllRelatedTabs, @TabListMode int mode,
+            @Nullable OnLongPressTabItemEventListener onLongPressTabItemEventListener) {
         super(0, 0);
         mModel = tabListModel;
         mTabModelSelector = tabModelSelector;
@@ -77,6 +98,9 @@ public TabGridItemTouchHelperCallback(Context context, TabListModel tabListModel
         mTabGridDialogHandler = tabGridDialogHandler;
         mContext = context;
         mMode = mode;
+        mOnLongPressTabItemEventListener = onLongPressTabItemEventListener;
+        mLongPressDpThreshold = context.getResources().getDimensionPixelSize(
+                R.dimen.tab_selection_editor_longpress_entry_threshold);
     }
 
     /**
@@ -161,6 +185,7 @@ public boolean onMove(RecyclerView recyclerView, RecyclerView.ViewHolder fromVie
             ((TabGroupModelFilter) filter).moveRelatedTabs(currentTabId, newIndex);
         }
         RecordUserAction.record("TabGrid.Drag.Reordered." + mComponentName);
+        mActionAttempted = true;
         return true;
     }
 
@@ -181,10 +206,12 @@ public void onSwiped(RecyclerView.ViewHolder viewHolder, int i) {
             viewHolder.itemView.findViewById(R.id.close_button).performClick();
             // TODO(crbug.com/1004570): UserAction swipe to dismiss.
         }
+        mActionAttempted = true;
     }
 
     @Override
     public void onSelectedChanged(RecyclerView.ViewHolder viewHolder, int actionState) {
+        super.onSelectedChanged(viewHolder, actionState);
         if (actionState == ItemTouchHelper.ACTION_STATE_DRAG) {
             mSelectedTabIndex = viewHolder.getAdapterPosition();
             mModel.updateSelectedTabForMergeToGroup(mSelectedTabIndex, true);
@@ -211,6 +238,7 @@ public void onSelectedChanged(RecyclerView.ViewHolder viewHolder, int actionStat
                             mModel.getTabCardCountsBefore(mHoveredTabIndex));
                     mRecyclerView.getLayoutManager().removeView(selectedItemView);
                 }
+                mActionAttempted = true;
             } else {
                 mModel.updateSelectedTabForMergeToGroup(mSelectedTabIndex, false);
             }
@@ -220,6 +248,7 @@ public void onSelectedChanged(RecyclerView.ViewHolder viewHolder, int actionStat
                                 ? mHoveredTabIndex
                                 : mModel.getTabIndexBefore(mHoveredTabIndex),
                         false);
+                mActionAttempted = true;
             }
             if (mUnGroupTabIndex != TabModel.INVALID_TAB_INDEX) {
                 TabGroupModelFilter filter =
@@ -238,6 +267,41 @@ public void onSelectedChanged(RecyclerView.ViewHolder viewHolder, int actionStat
                     }
                     RecordUserAction.record("TabGrid.Drag.RemoveFromGroup." + mComponentName);
                 }
+                mActionAttempted = true;
+            }
+
+            // The following is the entry point for the longpress action for TabSelectionEditorV2.
+            // If the conditions mentioned below are avoided, the block will trigger and perform a
+            // longpress action on the held tab, bringing up the selection editor interface.
+            //
+            // This block will not trigger if:
+            //      a swipe was started but unfinished as mSelectedTabIndex may not be set.
+            //      a swipe, move or group/ungroup happens.
+            //      a tab is moved beyond a minimum distance from its original location.
+            //
+            // An edge case exists where on longpress, if the held tab is not dragged (no movement
+            // occurs), the MOTION_UP event on release will not be intercepted by the below attached
+            // onLongPressTabItemEventListener. After processing the longpress action, the MOTION_UP
+            // event will propagate down to the subsequent recyclerViews and be consumed there,
+            // resulting in a click on the tab grid card. The unwanted click behaviour will be
+            // blocked by the logic below if the conditions are met.
+            if (mOnLongPressTabItemEventListener != null
+                    && (mSelectedTabIndex != TabModel.INVALID_TAB_INDEX && !mActionAttempted
+                            && mModel.get(mSelectedTabIndex).model.get(CARD_TYPE) == TAB
+                            && TabUiFeatureUtilities.ENABLE_TAB_SELECTION_EDITOR_V2_LONGPRESS_ENTRY
+                                       .getValue())) {
+                int tabId = mModel.get(mSelectedTabIndex).model.get(TabProperties.TAB_ID);
+                // If the child was ever dragged or swiped do not consume the next action, as the
+                // longpress will resolve safely due to the listener intercepting the DRAG event
+                // and negating any further action. However, if we just release the tab without
+                // starting a swipe or drag then it is possible the longpress instead resolves as a
+                // MOTION_UP click event which leads to tab selection occurring in the selection
+                // editor or resulting in clicking the tab itself. This issue can be avoided by
+                // requesting to block the next action.
+                if (!mActionStarted) {
+                    mShouldBlockAction = true;
+                }
+                mOnLongPressTabItemEventListener.onLongPressEvent(tabId);
             }
             mHoveredTabIndex = TabModel.INVALID_TAB_INDEX;
             mSelectedTabIndex = TabModel.INVALID_TAB_INDEX;
@@ -247,6 +311,8 @@ public void onSelectedChanged(RecyclerView.ViewHolder viewHolder, int actionStat
                         TabGridDialogView.UngroupBarStatus.HIDE);
             }
         }
+        mActionStarted = false;
+        mActionAttempted = false;
     }
 
     private boolean hasTabPropertiesModel(RecyclerView.ViewHolder viewHolder) {
@@ -272,6 +338,9 @@ public void clearView(RecyclerView recyclerView, RecyclerView.ViewHolder viewHol
     public void onChildDraw(Canvas c, RecyclerView recyclerView, RecyclerView.ViewHolder viewHolder,
             float dX, float dY, int actionState, boolean isCurrentlyActive) {
         super.onChildDraw(c, recyclerView, viewHolder, dX, dY, actionState, isCurrentlyActive);
+        if (Math.abs(dX) > 0 || Math.abs(dY) > 0) {
+            mActionStarted = true;
+        }
         if (actionState == ItemTouchHelper.ACTION_STATE_SWIPE) {
             float alpha = Math.max(0.2f, 1f - 0.8f * Math.abs(dX) / mSwipeToDismissThreshold);
 
@@ -298,6 +367,9 @@ public void onChildDraw(Canvas c, RecyclerView recyclerView, RecyclerView.ViewHo
             mIsSwipingToDismiss = isOverThreshold;
             return;
         }
+        if (dX * dX + dY * dY > mLongPressDpThreshold * mLongPressDpThreshold) {
+            mActionAttempted = true;
+        }
         mCurrentActionState = actionState;
         if (actionState == ItemTouchHelper.ACTION_STATE_DRAG && mActionsOnAllRelatedTabs) {
             if (!TabUiFeatureUtilities.isTabGroupsAndroidEnabled(mContext)) return;
@@ -358,6 +430,18 @@ private void onTabMergeToGroup(int selectedCardIndex, int hoveredCardIndex) {
         tracker.notifyEvent(EventConstants.TAB_DRAG_AND_DROP_TO_GROUP);
     }
 
+    /*
+     * Returns whether or not a touch action should be blocked on an item accessed from
+     * the TabListCoordinator. The bit is always defaulted to false and reset to that
+     * value after shouldBlockAction() is called. It is used primarily to prevent a
+     * secondary touch event from occurring on a longpress event on a tab grid item.
+     */
+    boolean shouldBlockAction() {
+        boolean out = mShouldBlockAction;
+        mShouldBlockAction = false;
+        return out;
+    }
+
     @VisibleForTesting
     void setActionsOnAllRelatedTabsForTesting(boolean flag) {
         mActionsOnAllRelatedTabs = flag;

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

@@ -144,7 +144,7 @@ public void initializeWithNative(Activity activity,
             mTabStripCoordinator = new TabListCoordinator(TabListCoordinator.TabListMode.STRIP,
                     mContext, mTabModelSelector, null, null, false, null, null,
                     TabProperties.UiType.STRIP, null, null, mTabListContainerView, true,
-                    COMPONENT_NAME, mRootView, onModelTokenChange);
+                    COMPONENT_NAME, mRootView, onModelTokenChange, null);
             mTabStripCoordinator.initWithNative(mDynamicResourceLoaderSupplier.get());
 
             mModelChangeProcessor = PropertyModelChangeProcessor.create(mModel,