[refactor] - Rename and Move WebWidget::CaretOrSelectionRange to WebInputMethodController::GetSelec…

content/browser/renderer_host/input/touch_selection_controller_client_child_frame.cc

Index: content/browser/renderer_host/input/touch_selection_controller_client_child_frame.cc
diff --git a/content/browser/renderer_host/input/touch_selection_controller_client_child_frame.cc b/content/browser/renderer_host/input/touch_selection_controller_client_child_frame.cc
index 5a40845ad3bb6352d397bec2e5122412ae2b9c6b..7a1325b0f4ffbf3fafd5ce3b05cc3f5cedb2e3e3 100644
--- a/content/browser/renderer_host/input/touch_selection_controller_client_child_frame.cc
+++ b/content/browser/renderer_host/input/touch_selection_controller_client_child_frame.cc
@@ -124,19 +124,10 @@ bool TouchSelectionControllerClientChildFrame::IsCommandIdEnabled(
     int command_id) const {
   bool editable = rwhv_->GetTextInputType() != ui::TEXT_INPUT_TYPE_NONE;
   bool readable = rwhv_->GetTextInputType() != ui::TEXT_INPUT_TYPE_PASSWORD;
-  // TODO(wjmaclean): The test for has_selection should be changed to
-  //
-  //  rwhv_->GetSelectionRange(&selection_range);
-  //  bool has_selection = !selection_range.is_empty();
-  //
-  // like in TouchSelectionControllerClientAura. Unfortunately this fails here
-  // due to https://crbug.com/723790, which means that the first text
-  // selected in an oopif subframe when it acquires focus will fail to send
-  // a FrameHostMsg_SelectionChanged, meaning the TextInputManager won't
-  // know about the new selection.
-  bool has_selection = selection_start_.type() != gfx::SelectionBound::EMPTY &&
-                       selection_end_.type() != gfx::SelectionBound::EMPTY &&
-                       selection_start_ != selection_end_;
+
+  gfx::Range selection_range;
+  bool has_selection =
+      rwhv_->GetSelectionRange(&selection_range) && !selection_range.is_empty();

[dcheng, 2017/06/20 09:05:47]

Can you help me understand what the difference between selection_start /
selection_end_ and RWHV::GetSelectionRange() here is?

[EhsanK, 2017/07/10 15:38:29]

They come from two different IPCs. 
  * |selection_start_| and |selection_end_|: gfx::SelectionBounds which I
beleive represents the whereabouts of the highlight text. This does not include
any text.

  * GetSelectionRange: This is based on text selection and does not include any
geometry data. Essentially, when FOO is highlighted/selected in a page, we send
a substring from page including FOO (preferrably 100 chars before and after it)
through FrameHostMsg_SelectionChanged IPC. This IPC includes the following data:
  1- text: which is the whole text (could be as much as 200 + length(FOO))
  2- range: the selection range in the text (indices are wrt the beginning of
the page)
  3- offset: from the beginning of the page.

So basically, GetSelectionRange(&range) should return a range with nonzero
length when we have highlighted text.

+wjmaclean@ for more input.

[wjmaclean, 2017/07/10 18:36:07]

If you compare the pre- vs. post-patch code here, and compare with the
equivalent code in TouchSelectionControllerClientAura, ekaramad@'s change brings
the code in line with what the top-level Aura-client is doing. I would have
written the child client the same way, except there's an issue where the
information for GetSelectionRange() in a child has different timing w.r.t. the
mainframe's version (and it's a different IPC from the OnSelectionUpdate one).

I guess Im sort of repeating what ekaramad@ has already said.

   switch (command_id) {
     case IDS_APP_CUT:
       return editable && readable && has_selection;