Blink handle selection handle visibility

content/browser/renderer_host/input/touch_selection_controller_client_aura.cc

 // Copyright 2015 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.
 
 #include "content/browser/renderer_host/input/touch_selection_controller_client_aura.h"
 
 #include "base/macros.h"
 #include "content/browser/renderer_host/render_widget_host_delegate.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_view_aura.h"
@@ skipping 139 matching lines @@
 
 void TouchSelectionControllerClientAura::OnScrollCompleted() {
   scroll_in_progress_ = false;
   rwhva_->selection_controller()->SetTemporarilyHidden(false);
   UpdateQuickMenu();
 }
 
 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());
     quick_menu_requested_ = true;
     UpdateQuickMenu();
     return true;
   }
   rwhva_->selection_controller()->HideAndDisallowShowingAutomatically();
   return false;
 }
@@ skipping 122 matching lines @@
       UpdateQuickMenu();
       break;
     case ui::SELECTION_HANDLES_MOVED:
     case ui::INSERTION_HANDLE_MOVED:
       UpdateQuickMenu();
       break;
     case ui::INSERTION_HANDLE_TAPPED:
       quick_menu_requested_ = !quick_menu_requested_;
       UpdateQuickMenu();
       break;
-    case ui::SELECTION_ESTABLISHED:
-    case ui::SELECTION_DISSOLVED:
-      break;
   };
 }
 
 std::unique_ptr<ui::TouchHandleDrawable>
 TouchSelectionControllerClientAura::CreateDrawable() {
   return std::unique_ptr<ui::TouchHandleDrawable>(
       new ui::TouchHandleDrawableAura(rwhva_->GetNativeView()));
 }
 
 bool TouchSelectionControllerClientAura::IsCommandIdEnabled(
@@ skipping 54 matching lines @@
                                          ui::MENU_SOURCE_TOUCH_EDIT_MENU,
                                          gfx::ToRoundedPoint(anchor_point)));
 
   // Hide selection handles after getting rect-between-bounds from touch
   // selection controller; otherwise, rect would be empty and the above
   // calculations would be invalid.
   rwhva_->selection_controller()->HideAndDisallowShowingAutomatically();
 }
 
 }  // namespace content