[DMC #2] Add DocumentMarkerList interface and GenericDocumentMarkerListImpl

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 17 matching lines @@
 
 #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/GenericDocumentMarkerListImpl.h"
 #include "core/editing/markers/RenderedDocumentMarker.h"
 #include "core/frame/FrameView.h"
 #include "core/layout/LayoutObject.h"
 
 #ifndef NDEBUG
 #include <stdio.h>
 #endif
 
 namespace blink {
 
@@ skipping 11 matching lines @@
     case DocumentMarker::kComposition:
       return DocumentMarker::kCompositionMarkerIndex;
   }
 
   NOTREACHED();
   return DocumentMarker::kSpellingMarkerIndex;
 }
 
 }  // namespace
 
-Member<DocumentMarkerController::MarkerList>&
-DocumentMarkerController::ListForType(MarkerLists* marker_lists,
-                                      DocumentMarker::MarkerType type) {
+Member<DocumentMarkerList>& DocumentMarkerController::ListForType(
+    MarkerLists* marker_lists,
+    DocumentMarker::MarkerType type) {
   const size_t marker_list_index = MarkerTypeToMarkerIndex(type);
   return (*marker_lists)[marker_list_index];
 }
 
 inline bool DocumentMarkerController::PossiblyHasMarkers(
     DocumentMarker::MarkerTypes types) {
   return possibly_existing_marker_types_.Intersects(types);
 }
 
 DocumentMarkerController::DocumentMarkerController(Document& document)
@@ skipping 116 matching lines @@
   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();
-  if (!ListForType(markers, new_marker_type))
-    ListForType(markers, new_marker_type) = new MarkerList;
+  if (!ListForType(markers, new_marker_type)) {
+    // TODO(rlanday): add method for deciding what type of list to create based
+    // on the MarkerType
+    ListForType(markers, new_marker_type) = new GenericDocumentMarkerListImpl;
+  }
 
-  Member<MarkerList>& list = ListForType(markers, new_marker_type);
-  DocumentMarkerListEditor::AddMarker(list, new_marker);
+  DocumentMarkerList* list = ListForType(markers, new_marker_type);

[yosin_UTC9, 2017/04/25 08:20:58]

nit: s/DocumentMarkerList*/DocumentMarkerList* const/

+  list->Add(new_marker);
 
   // repaint the affected node
   if (node->GetLayoutObject()) {
     node->GetLayoutObject()->SetShouldDoFullPaintInvalidation(
         kPaintInvalidationDocumentMarkerChange);
   }
 }
 
 // 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.
@@ skipping 12 matching lines @@
     return;
 
   if (!markers_.Contains(dst_node)) {
     markers_.insert(dst_node,
                     new MarkerLists(DocumentMarker::kMarkerTypeIndexesCount));
   }
   MarkerLists* dst_markers = markers_.at(dst_node);
 
   bool doc_dirty = false;
   for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
-    MarkerList* src_list = ListForType(src_markers, type);
+    DocumentMarkerList* src_list = ListForType(src_markers, type);

[yosin_UTC9, 2017/04/25 08:20:58]

nit: s/DocumentMarkerList*/DocumentMarkerList* const/

     if (!src_list)
       continue;
 
-    if (!ListForType(dst_markers, type))
-      ListForType(dst_markers, type) = new MarkerList;
-    MarkerList* dst_list = ListForType(dst_markers, type);
+    if (!ListForType(dst_markers, type)) {
+      // TODO(rlanday): add method for deciding what type of list to create
+      // based on the MarkerType
+      ListForType(dst_markers, type) = new GenericDocumentMarkerListImpl;
+    }
 
-    if (DocumentMarkerListEditor::MoveMarkers(src_list, length, dst_list))
+    DocumentMarkerList* dst_list = ListForType(dst_markers, type);
+    if (src_list->MoveMarkers(length, dst_list))
       doc_dirty = true;
   }
 
   // repaint the affected node
   if (doc_dirty && dst_node->GetLayoutObject()) {
     dst_node->GetLayoutObject()->SetShouldDoFullPaintInvalidation(
         kPaintInvalidationDocumentMarkerChange);
   }
 }
 
 void DocumentMarkerController::RemoveMarkersInternal(
     Node* node,
     unsigned start_offset,
     int length,
     DocumentMarker::MarkerTypes marker_types) {
   if (length <= 0)
     return;
 
   if (!PossiblyHasMarkers(marker_types))
     return;
   DCHECK(!(markers_.IsEmpty()));
 
   MarkerLists* markers = markers_.at(node);
   if (!markers)
     return;
 
   bool doc_dirty = false;
   size_t empty_lists_count = 0;
   for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
-    Member<MarkerList>& list = ListForType(markers, type);
+    DocumentMarkerList* list = ListForType(markers, type);

[yosin_UTC9, 2017/04/25 08:20:58]

nit: s/DocumentMarkerList*/DocumentMarkerList* const/

     if (!list || list->IsEmpty()) {
-      if (list.Get() && list->IsEmpty())
-        list.Clear();
+      if (list && list->IsEmpty())
+        ListForType(markers, type) = nullptr;
       ++empty_lists_count;
       continue;
     }
     if (!marker_types.Contains(type))
       continue;
 
-    if (DocumentMarkerListEditor::RemoveMarkers(list, start_offset, length))
+    if (list->RemoveMarkers(start_offset, length))
       doc_dirty = true;
 
     if (list->IsEmpty()) {
-      list.Clear();
+      ListForType(markers, type) = nullptr;
       ++empty_lists_count;
     }
   }
 
   if (empty_lists_count == DocumentMarker::kMarkerTypeIndexesCount) {
     markers_.erase(node);
     if (markers_.IsEmpty())
       possibly_existing_marker_types_ = 0;
   }
 
   // repaint the affected node
   if (doc_dirty && node->GetLayoutObject()) {
     node->GetLayoutObject()->SetShouldDoFullPaintInvalidation(
         kPaintInvalidationDocumentMarkerChange);
   }
 }
 
 DocumentMarkerVector DocumentMarkerController::MarkersFor(
     Node* node,
     DocumentMarker::MarkerTypes marker_types) {
   DocumentMarkerVector result;
 
   MarkerLists* markers = markers_.at(node);
   if (!markers)
     return result;
 
   for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
-    Member<MarkerList>& list = ListForType(markers, type);
-    if (!list || list->IsEmpty() ||
-        !marker_types.Contains((*list->begin())->GetType()))
+    DocumentMarkerList* list = ListForType(markers, type);

[yosin_UTC9, 2017/04/25 08:20:58]

nit: s/DocumentMarkerList*/DocumentMarkerList* const/

+    if (!list || list->IsEmpty() || !marker_types.Contains(type))
       continue;
 
-    for (size_t i = 0; i < list->size(); ++i)
-      result.push_back(list->at(i).Get());
+    result.AppendVector(list->GetMarkers());

[yosin_UTC9, 2017/04/25 08:20:58]

Wow, AppendVector() is easy.

[rlanday, 2017/04/25 19:29:52]

Except that as far as I'm aware, we haven't solved the problem of how to do this
efficiently
once some of the marker lists are storing pointers to DocumentMarker and
TextMatchMarkerList
is storing pointers to RenderedDocumentMarker (maybe we can make them all store
DocumentMarker*
and cast to RenderedDocumentMarker* in TextMatchMarkerList?).

   }
 
   std::sort(result.begin(), result.end(),
             [](const Member<DocumentMarker>& marker1,
                const Member<DocumentMarker>& marker2) {
               return marker1->StartOffset() < marker2->StartOffset();
             });
   return result;
 }
 
 DocumentMarkerVector DocumentMarkerController::Markers() {
   DocumentMarkerVector result;
   for (MarkerMap::iterator i = markers_.begin(); i != markers_.end(); ++i) {
     MarkerLists* markers = i->value.Get();
     for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
-      Member<MarkerList>& list = ListForType(markers, type);
-      for (size_t j = 0; list.Get() && j < list->size(); ++j)
-        result.push_back(list->at(j).Get());
+      DocumentMarkerList* list = ListForType(markers, type);
+      if (!list)
+        continue;
+      result.AppendVector(list->GetMarkers());
     }
   }
   std::sort(result.begin(), result.end(),
             [](const Member<DocumentMarker>& marker1,
                const Member<DocumentMarker>& marker2) {
               return marker1->StartOffset() < marker2->StartOffset();
             });
   return result;
 }
 
@@ skipping 39 matching lines @@
   // outer loop: process each node
   MarkerMap::iterator end = markers_.end();
   for (MarkerMap::iterator node_iterator = markers_.begin();
        node_iterator != end; ++node_iterator) {
     // inner loop; process each marker in this node
     const Node& node = *node_iterator->key;
     if (!node.isConnected())
       continue;
     MarkerLists* markers = node_iterator->value.Get();
     for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
-      Member<MarkerList>& list = ListForType(markers, type);
+      DocumentMarkerList* list = ListForType(markers, type);

[yosin_UTC9, 2017/04/25 08:20:59]

nit: s/DocumentMarkerList*/DocumentMarkerList* const/

       if (!list || list->IsEmpty() || type != marker_type)
         continue;
-      for (unsigned marker_index = 0; marker_index < list->size();
-           ++marker_index) {
-        RenderedDocumentMarker* marker = list->at(marker_index).Get();
-        UpdateMarkerRenderedRectIfNeeded(node, *marker);
-        if (!marker->IsRendered())
+
+      for (RenderedDocumentMarker* rendered_marker : list->GetMarkers()) {
+        UpdateMarkerRenderedRectIfNeeded(node, *rendered_marker);
+        if (!rendered_marker->IsRendered())
           continue;
-        result.push_back(marker->RenderedRect());
+        result.push_back(rendered_marker->RenderedRect());
       }
     }
   }
 
   return result;
 }
 
 static void InvalidatePaintForTickmarks(const Node& node) {
   if (FrameView* frame_view = node.GetDocument().View())
     frame_view->InvalidatePaintForTickmarks();
 }
 
 void DocumentMarkerController::UpdateMarkerRenderedRectIfNeeded(
     const Node& node,
     RenderedDocumentMarker& marker) {
   DCHECK(!document_->View() || !document_->View()->NeedsLayout());
   DCHECK(!document_->NeedsLayoutTreeUpdate());
   if (!marker.IsValid())
     UpdateMarkerRenderedRect(node, marker);
 }
 
 void DocumentMarkerController::InvalidateRectsForMarkersInNode(
     const Node& node) {
   MarkerLists* markers = markers_.at(&node);
 
   for (auto& marker_list : *markers) {
     if (!marker_list || marker_list->IsEmpty())
       continue;
 
-    for (auto& marker : *marker_list)
+    const HeapVector<Member<RenderedDocumentMarker>>& markers_in_list =
+        marker_list->GetMarkers();
+    for (auto& marker : markers_in_list)
       marker->Invalidate();
 
-    if (marker_list->front()->GetType() == DocumentMarker::kTextMatch)
+    if (markers_in_list.front()->GetType() == DocumentMarker::kTextMatch)
       InvalidatePaintForTickmarks(node);
   }
 }
 
 void DocumentMarkerController::InvalidateRectsForAllMarkers() {
   for (auto& node_markers : markers_) {
     const Node& node = *node_markers.key;
     for (auto& marker_list : *node_markers.value) {
       if (!marker_list || marker_list->IsEmpty())
         continue;
 
-      for (auto& marker : *marker_list)
-        marker->Invalidate();
+      const HeapVector<Member<RenderedDocumentMarker>>& markers_in_list =
+          marker_list->GetMarkers();
+      for (DocumentMarker* marker : markers_in_list)
+        ToRenderedDocumentMarker(marker)->Invalidate();
 
-      if (marker_list->front()->GetType() == DocumentMarker::kTextMatch)
+      if (markers_in_list.front()->GetType() == DocumentMarker::kTextMatch)
         InvalidatePaintForTickmarks(node);
     }
   }
 }
 
 DEFINE_TRACE(DocumentMarkerController) {
   visitor->Trace(markers_);
   visitor->Trace(document_);
   SynchronousMutationObserver::Trace(visitor);
 }
@@ skipping 11 matching lines @@
 }
 
 void DocumentMarkerController::RemoveSpellingMarkersUnderWords(
     const Vector<String>& words) {
   for (auto& node_markers : markers_) {
     const Node& node = *node_markers.key;
     if (!node.IsTextNode())  // MarkerRemoverPredicate requires a Text node.
       continue;
     MarkerLists* markers = node_markers.value;
     for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
-      MarkerList* list = ListForType(markers, type);
+      DocumentMarkerList* list = ListForType(markers, type);

[yosin_UTC9, 2017/04/25 08:20:58]

nit: s/DocumentMarkerList*/DocumentMarkerList* const/

       if (!list)
         continue;
-      bool removed_markers = DocumentMarkerListEditor::RemoveMarkersUnderWords(
-          list, ToText(node).data(), words);
+      bool removed_markers =

[yosin_UTC9, 2017/04/25 08:20:58]

nit: s/bool/const bool/

+          list->RemoveMarkersUnderWords(ToText(node).data(), words);
       if (removed_markers && type == DocumentMarker::kTextMatch)
         InvalidatePaintForTickmarks(node);
     }
   }
 }
 
 void DocumentMarkerController::RemoveMarkersOfTypes(
     DocumentMarker::MarkerTypes marker_types) {
   if (!PossiblyHasMarkers(marker_types))
     return;
@@ skipping 18 matching lines @@
   bool node_can_be_removed;
 
   size_t empty_lists_count = 0;
   if (marker_types == DocumentMarker::AllMarkers()) {
     needs_repainting = true;
     node_can_be_removed = true;
   } else {
     MarkerLists* markers = iterator->value.Get();
 
     for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
-      Member<MarkerList>& list = ListForType(markers, type);
+      DocumentMarkerList* list = ListForType(markers, type);

[yosin_UTC9, 2017/04/25 08:20:59]

nit: s/DocumentMarkerList*/DocumentMarkerList* const/

       if (!list || list->IsEmpty()) {
-        if (list.Get() && list->IsEmpty())
-          list.Clear();
+        if (list && list->IsEmpty())
+          ListForType(markers, type) = nullptr;
         ++empty_lists_count;
         continue;
       }
       if (marker_types.Contains(type)) {
-        list->clear();
-        list.Clear();
+        list->Clear();
+        ListForType(markers, type) = nullptr;
         ++empty_lists_count;
         needs_repainting = true;
       }
     }
 
     node_can_be_removed =
         empty_lists_count == DocumentMarker::kMarkerTypeIndexesCount;
   }
 
   if (needs_repainting) {
@@ skipping 19 matching lines @@
   DCHECK(!markers_.IsEmpty());
 
   // outer loop: process each markered node in the document
   MarkerMap::iterator end = markers_.end();
   for (MarkerMap::iterator i = markers_.begin(); i != end; ++i) {
     const Node* node = i->key;
 
     // inner loop: process each marker in the current node
     MarkerLists* markers = i->value.Get();
     for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
-      Member<MarkerList>& list = ListForType(markers, type);
+      DocumentMarkerList* list = ListForType(markers, type);

[yosin_UTC9, 2017/04/25 08:20:58]

nit: s/DocumentMarkerList*/DocumentMarkerList* const/

       if (!list || list->IsEmpty() || !marker_types.Contains(type))
         continue;
 
       // cause the node to be redrawn
       if (LayoutObject* layout_object = node->GetLayoutObject()) {
         layout_object->SetShouldDoFullPaintInvalidation(
             kPaintInvalidationDocumentMarkerChange);
         break;
       }
     }
@@ skipping 28 matching lines @@
 
 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);
+  DocumentMarkerList* list = ListForType(markers, DocumentMarker::kTextMatch);

[yosin_UTC9, 2017/04/25 08:20:59]

nit: s/DocumentMarkerList*/DocumentMarkerList* const/

+
   if (!list)
     return false;
-  MarkerList::iterator start_pos = std::upper_bound(
-      list->begin(), list->end(), start_offset,
-      [](size_t start_offset, const Member<RenderedDocumentMarker>& marker) {
+
+  const HeapVector<Member<RenderedDocumentMarker>>& markers_in_list =
+      list->GetMarkers();
+  // TODO(rlanday): this assumes that the markers are stored in sorted order.
+  // This method should probably eventually be implemented by a
+  // TextMatch-specific marker list
+  const auto start_pos = std::upper_bound(
+      markers_in_list.begin(), markers_in_list.end(), start_offset,
+      [](size_t start_offset, const Member<DocumentMarker>& marker) {
         return start_offset < marker->EndOffset();
       });
-  for (MarkerList::iterator marker = start_pos; marker != list->end();
-       ++marker) {
+  for (auto marker = start_pos; marker != markers_in_list.end(); ++marker) {
     // Markers are returned in order, so stop if we are now past the specified
     // range.
     if ((*marker)->StartOffset() >= end_offset)
       break;
 
     (*marker)->SetIsActiveMatch(active);
     doc_dirty = true;
   }
 
   // repaint the affected node
   if (doc_dirty && node->GetLayoutObject()) {
     node->GetLayoutObject()->SetShouldDoFullPaintInvalidation(
         kPaintInvalidationDocumentMarkerChange);
   }
   return doc_dirty;
 }
 
 #ifndef NDEBUG
 void DocumentMarkerController::ShowMarkers() const {
   StringBuilder builder;
   MarkerMap::const_iterator end = markers_.end();
   for (MarkerMap::const_iterator node_iterator = markers_.begin();
        node_iterator != end; ++node_iterator) {
     const Node* node = node_iterator->key;
     builder.Append(String::Format("%p", node));
     MarkerLists* markers = markers_.at(node);
     for (DocumentMarker::MarkerType type : DocumentMarker::AllMarkers()) {
-      Member<MarkerList>& list = ListForType(markers, type);
-      for (unsigned marker_index = 0; list.Get() && marker_index < list->size();
-           ++marker_index) {
-        DocumentMarker* marker = list->at(marker_index).Get();
+      DocumentMarkerList* list = ListForType(markers, type);
+      const HeapVector<Member<RenderedDocumentMarker>>& markers_in_list =
+          list->GetMarkers();
+      for (const DocumentMarker* marker : markers_in_list) {
         builder.Append(" ");
         builder.AppendNumber(marker->GetType());
         builder.Append(":[");
         builder.AppendNumber(marker->StartOffset());
         builder.Append(":");
         builder.AppendNumber(marker->EndOffset());
         builder.Append("](");
         builder.AppendNumber(marker->IsActiveMatch());
         builder.Append(")");
       }
@@ skipping 12 matching lines @@
                                                       unsigned new_length) {
   if (!PossiblyHasMarkers(DocumentMarker::AllMarkers()))
     return;
   DCHECK(!markers_.IsEmpty());
 
   MarkerLists* markers = markers_.at(node);
   if (!markers)
     return;
 
   bool did_shift_marker = false;
-  for (MarkerList* list : *markers) {
+  for (DocumentMarkerList* list : *markers) {
     if (!list)
       continue;
 
-    if (DocumentMarkerListEditor::ShiftMarkers(list, offset, old_length,
-                                               new_length))
+    if (list->ShiftMarkers(offset, old_length, new_length))
       did_shift_marker = true;
   }
 
   if (!did_shift_marker)
     return;
   if (!node->GetLayoutObject())
     return;
   InvalidateRectsForMarkersInNode(*node);
   // repaint the affected node
   node->GetLayoutObject()->SetShouldDoFullPaintInvalidation();
 }
 
 }  // namespace blink
 
 #ifndef NDEBUG
 void showDocumentMarkers(const blink::DocumentMarkerController* controller) {
   if (controller)
     controller->ShowMarkers();
 }
 #endif