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

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 10 matching lines @@
  *
  * You should have received a copy of the GNU Library General Public License
  * along with this library; see the file COPYING.LIB.  If not, write to
  * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
  * Boston, MA 02110-1301, USA.
  *
  */
 
 #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>
 #endif
 
 namespace blink {
 
 MarkerRemoverPredicate::MarkerRemoverPredicate(const Vector<String>& words)
     : words_(words) {}
 
@@ skipping 118 matching lines @@
 
 void DocumentMarkerController::RemoveMarkers(
     const EphemeralRange& range,
     DocumentMarker::MarkerTypes marker_types) {
   DCHECK(!document_->NeedsLayoutTreeUpdate());
 
   TextIterator marked_text(range.StartPosition(), range.EndPosition());
   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());
   // The offsets of the marker may be out-dated, so check for exceptions.
   DummyExceptionStateForTesting exception_state;
   range->setStart(&const_cast<Node&>(node), marker.StartOffset(),
                   exception_state);
   if (!exception_state.HadException()) {
     range->setEnd(&const_cast<Node&>(node), marker.EndOffset(),
                   IGNORE_EXCEPTION_FOR_TESTING);
@@ skipping 33 matching lines @@
   Member<MarkerList>& list = ListForType(markers, new_marker_type);
   DocumentMarkerListEditor::AddMarker(list, &new_marker);
 
   // repaint the affected node
   if (node->GetLayoutObject()) {
     node->GetLayoutObject()->SetShouldDoFullPaintInvalidation(
         kPaintInvalidationDocumentMarkerChange);
   }
 }
 
-// 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,
                                            int length,
                                            Node* dst_node) {
   if (length <= 0)
     return;
 
   if (!PossiblyHasMarkers(DocumentMarker::AllMarkers()))
     return;
@@ skipping 24 matching lines @@
       doc_dirty = true;
   }
 
   // repaint the affected node
   if (doc_dirty && dst_node->GetLayoutObject()) {
     dst_node->GetLayoutObject()->SetShouldDoFullPaintInvalidation(
         kPaintInvalidationDocumentMarkerChange);
   }
 }
 
-// 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,
     int length,
     DocumentMarker::MarkerTypes marker_types) {
   if (length <= 0)
     return;
 
   if (!PossiblyHasMarkers(marker_types))
     return;
@@ skipping 33 matching lines @@
       possibly_existing_marker_types_ = 0;
   }
 
   // repaint the affected node
   if (doc_dirty && node->GetLayoutObject()) {
     node->GetLayoutObject()->SetShouldDoFullPaintInvalidation(
         kPaintInvalidationDocumentMarkerChange);
   }
 }
 
-// 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) {
   DocumentMarkerVector result;
 
   MarkerLists* markers = markers_.at(node);
   if (!markers)
     return result;
 
   for (size_t marker_list_index = 0;
@@ skipping 308 matching lines @@
 
   bool marker_found = false;
   for (Node& node : range.Nodes()) {
     int start_offset = node == start_container ? container_start_offset : 0;
     int end_offset = node == end_container ? container_end_offset : INT_MAX;
     marker_found |= SetMarkersActive(&node, start_offset, end_offset, active);
   }
   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,
                                                 bool active) {
   MarkerLists* markers = markers_.at(node);
   if (!markers)
     return false;
 
   bool doc_dirty = false;
   Member<MarkerList>& list = ListForType(markers, DocumentMarker::kTextMatch);
@@ skipping 79 matching lines @@
 
   if (!did_shift_marker)
     return;
   if (!node->GetLayoutObject())
     return;
   InvalidateRectsForMarkersInNode(*node);
   // repaint the affected 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
 void showDocumentMarkers(const blink::DocumentMarkerController* controller) {
   if (controller)
     controller->ShowMarkers();
 }
 #endif