[DMC #3.5] Split up DocumentMarkerListEditor::AddMarker() into two methods

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

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/editing/markers/DocumentMarkerListEditor.h"
 
 #include "core/editing/markers/RenderedDocumentMarker.h"
 
 namespace blink {
 
-// TODO(rlanday): this method was created by cutting and pasting code from
-// DocumentMarkerController::AddMarker(), it should be refactored in a future CL
-void DocumentMarkerListEditor::AddMarker(MarkerList* list,
-                                         const DocumentMarker* marker) {
+void DocumentMarkerListEditor::AddMarkerAndMergeOverlapping(

[Xiaocheng, 2017/04/26 22:12:07]

Could you put DocumentMarkerListEditor::AddMarkerWithoutMergingOverlapping here
to minimize the change set?

+    MarkerList* list,
+    const DocumentMarker* marker) {
   RenderedDocumentMarker* rendered_marker =
       RenderedDocumentMarker::Create(*marker);
   if (list->IsEmpty() || list->back()->EndOffset() < marker->StartOffset()) {
     list->push_back(rendered_marker);
-  } else {
-    if (marker->GetType() != DocumentMarker::kTextMatch &&
-        marker->GetType() != DocumentMarker::kComposition) {
-      MergeOverlapping(list, rendered_marker);
-    } else {
-      MarkerList::iterator pos = std::lower_bound(
-          list->begin(), list->end(), marker,
-          [](const Member<RenderedDocumentMarker>& marker_in_list,
-             const DocumentMarker* marker_to_insert) {
-            return marker_in_list->StartOffset() <
-                   marker_to_insert->StartOffset();
-          });
-      list->insert(pos - list->begin(), rendered_marker);
-    }
+    return;
   }
+
+  auto first_overlapping =
+      std::lower_bound(list->begin(), list->end(), rendered_marker,
+                       [](const Member<RenderedDocumentMarker>& marker_in_list,
+                          const DocumentMarker* marker_rendered_marker) {

[Xiaocheng, 2017/04/26 22:12:07]

No need to rename this parameter.

[rlanday, 2017/04/26 22:14:46]

Oops, bad find-and-replace :)

+                         return marker_in_list->EndOffset() <
+                                marker_rendered_marker->StartOffset();
+                       });
+
+  size_t index = first_overlapping - list->begin();
+  list->insert(index, rendered_marker);
+  const auto inserted = list->begin() + index;
+  first_overlapping = inserted + 1;
+  // TODO(rlanday): optimize this loop so it runs in O(N) time and not O(N^2)
+  for (const auto i = first_overlapping;
+       i != list->end() && (*i)->StartOffset() <= (*inserted)->EndOffset();) {
+    (*inserted)->SetStartOffset(
+        std::min((*inserted)->StartOffset(), (*i)->StartOffset()));
+    (*inserted)->SetEndOffset(
+        std::max((*inserted)->EndOffset(), (*i)->EndOffset()));
+    list->erase(i - list->begin());
+  }
+}
+
+void DocumentMarkerListEditor::AddMarkerWithoutMergingOverlapping(
+    MarkerList* list,
+    const DocumentMarker* marker) {
+  RenderedDocumentMarker* rendered_marker =
+      RenderedDocumentMarker::Create(*marker);
+  if (list->IsEmpty() || list->back()->EndOffset() <= marker->StartOffset()) {
+    list->push_back(rendered_marker);
+    return;
+  }
+
+  MarkerList::iterator pos = std::lower_bound(
+      list->begin(), list->end(), marker,
+      [](const Member<RenderedDocumentMarker>& marker_in_list,
+         const DocumentMarker* marker_to_insert) {
+        return marker_in_list->StartOffset() < marker_to_insert->StartOffset();
+      });
+  list->insert(pos - list->begin(), rendered_marker);
 }
 
 bool DocumentMarkerListEditor::MoveMarkers(MarkerList* src_list,
                                            int length,
                                            DocumentMarkerList* dst_list) {
   DCHECK_GT(length, 0);
   bool didMoveMarker = false;
   unsigned end_offset = length - 1;
 
   MarkerList::iterator it;
@@ skipping 83 matching lines @@
         marker.EndOffset() != result.value().end_offset) {
       did_shift_marker = true;
       marker.SetStartOffset(result.value().start_offset);
       marker.SetEndOffset(result.value().end_offset);
     }
   }
 
   return did_shift_marker;
 }
 
-void DocumentMarkerListEditor::MergeOverlapping(

[Xiaocheng, 2017/04/26 22:12:07]

Could you put DocumentMarkerListEditor::AddMarkerAndMergeOverlapping here to
minimize the changeset?

-    DocumentMarkerListEditor::MarkerList* list,
-    RenderedDocumentMarker* to_insert) {
-  auto first_overlapping = std::lower_bound(
-      list->begin(), list->end(), to_insert,
-      [](const Member<RenderedDocumentMarker>& marker_in_list,
-         const DocumentMarker* marker_to_insert) {
-        return marker_in_list->EndOffset() < marker_to_insert->StartOffset();
-      });
-  size_t index = first_overlapping - list->begin();
-  list->insert(index, to_insert);
-  const auto inserted = list->begin() + index;
-  first_overlapping = inserted + 1;
-  for (const auto i = first_overlapping;
-       i != list->end() && (*i)->StartOffset() <= (*inserted)->EndOffset();) {
-    (*inserted)->SetStartOffset(
-        std::min((*inserted)->StartOffset(), (*i)->StartOffset()));
-    (*inserted)->SetEndOffset(
-        std::max((*inserted)->EndOffset(), (*i)->EndOffset()));
-    list->erase(i - list->begin());
-  }
-}
-
 }  // namespace blink