[DMC #1.9] Move DocumentMarkerListEditor to its own .h/.cpp files

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

Index: third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
diff --git a/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp b/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
index 3ca0272a18c56e0da3172fc649819a4d854f84a4..4a63481c503c2f311e2c0ba5d7b327798c24cdd9 100644
--- a/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
+++ b/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
@@ -28,15 +28,16 @@
 
 #include "core/editing/markers/DocumentMarkerController.h"
 
+#include <algorithm>
 #include "core/dom/Node.h"
 #include "core/dom/NodeTraversal.h"
 #include "core/dom/Range.h"
 #include "core/dom/Text.h"
 #include "core/editing/iterators/TextIterator.h"
+#include "core/editing/markers/DocumentMarkerListEditor.h"
 #include "core/editing/markers/RenderedDocumentMarker.h"
 #include "core/frame/FrameView.h"
 #include "core/layout/LayoutObject.h"
-#include <algorithm>
 
 #ifndef NDEBUG
 #include <stdio.h>
@@ -175,26 +176,11 @@ void DocumentMarkerController::RemoveMarkers(
   DocumentMarkerController::RemoveMarkers(marked_text, marker_types);
 }
 
-static bool StartsFurther(const Member<RenderedDocumentMarker>& lhv,
-                          const DocumentMarker* rhv) {
-  return lhv->StartOffset() < rhv->StartOffset();
-}
-
-static bool EndsBefore(size_t start_offset,
-                       const Member<RenderedDocumentMarker>& rhv) {
-  return start_offset < rhv->EndOffset();
-}
-
 static bool CompareByStart(const Member<DocumentMarker>& lhv,
                            const Member<DocumentMarker>& rhv) {
   return lhv->StartOffset() < rhv->StartOffset();
 }
 
-static bool DoesNotOverlap(const Member<RenderedDocumentMarker>& lhv,
-                           const DocumentMarker* rhv) {
-  return lhv->EndOffset() < rhv->StartOffset();
-}
-
 static void UpdateMarkerRenderedRect(const Node& node,
                                      RenderedDocumentMarker& marker) {
   Range* range = Range::Create(node.GetDocument());
@@ -248,47 +234,6 @@ void DocumentMarkerController::AddMarker(Node* node,
   }
 }
 
-// TODO(rlanday): move DocumentMarkerListEditor into its own .h/.cpp files
-// 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) {
-  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, StartsFurther);
-      list->insert(pos - list->begin(), rendered_marker);
-    }
-  }
-}
-
-// TODO(rlanday): move DocumentMarkerListEditor into its own .h/.cpp files
-void DocumentMarkerListEditor::MergeOverlapping(
-    MarkerList* list,
-    RenderedDocumentMarker* to_insert) {
-  MarkerList::iterator first_overlapping =
-      std::lower_bound(list->begin(), list->end(), to_insert, DoesNotOverlap);
-  size_t index = first_overlapping - list->begin();
-  list->insert(index, to_insert);
-  MarkerList::iterator inserted = list->begin() + index;
-  first_overlapping = inserted + 1;
-  for (MarkerList::iterator 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());
-  }
-}
-
 // Moves markers from src_node to dst_node. Markers are moved if their start
 // offset is less than length. Markers that run past that point are truncated.
 void DocumentMarkerController::MoveMarkers(Node* src_node,
@@ -333,35 +278,6 @@ void DocumentMarkerController::MoveMarkers(Node* src_node,
   }
 }
 
-// TODO(rlanday): move DocumentMarkerListEditor into its own .h/.cpp files
-bool DocumentMarkerListEditor::MoveMarkers(MarkerList* src_list,
-                                           int length,
-                                           MarkerList* dst_list) {
-  DCHECK_GT(length, 0);
-  bool doc_dirty = false;
-  unsigned end_offset = length - 1;
-  MarkerList::iterator it;
-  for (it = src_list->begin(); it != src_list->end(); ++it) {
-    DocumentMarker* marker = it->Get();
-
-    // stop if we are now past the specified range
-    if (marker->StartOffset() > end_offset)
-      break;
-
-    // pin the marker to the specified range
-    doc_dirty = true;
-    if (marker->EndOffset() > end_offset)
-      marker->SetEndOffset(end_offset);
-
-    DocumentMarkerListEditor::AddMarker(dst_list, marker);
-  }
-
-  // Remove the range of markers that were moved to dst_node
-  src_list->erase(0, it - src_list->begin());
-
-  return doc_dirty;
-}
-
 void DocumentMarkerController::RemoveMarkers(
     Node* node,
     unsigned start_offset,
@@ -415,32 +331,6 @@ void DocumentMarkerController::RemoveMarkers(
   }
 }
 
-// TODO(rlanday): move DocumentMarkerListEditor into its own .h/.cpp files
-// TODO(rlanday): this method was created by cutting and pasting code from
-// DocumentMarkerController::RemoveMarkers(), it should be refactored in a
-// future CL
-bool DocumentMarkerListEditor::RemoveMarkers(MarkerList* list,
-                                             unsigned start_offset,
-                                             int length) {
-  bool doc_dirty = false;
-  unsigned end_offset = start_offset + length;
-  MarkerList::iterator start_pos =
-      std::upper_bound(list->begin(), list->end(), start_offset, EndsBefore);
-  for (MarkerList::iterator i = start_pos; i != list->end();) {
-    DocumentMarker marker(*i->Get());
-
-    // markers are returned in order, so stop if we are now past the specified
-    // range
-    if (marker.StartOffset() >= end_offset)
-      break;
-
-    list->erase(i - list->begin());
-    doc_dirty = true;
-  }
-
-  return doc_dirty;
-}
-
 DocumentMarkerVector DocumentMarkerController::MarkersFor(
     Node* node,
     DocumentMarker::MarkerTypes marker_types) {
@@ -769,6 +659,11 @@ bool DocumentMarkerController::SetMarkersActive(const EphemeralRange& range,
   return marker_found;
 }
 
+static bool EndsBefore(size_t start_offset,

[Xiaocheng, 2017/04/19 03:33:23]

It's weird to have a chunk of new code in an old file in a patch named "move
code". It's also bad to have the same function duplicated in two files.

How about moving these static functions to, say, DocumentMarker as static
members first? So that they can be used everywhere without having duplicate
code.

Btw, can these functions take |const DocumentMarker*| as parameters? If so, we
can do things like:

1. Change these functions to take |const DocumentMarker*| instead of |const
Member<RDM>&|
2. Move these functions as static members of DocumentMarker

[rlanday, 2017/04/20 02:19:05]

This isn't new code, I moved it because I thought it made sense to put it closer
to where it's used since I was removing the other functions it was grouped with.

+                       const Member<RenderedDocumentMarker>& rhv) {

[yosin_UTC9, 2017/04/20 01:43:01]

Please use descriptive parameter name instead of |rhv|, I could not get what
this stands for...

[rlanday, 2017/04/20 02:19:05]

I'm only moving this code; I think it stands for "right-hand value", but we can
call it something like "marker" if that's better...

+  return start_offset < rhv->EndOffset();
+}
+
 bool DocumentMarkerController::SetMarkersActive(Node* node,
                                                 unsigned start_offset,
                                                 unsigned end_offset,
@@ -868,34 +763,6 @@ void DocumentMarkerController::DidUpdateCharacterData(CharacterData* node,
   node->GetLayoutObject()->SetShouldDoFullPaintInvalidation();
 }
 
-// TODO(rlanday): move DocumentMarkerListEditor into its own .h/.cpp files
-bool DocumentMarkerListEditor::ShiftMarkers(MarkerList* list,
-                                            unsigned offset,
-                                            unsigned old_length,
-                                            unsigned new_length) {
-  bool did_shift_marker = false;
-  for (MarkerList::iterator it = list->begin(); it != list->end(); ++it) {
-    RenderedDocumentMarker& marker = **it;
-    Optional<DocumentMarker::MarkerOffsets> result =
-        marker.ComputeOffsetsAfterShift(offset, old_length, new_length);
-    if (result == WTF::kNullopt) {
-      list->erase(it - list->begin());
-      --it;
-      did_shift_marker = true;
-      continue;
-    }
-
-    if (marker.StartOffset() != result.value().start_offset ||
-        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;
-}
-
 }  // namespace blink
 
 #ifndef NDEBUG