Refactor DocumentMarkerController::Add*Marker() methods

third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights
  * reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  * (http://www.torchmobile.com/)
  * Copyright (C) Research In Motion Limited 2010. All rights reserved.
@@ skipping 120 matching lines @@
                                                 const String& description) {
   AddSpellCheckMarker(range, DocumentMarker::kGrammar, description);
 }
 
 void DocumentMarkerController::AddSpellCheckMarker(
     const EphemeralRange& range,
     DocumentMarker::MarkerType type,
     const String& description) {
   DCHECK(type == DocumentMarker::kSpelling || type == DocumentMarker::kGrammar)
       << type;
-  // Use a TextIterator to visit the potentially multiple nodes the range
-  // covers.
-  for (TextIterator marked_text(range.StartPosition(), range.EndPosition());
-       !marked_text.AtEnd(); marked_text.Advance()) {
-    AddMarker(marked_text.CurrentContainer(),
-              new DocumentMarker(
-                  type, marked_text.StartOffsetInCurrentContainer(),
-                  marked_text.EndOffsetInCurrentContainer(), description));
-  }
+  AddMarkerInternal(
+      range, [type, &description](int start_offset, int end_offset) {
+        return new DocumentMarker(type, start_offset, end_offset, description);
+      });
 }
 
 void DocumentMarkerController::AddTextMatchMarker(
     const EphemeralRange& range,
     DocumentMarker::MatchStatus match_status) {
   DCHECK(!document_->NeedsLayoutTreeUpdate());
-
-  // Use a TextIterator to visit the potentially multiple nodes the range
-  // covers.
-  for (TextIterator marked_text(range.StartPosition(), range.EndPosition());
-       !marked_text.AtEnd(); marked_text.Advance()) {
-    AddMarker(marked_text.CurrentContainer(),
-              new DocumentMarker(marked_text.StartOffsetInCurrentContainer(),
-                                 marked_text.EndOffsetInCurrentContainer(),
-                                 match_status));
-  }
+  AddMarkerInternal(range, [match_status](int start_offset, int end_offset) {
+    return new DocumentMarker(start_offset, end_offset, match_status);
+  });
   // Don't invalidate tickmarks here. TextFinder invalidates tickmarks using a
   // throttling algorithm. crbug.com/6819.
 }
 
 void DocumentMarkerController::AddCompositionMarker(const EphemeralRange& range,
                                                     Color underline_color,
                                                     bool thick,
                                                     Color background_color) {
   DCHECK(!document_->NeedsLayoutTreeUpdate());
-
-  for (TextIterator marked_text(range.StartPosition(), range.EndPosition());
-       !marked_text.AtEnd(); marked_text.Advance()) {
-    AddMarker(marked_text.CurrentContainer(),
-              new DocumentMarker(marked_text.StartOffsetInCurrentContainer(),
-                                 marked_text.EndOffsetInCurrentContainer(),
-                                 underline_color, thick, background_color));
-  }
+  AddMarkerInternal(range, [underline_color, thick, background_color](
+                               int start_offset, int end_offset) {
+    return new DocumentMarker(start_offset, end_offset, underline_color, thick,
+                              background_color);
+  });
 }
 
 void DocumentMarkerController::PrepareForDestruction() {
   Clear();
 }
 
 void DocumentMarkerController::RemoveMarkers(
     TextIterator& marked_text,
     DocumentMarker::MarkerTypes marker_types) {
   for (; !marked_text.AtEnd(); marked_text.Advance()) {
@@ skipping 10 matching lines @@
 
 void DocumentMarkerController::RemoveMarkersInRange(
     const EphemeralRange& range,
     DocumentMarker::MarkerTypes marker_types) {
   DCHECK(!document_->NeedsLayoutTreeUpdate());
 
   TextIterator marked_text(range.StartPosition(), range.EndPosition());
   DocumentMarkerController::RemoveMarkers(marked_text, marker_types);
 }
 
+void DocumentMarkerController::AddMarkerInternal(
+    const EphemeralRange& range,
+    std::function<DocumentMarker*(int, int)> create_marker_from_offsets) {
+  for (TextIterator marked_text(range.StartPosition(), range.EndPosition());

[yosin_UTC9, 2017/05/31 02:01:13]

FYI: It is nice that we have TextIterator::begin()/end().

+       !marked_text.AtEnd(); marked_text.Advance()) {
+    const int start_offset_in_current_container =
+        marked_text.StartOffsetInCurrentContainer();
+    const int end_offset_in_current_container =
+        marked_text.EndOffsetInCurrentContainer();
+
+    DCHECK_GE(end_offset_in_current_container,
+              start_offset_in_current_container);
+
+    // TODO(editing-dev): TextIterator sometimes emits ranges where the start
+    // and end offsets are the same. Investigate if TextIterator should be
+    // changed to not do this. See crbug.com/727929
+    if (end_offset_in_current_container == start_offset_in_current_container)
+      continue;
+
+    // Ignore text emitted by TextIterator for non-text nodes (e.g. implicit
+    // newlines)
+    Node* node = marked_text.CurrentContainer();

[yosin_UTC9, 2017/05/31 02:01:14]

nit: s/Node*/Node* const/

+    if (!node->IsTextNode())
+      continue;
+
+    DocumentMarker* const new_marker = create_marker_from_offsets(
+        start_offset_in_current_container, end_offset_in_current_container);
+    AddMarkerToNode(node, new_marker);
+  }
+}
+
 // Markers are stored in order sorted by their start offset.
 // Markers of the same type do not overlap each other.
-
-void DocumentMarkerController::AddMarker(Node* node,
-                                         DocumentMarker* new_marker) {
-  DCHECK_GE(new_marker->EndOffset(), new_marker->StartOffset());
-  if (new_marker->EndOffset() == new_marker->StartOffset())
-    return;
-
+void DocumentMarkerController::AddMarkerToNode(Node* node,
+                                               DocumentMarker* new_marker) {
   possibly_existing_marker_types_.Add(new_marker->GetType());
 
   Member<MarkerLists>& markers =
       markers_.insert(node, nullptr).stored_value->value;
   if (!markers) {
     markers = new MarkerLists;
     markers->Grow(DocumentMarker::kMarkerTypeIndexesCount);
   }
 
   const DocumentMarker::MarkerType new_marker_type = new_marker->GetType();
@@ skipping 473 matching lines @@
 }
 
 }  // namespace blink
 
 #ifndef NDEBUG
 void showDocumentMarkers(const blink::DocumentMarkerController* controller) {
   if (controller)
     controller->ShowMarkers();
 }
 #endif