Track text selection on the browser side (Mac)

content/browser/renderer_host/text_input_manager.cc

Index: content/browser/renderer_host/text_input_manager.cc
diff --git a/content/browser/renderer_host/text_input_manager.cc b/content/browser/renderer_host/text_input_manager.cc
index 3207b9218d82cffcd159782e786d8e237f3d88df..b1fb228bbc48daa69d054c25fde33db28d8cf100 100644
--- a/content/browser/renderer_host/text_input_manager.cc
+++ b/content/browser/renderer_host/text_input_manager.cc
@@ -85,6 +85,28 @@ const TextInputManager::TextSelection* TextInputManager::GetTextSelection(
   return !!view ? &text_selection_map_.at(view) : nullptr;
 }
 
+bool TextInputManager::GetSelectedText(RenderWidgetHostViewBase* view,
+                                       base::string16* selected_text) const {
+  const TextSelection* selection = GetTextSelection(view);
+  if (!selection)
+    return false;
+
+  if (selection->text.empty() || selection->range.is_empty())
+    return false;
+
+  selected_text->clear();
+  size_t pos = selection->range.GetMin() - selection->offset;
+  size_t n = selection->range.length();
+  DCHECK(pos + n <= selection->text.length())

[Avi (use Gerrit), 2016/08/12 15:53:10]

DCHECK_LE

[EhsanK, 2016/08/16 13:36:45]

Done.

+      << "The text can not fully cover range.";
+  if (pos >= selection->text.length()) {

[erikchen, 2016/08/12 19:33:43]

Is it ever possible for an instance of TextSelection to contain invalid
parameters? 

If so, we shouldn't have DCHECKs.
If not, we shouldn't have a conditional.

The code pattern:
"""
if (...) {
  NOTREACHED()
}
"""
is a red flag.

[EhsanK, 2016/08/16 13:36:45]

I am not sure but the selection information are coming from the renderer side,
so they could be wrong maybe?

I cannot DCHECK and not return early since it might throw exception in
text.substr() later on. So, if DCHECK is incorrect here, I think I should just
early return if the range values are invalid. WDYT?

[erikchen, 2016/08/16 16:36:22]

Yes, early return is appropriate. Short of memory corruption or someone breaking
this class in the future, nothing should make a DCHECK be hit. You might be want
a LOG(WARNING) instead? 

[EhsanK, 2016/08/17 06:20:50]

I think the DCHECK above should also change then. I did replace both checks with
early return and logs.

+    NOTREACHED() << "The text can not cover range.";
+    return false;
+  }
+  selected_text->append(selection->text.substr(pos, n));
+  return true;
+}
+
 void TextInputManager::UpdateTextInputState(
     RenderWidgetHostViewBase* view,
     const TextInputState& text_input_state) {