[MarkersIntersectingRange #4] Refactor ContextMenuClient::SelectMisspellingAsync()

third_party/WebKit/Source/core/page/ContextMenuClient.cpp

 /*
  * Copyright (C) 2009, 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 91 matching lines @@
   }
   return WebURL();
 }
 
 static bool IsWhiteSpaceOrPunctuation(UChar c) {
   return IsSpaceOrNewline(c) || WTF::Unicode::IsPunct(c);
 }
 
 static String SelectMisspellingAsync(LocalFrame* selected_frame,
                                      String& description) {
+  const Optional<std::pair<Node*, SpellCheckMarker*>>& node_and_marker =
+      selected_frame->GetSpellChecker().GetSpellCheckMarkerUnderSelection();
+  if (!node_and_marker)

[yosin_UTC9, 2017/07/20 01:19:07]

How about using |pair.first| which is Node*, as no spelling marker?
If so, we could write:

if (!node_and_mark.first)
  return;

[rlanday, 2017/07/21 00:16:04]

I put up a separate CL for this:
https://chromium-review.googlesource.com/c/580688/

+    return String();
+
+  Node* const marker_node = node_and_marker.value().first;
+  const SpellCheckMarker* const marker = node_and_marker.value().second;
+  description = marker->Description();
+
+  Range* const marker_range =
+      Range::Create(*selected_frame->GetDocument(), marker_node,

[yosin_UTC9, 2017/07/20 01:19:07]

Could you avoid to use temporary Range make this code Oilpan friendly?
We can use EpehemralRange and PlainText() to work as Range::GetTExt()

String Range::GetText() const {
  DCHECK(!owner_document_->NeedsLayoutTreeUpdate());
  return PlainText(EphemeralRange(this),
                   TextIteratorBehavior::Builder()
                       .SetEmitsObjectReplacementCharacter(true)
                       .Build());
}

+                    marker->StartOffset(), marker_node, marker->EndOffset());
+
   VisibleSelection selection =
       selected_frame->Selection().ComputeVisibleSelectionInDOMTree();
-  if (selection.IsNone())
-    return String();
-
-  // Caret and range selections always return valid normalized ranges.
+  // Caret and range selections (one of which we must have since we found a
+  // marker) always return valid normalized ranges.
   const EphemeralRange& selection_range =
       selection.ToNormalizedEphemeralRange();
 
-  Node* const selection_start_container =
-      selection_range.StartPosition().ComputeContainerNode();
-  Node* const selection_end_container =
-      selection_range.EndPosition().ComputeContainerNode();
-
-  // We don't currently support the case where a misspelling spans multiple
-  // nodes
-  if (selection_start_container != selection_end_container)
-    return String();
-
-  const unsigned selection_start_offset =
-      selection_range.StartPosition().ComputeOffsetInContainerNode();
-  const unsigned selection_end_offset =
-      selection_range.EndPosition().ComputeOffsetInContainerNode();
-
-  const DocumentMarkerVector& markers_in_node =
-      selected_frame->GetDocument()->Markers().MarkersFor(
-          selection_start_container, DocumentMarker::MisspellingMarkers());
-
-  const auto marker_it =
-      std::find_if(markers_in_node.begin(), markers_in_node.end(),
-                   [=](const DocumentMarker* marker) {
-                     return marker->StartOffset() < selection_end_offset &&
-                            marker->EndOffset() > selection_start_offset;
-                   });
-  if (marker_it == markers_in_node.end())
-    return String();
-
-  const SpellCheckMarker* const found_marker = ToSpellCheckMarker(*marker_it);
-  description = found_marker->Description();
-
-  Range* const marker_range =
-      Range::Create(*selected_frame->GetDocument(), selection_start_container,
-                    found_marker->StartOffset(), selection_start_container,
-                    found_marker->EndOffset());
-
   if (marker_range->GetText().StripWhiteSpace(&IsWhiteSpaceOrPunctuation) !=
       CreateRange(selection_range)
           ->GetText()
           .StripWhiteSpace(&IsWhiteSpaceOrPunctuation))
     return String();
 
   return marker_range->GetText();
 }
 
 // static
@@ skipping 400 matching lines @@
     output_items[i] = sub_items[i];
   sub_menu_items.Swap(output_items);
 }
 
 void ContextMenuClient::PopulateCustomMenuItems(const ContextMenu* default_menu,
                                                 WebContextMenuData* data) {
   PopulateSubMenuItems(default_menu->Items(), data->custom_items);
 }
 
 }  // namespace blink