[MarkersIntersectingRange #3] Optimize SpellChecker::GetSpellCheckMarkerUnderSelection()

third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 815 matching lines @@
                                                                marker_types);
     }
   }
 }
 
 Optional<std::pair<Node*, SpellCheckMarker*>>
 SpellChecker::GetSpellCheckMarkerUnderSelection() {
   const VisibleSelection& selection =
       GetFrame().Selection().ComputeVisibleSelectionInDOMTree();
   if (selection.IsNone())
     return Optional<std::pair<Node*, SpellCheckMarker*>>();

[Xiaocheng, 2017/07/20 21:41:42]

Use |return WTF::nullopt|

 
-  const EphemeralRange& range_to_check = FirstEphemeralRangeOf(selection);
+  // Caret and range selections always return valid normalized ranges.
+  const EphemeralRange& selection_range =
+      selection.ToNormalizedEphemeralRange();

[yosin_UTC9, 2017/07/20 01:12:11]

Could you use |FirstEphemeralRangeOf(selection)| as original?
Since VS::ToNormalizedRange() does lots of thing and we would like to
deprecate it.

 
-  Node* const start_container =
-      range_to_check.StartPosition().ComputeContainerNode();
-  const unsigned start_offset =
-      range_to_check.StartPosition().ComputeOffsetInContainerNode();
-  Node* const end_container =
-      range_to_check.EndPosition().ComputeContainerNode();
-  const unsigned end_offset =
-      range_to_check.EndPosition().ComputeOffsetInContainerNode();
+  Node* const selection_start_container =
+      selection_range.StartPosition().ComputeContainerNode();
+  Node* const selection_end_container =
+      selection_range.EndPosition().ComputeContainerNode();
 
-  for (Node& node : range_to_check.Nodes()) {
-    const DocumentMarkerVector& markers_in_node =
-        GetFrame().GetDocument()->Markers().MarkersFor(
-            &node, DocumentMarker::MisspellingMarkers());
-    for (DocumentMarker* marker : markers_in_node) {
-      if (node == start_container && marker->EndOffset() <= start_offset)
-        continue;
-      if (node == end_container && marker->StartOffset() >= end_offset)
-        continue;
+  // We don't currently support the case where a misspelling spans multiple
+  // nodes. See crbug.com/720065
+  if (selection_start_container != selection_end_container)
+    return Optional<std::pair<Node*, SpellCheckMarker*>>();

[yosin_UTC9, 2017/07/20 01:12:11]

Could you try to write |return {}| to make code simpler?

[Xiaocheng, 2017/07/20 21:41:42]

I prefer WTF::nullopt

 
-      return std::make_pair(&node, &ToSpellCheckMarker(*marker));
-    }
-  }
+  if (!selection_start_container->IsTextNode())
+    return Optional<std::pair<Node*, SpellCheckMarker*>>();
 
-  // No marker found
-  return Optional<std::pair<Node*, SpellCheckMarker*>>();
+  const unsigned selection_start_offset =
+      selection_range.StartPosition().ComputeOffsetInContainerNode();
+  const unsigned selection_end_offset =
+      selection_range.EndPosition().ComputeOffsetInContainerNode();
+
+  DocumentMarker* const marker =
+      GetFrame().GetDocument()->Markers().FirstMarkerIntersectingOffsetRange(
+          ToText(*selection_start_container), selection_start_offset,
+          selection_end_offset, DocumentMarker::MisspellingMarkers());
+  if (!marker)
+    return Optional<std::pair<Node*, SpellCheckMarker*>>();
+
+  return std::make_pair(selection_start_container, ToSpellCheckMarker(marker));
 }
 
 void SpellChecker::ReplaceMisspelledRange(const String& text) {
   const Optional<std::pair<Node*, SpellCheckMarker*>>& node_and_marker =
       GetSpellCheckMarkerUnderSelection();
   if (!node_and_marker)
     return;
 
   Node* const container_node = node_and_marker.value().first;
   const SpellCheckMarker* const marker = node_and_marker.value().second;
@@ skipping 386 matching lines @@
       if (!input.IsFocusedElementInDocument())
         return false;
     }
   }
   HTMLElement* element =
       Traversal<HTMLElement>::FirstAncestorOrSelf(*position.AnchorNode());
   return element && element->IsSpellCheckingEnabled();
 }
 
 }  // namespace blink