Make long press & release to trigger context menu on tab

chrome/browser/ui/views/tabs/tab_strip.cc

Index: chrome/browser/ui/views/tabs/tab_strip.cc
diff --git a/chrome/browser/ui/views/tabs/tab_strip.cc b/chrome/browser/ui/views/tabs/tab_strip.cc
index 6e76a3d6c580e333fee675f49c0d36c3698ade90..bc4f23e966f6a8caf899bae15a91b1dc72c78ae9 100644
--- a/chrome/browser/ui/views/tabs/tab_strip.cc
+++ b/chrome/browser/ui/views/tabs/tab_strip.cc
@@ -587,6 +587,8 @@ TabStrip::TabStrip(TabStripController* controller)
       layout_type_(TAB_STRIP_LAYOUT_SHRINK),
       adjust_layout_(false),
       reset_to_shrink_on_exit_(false),
+      was_dragging_(false),
+      had_gesture_long_press_(false),
       mouse_move_count_(0),
       immersive_style_(false) {
   Init();
@@ -1500,15 +1502,30 @@ void TabStrip::OnGestureEvent(ui::GestureEvent* event) {
         SetLayoutType(TAB_STRIP_LAYOUT_STACKED, true);
         controller_->LayoutTypeMaybeChanged();
       }
+      // If there was long press gesture and no dragging,
+      // show the context menu at finger lift.
+      if (!was_dragging_ && had_gesture_long_press_) {
+        Tab* tab = FindTabForEvent(last_long_press_location_);
+        if (tab) {
+          gfx::Point local_point(last_long_press_location_);
+          ConvertPointToScreen(this, &local_point);
+          ShowContextMenuForTab(tab, local_point);

[sadrul, 2013/03/19 22:17:31]

The finger may have moved a significant distance after the long-press event. I
don't think we should show the context menu in such scenario.

You should should the context menu on GESTURE_LONG_TAP gesture instead. (so you
won't need any of the new states, i.e. was_dragging_, had_gesture_long_press_ or
last_long_press_location)

[Yufeng Shen (Slow to review), 2013/03/19 23:37:38]

If the finger has moved a significant distance, then it would have triggered
ui::ET_GESTURE_SCROLL_UPDATE and marked as dragging I think.
 
The tricky part is that we currently use GESTURE_LONG_PRESS
as switch between dragging and scrolling mode on the tab strip (see issue
161634). So showing context menu after long press but before finger lift would
somehow make the whole thing even more confusing.

+        }
+      }
+      had_gesture_long_press_ = false;
+      was_dragging_ = false;
       break;
 
     case ui::ET_GESTURE_LONG_PRESS:
       if (drag_controller_.get())
         drag_controller_->SetMoveBehavior(TabDragController::REORDER);
+      had_gesture_long_press_ = true;
+      last_long_press_location_ = event->location();
       break;
 
     case ui::ET_GESTURE_SCROLL_UPDATE:
       ContinueDrag(this, *event);
+      was_dragging_ = true;
       break;
 
     case ui::ET_GESTURE_BEGIN:
@@ -2493,15 +2510,23 @@ int TabStrip::GetStartXForNormalTabs() const {
 }
 
 Tab* TabStrip::FindTabForEvent(const gfx::Point& point) {
-  DCHECK(touch_layout_.get());
-  int active_tab_index = touch_layout_->active_index();
   Tab* tab = NULL;
-  if (active_tab_index != -1) {
-    tab = FindTabForEventFrom(point, active_tab_index, -1);
-    if (!tab)
-      tab = FindTabForEventFrom(point, active_tab_index + 1, 1);
-  } else if (tab_count()) {
-    tab = FindTabForEventFrom(point, 0, 1);
+  if (touch_layout_.get()) {
+    int active_tab_index = touch_layout_->active_index();
+    if (active_tab_index != -1) {
+      tab = FindTabForEventFrom(point, active_tab_index, -1);
+      if (!tab)
+        tab = FindTabForEventFrom(point, active_tab_index + 1, 1);
+    } else if (tab_count()) {
+      tab = FindTabForEventFrom(point, 0, 1);
+    }
+  } else {
+    for (int i = 0; i < tab_count(); ++i) {
+      if (IsPointInTab(tab_at(i), point)) {
+        tab = tab_at(i);
+        break;
+      }
+    }
   }
   return tab;
 }