Blink handle selection handle visibility

content/browser/renderer_host/input/touch_selection_controller_client_aura.cc

Index: content/browser/renderer_host/input/touch_selection_controller_client_aura.cc
diff --git a/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc b/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc
index 1bf91a5447060e31afbc5159cc7474d592c4993d..16978aeec963f3d2639bb5d9511adc3d3f240cca 100644
--- a/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc
+++ b/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc
@@ -157,7 +157,8 @@ void TouchSelectionControllerClientAura::OnScrollCompleted() {
 bool TouchSelectionControllerClientAura::HandleContextMenu(
     const ContextMenuParams& params) {
   if (params.source_type == ui::MENU_SOURCE_LONG_PRESS &&
-      rwhva_->selection_controller()->insertion_active_or_requested() &&
+      (rwhva_->selection_controller()->active_status() ==
+       ui::TouchSelectionController::INSERTION_ACTIVE) &&

[mohsen, 2016/10/15 03:49:36]

I think this will break long-press on empty textfields. Long press generates a
context menu request from blink which is handled here and the touch selection
menu is shown instead of the regular context menu if touch selection is or
"going to be" active. The "going to be" case happens in cases that the context
menu request arrives before the selection update (which is very likely as the
selection update happens at the frame boundary, but the context menu request
happens immediately). You have removed the "going to be" check here. Maybe, the
proper solution is to not request the context menu in the first place now that
we know in blink side that a handle is going to be shown?

[amaralp, 2016/10/19 04:26:38]

Yeah I think you are right.
I'm not sure I understand what you mean by not requesting the context menu.
Would moving the two DCHECK's (|param.is_editable| and
|params.selection_text.empty()|) into the if condition be a viable workaround?

[mohsen, 2016/10/19 16:08:34]

When user long-presses on a page, blink either selects some text or decides to
show a context menu, for which a request is sent to the browser (browser is
responsible to show the context menu). Here, we are intercepting that request
and sometimes show the touch selection menu instead of the context menu.
Instead, now that handle visibility is determined in blink, blink can probably
be smart enough to detect this case and avoid requesting the context menu in the
first place. I'm not sure I understand your suggestion to move DCHECK's into the
if condition (which if condition?) and how it would help?

       IsQuickMenuAvailable()) {
     DCHECK(params.is_editable);
     DCHECK(params.selection_text.empty());
@@ -300,9 +301,6 @@ void TouchSelectionControllerClientAura::OnSelectionEvent(
       quick_menu_requested_ = !quick_menu_requested_;
       UpdateQuickMenu();
       break;
-    case ui::SELECTION_ESTABLISHED:
-    case ui::SELECTION_DISSOLVED:
-      break;
   };
 }