Rewrite DocumentMarkerController to use SynchronousMutationObserver

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 66 matching lines @@
   return DocumentMarker::SpellingMarkerIndex;
 }
 
 }  // namespace
 
 inline bool DocumentMarkerController::possiblyHasMarkers(
     DocumentMarker::MarkerTypes types) {
   return m_possiblyExistingMarkerTypes.intersects(types);
 }
 
-DocumentMarkerController::DocumentMarkerController(const Document& document)
-    : m_possiblyExistingMarkerTypes(0), m_document(&document) {}
+DocumentMarkerController::DocumentMarkerController(Document& document)
+    : m_possiblyExistingMarkerTypes(0), m_document(&document) {
+  setContext(&document);
+}
 
 void DocumentMarkerController::clear() {
   m_markers.clear();
   m_possiblyExistingMarkerTypes = 0;
 }
 
 void DocumentMarkerController::addMarker(const Position& start,
                                          const Position& end,
                                          DocumentMarker::MarkerType type,
                                          const String& description,
@@ skipping 72 matching lines @@
   TextIterator markedText(range.startPosition(), range.endPosition());
   DocumentMarkerController::removeMarkers(
       markedText, markerTypes, shouldRemovePartiallyOverlappingMarker);
 }
 
 static bool startsFurther(const Member<RenderedDocumentMarker>& lhv,
                           const DocumentMarker* rhv) {
   return lhv->startOffset() < rhv->startOffset();
 }
 
-static bool startsAfter(const Member<RenderedDocumentMarker>& marker,
-                        size_t startOffset) {
-  return marker->startOffset() < startOffset;
-}
-
 static bool endsBefore(size_t startOffset,
                        const Member<RenderedDocumentMarker>& rhv) {
   return startOffset < rhv->endOffset();
 }
 
 static bool compareByStart(const Member<DocumentMarker>& lhv,
                            const Member<DocumentMarker>& rhv) {
   return lhv->startOffset() < rhv->startOffset();
 }
 
@@ skipping 62 matching lines @@
         newMarker.type() != DocumentMarker::Composition) {
       mergeOverlapping(list.get(), newRenderedMarker);
     } else {
       MarkerList::iterator pos = std::lower_bound(list->begin(), list->end(),
                                                   &newMarker, startsFurther);
       list->insert(pos - list->begin(), newRenderedMarker);
     }
   }
 
   // repaint the affected node
-  if (node->layoutObject())
-    node->layoutObject()->setShouldDoFullPaintInvalidation();
+  if (node->layoutObject()) {
+    node->layoutObject()->setShouldDoFullPaintInvalidation(
+        PaintInvalidationDocumentMarkerChange);
+  }
 }
 
 void DocumentMarkerController::mergeOverlapping(
     MarkerList* list,
     RenderedDocumentMarker* toInsert) {
   MarkerList::iterator firstOverlapping =
       std::lower_bound(list->begin(), list->end(), toInsert, doesNotOverlap);
   size_t index = firstOverlapping - list->begin();
   list->insert(index, toInsert);
   MarkerList::iterator inserted = list->begin() + index;
@@ skipping 21 matching lines @@
 
   if (!possiblyHasMarkers(DocumentMarker::AllMarkers()))
     return;
   DCHECK(!m_markers.isEmpty());
 
   MarkerLists* markers = m_markers.get(srcNode);
   if (!markers)
     return;
 
   bool docDirty = false;
-  for (size_t markerListIndex = 0;
-       markerListIndex < DocumentMarker::MarkerTypeIndexesCount;
-       ++markerListIndex) {
-    Member<MarkerList>& list = (*markers)[markerListIndex];
+  for (Member<MarkerList> list : *markers) {
     if (!list)
       continue;
 
     unsigned endOffset = startOffset + length - 1;
     MarkerList::iterator startPos = std::lower_bound(
         list->begin(), list->end(), startOffset, doesNotInclude);
     for (MarkerList::iterator i = startPos; i != list->end(); ++i) {
       DocumentMarker* marker = i->get();
 
       // stop if we are now past the specified range
       if (marker->startOffset() > endOffset)
         break;
 
       // pin the marker to the specified range and apply the shift delta
       docDirty = true;
       if (marker->startOffset() < startOffset)
         marker->setStartOffset(startOffset);
       if (marker->endOffset() > endOffset)
         marker->setEndOffset(endOffset);
       marker->shiftOffsets(delta);
 
       addMarker(dstNode, *marker);
     }
   }
 
   // repaint the affected node
-  if (docDirty && dstNode->layoutObject())
-    dstNode->layoutObject()->setShouldDoFullPaintInvalidation();
+  if (docDirty && dstNode->layoutObject()) {
+    dstNode->layoutObject()->setShouldDoFullPaintInvalidation(
+        PaintInvalidationDocumentMarkerChange);
+  }
 }
 
 void DocumentMarkerController::removeMarkers(
     Node* node,
     unsigned startOffset,
     int length,
     DocumentMarker::MarkerTypes markerTypes,
     RemovePartiallyOverlappingMarkerOrNot
         shouldRemovePartiallyOverlappingMarker) {
   if (length <= 0)
@@ skipping 68 matching lines @@
     }
   }
 
   if (emptyListsCount == DocumentMarker::MarkerTypeIndexesCount) {
     m_markers.erase(node);
     if (m_markers.isEmpty())
       m_possiblyExistingMarkerTypes = 0;
   }
 
   // repaint the affected node
-  if (docDirty && node->layoutObject())
-    node->layoutObject()->setShouldDoFullPaintInvalidation();
+  if (docDirty && node->layoutObject()) {
+    node->layoutObject()->setShouldDoFullPaintInvalidation(
+        PaintInvalidationDocumentMarkerChange);
+  }
 }
 
 DocumentMarkerVector DocumentMarkerController::markersFor(
     Node* node,
     DocumentMarker::MarkerTypes markerTypes) {
   DocumentMarkerVector result;
 
   MarkerLists* markers = m_markers.get(node);
   if (!markers)
     return result;
@@ skipping 138 matching lines @@
 
       if (markerList->front()->type() == DocumentMarker::TextMatch)
         invalidatePaintForTickmarks(node);
     }
   }
 }
 
 DEFINE_TRACE(DocumentMarkerController) {
   visitor->trace(m_markers);
   visitor->trace(m_document);
+  SynchronousMutationObserver::trace(visitor);
 }
 
 void DocumentMarkerController::removeMarkers(
     Node* node,
     DocumentMarker::MarkerTypes markerTypes) {
   if (!possiblyHasMarkers(markerTypes))
     return;
   DCHECK(!m_markers.isEmpty());
 
   MarkerMap::iterator iterator = m_markers.find(node);
@@ skipping 77 matching lines @@
         needsRepainting = true;
       }
     }
 
     nodeCanBeRemoved =
         emptyListsCount == DocumentMarker::MarkerTypeIndexesCount;
   }
 
   if (needsRepainting) {
     const Node& node = *iterator->key;
-    if (LayoutObject* layoutObject = node.layoutObject())
-      layoutObject->setShouldDoFullPaintInvalidation();
+    if (LayoutObject* layoutObject = node.layoutObject()) {
+      layoutObject->setShouldDoFullPaintInvalidation(
+          PaintInvalidationDocumentMarkerChange);
+    }
     invalidatePaintForTickmarks(node);
   }
 
   if (nodeCanBeRemoved) {
     m_markers.remove(iterator);
     if (m_markers.isEmpty())
       m_possiblyExistingMarkerTypes = 0;
   }
 }
 
@@ skipping 13 matching lines @@
     for (size_t markerListIndex = 0;
          markerListIndex < DocumentMarker::MarkerTypeIndexesCount;
          ++markerListIndex) {
       Member<MarkerList>& list = (*markers)[markerListIndex];
       if (!list || list->isEmpty() ||
           !markerTypes.contains((*list->begin())->type()))
         continue;
 
       // cause the node to be redrawn
       if (LayoutObject* layoutObject = node->layoutObject()) {
-        layoutObject->setShouldDoFullPaintInvalidation();
+        layoutObject->setShouldDoFullPaintInvalidation(
+            PaintInvalidationDocumentMarkerChange);
         break;
       }
     }
   }
 }
 
-void DocumentMarkerController::shiftMarkers(Node* node,
-                                            unsigned startOffset,
-                                            int delta) {
-  if (!possiblyHasMarkers(DocumentMarker::AllMarkers()))
-    return;
-  DCHECK(!m_markers.isEmpty());
-
-  MarkerLists* markers = m_markers.get(node);
-  if (!markers)
-    return;
-
-  bool didShiftMarker = false;
-  for (size_t markerListIndex = 0;
-       markerListIndex < DocumentMarker::MarkerTypeIndexesCount;
-       ++markerListIndex) {
-    Member<MarkerList>& list = (*markers)[markerListIndex];
-    if (!list)
-      continue;
-    MarkerList::iterator startPos =
-        std::lower_bound(list->begin(), list->end(), startOffset, startsAfter);
-    for (MarkerList::iterator marker = startPos; marker != list->end();
-         ++marker) {
-#if DCHECK_IS_ON()
-      int startOffset = (*marker)->startOffset();
-      DCHECK_GE(startOffset + delta, 0);
-#endif
-      (*marker)->shiftOffsets(delta);
-      didShiftMarker = true;
-    }
-  }
-
-  if (didShiftMarker) {
-    invalidateRectsForMarkersInNode(*node);
-    // repaint the affected node
-    if (node->layoutObject())
-      node->layoutObject()->setShouldDoFullPaintInvalidation();
-  }
-}
-
 bool DocumentMarkerController::setMarkersActive(const EphemeralRange& range,
                                                 bool active) {
   if (!possiblyHasMarkers(DocumentMarker::AllMarkers()))
     return false;
 
   DCHECK(!m_markers.isEmpty());
 
   Node* const startContainer = range.startPosition().computeContainerNode();
   DCHECK(startContainer);
   Node* const endContainer = range.endPosition().computeContainerNode();
@@ skipping 33 matching lines @@
     // Markers are returned in order, so stop if we are now past the specified
     // range.
     if ((*marker)->startOffset() >= endOffset)
       break;
 
     (*marker)->setActiveMatch(active);
     docDirty = true;
   }
 
   // repaint the affected node
-  if (docDirty && node->layoutObject())
-    node->layoutObject()->setShouldDoFullPaintInvalidation();
+  if (docDirty && node->layoutObject()) {
+    node->layoutObject()->setShouldDoFullPaintInvalidation(
+        PaintInvalidationDocumentMarkerChange);
+  }
   return docDirty;
 }
 
 #ifndef NDEBUG
 void DocumentMarkerController::showMarkers() const {
   StringBuilder builder;
   MarkerMap::const_iterator end = m_markers.end();
   for (MarkerMap::const_iterator nodeIterator = m_markers.begin();
        nodeIterator != end; ++nodeIterator) {
     const Node* node = nodeIterator->key;
@@ skipping 17 matching lines @@
         builder.append(")");
       }
     }
     builder.append("\n");
   }
   LOG(INFO) << m_markers.size() << " nodes have markers:\n"
             << builder.toString().utf8().data();
 }
 #endif
 
+// SynchronousMutationObserver
+void DocumentMarkerController::didUpdateCharacterData(CharacterData* node,
+                                                      unsigned offset,
+                                                      unsigned oldLength,
+                                                      unsigned newLength) {
+  // If we're doing a pure remove operation, remove the markers in the range
+  // being removed (markers containing, but larger than, the range, will be
+  // split)
+  if (newLength == 0)
+    removeMarkers(node, offset, oldLength);
+
+  if (!possiblyHasMarkers(DocumentMarker::AllMarkers()))
+    return;
+  DCHECK(!m_markers.isEmpty());
+
+  MarkerLists* markers = m_markers.get(node);
+  if (!markers)
+    return;
+
+  bool didShiftMarker = false;
+  for (size_t markerListIndex = 0;

[Xiaocheng, 2017/02/22 23:39:28]

Can we use

for (MarkerList* list : markers)

It seems fine, as we do not remove |list| in the loop.

[rlanday, 2017/02/23 01:27:28]

Oops...thought I did this

+       markerListIndex < DocumentMarker::MarkerTypeIndexesCount;
+       ++markerListIndex) {
+    Member<MarkerList> list = (*markers)[markerListIndex];
+    if (!list)
+      continue;
+
+    for (MarkerList::iterator marker = list->begin(); marker != list->end();
+         ++marker) {
+      // if marker is contained by but not equal to the replaced range, remove
+      // the marker
+      if ((offset <= (*marker)->startOffset() &&
+           (*marker)->endOffset() < offset + oldLength) ||
+          (offset < (*marker)->startOffset() &&
+           (*marker)->endOffset() <= offset + oldLength)) {
+        list->remove(marker - list->begin());
+        --marker;
+        continue;
+      }
+
+      // algorithm inspired by https://dom.spec.whatwg.org/#concept-cd-replace
+      // but with some changes
+      if ((*marker)->startOffset() > offset) {
+        // Deviation from the concept-cd-replace algorithm: < instead of <= in
+        // the next line
+        if ((*marker)->startOffset() < offset + oldLength) {
+          // Marker start was in the replaced text. Move to end of new text
+          // (Deviation from the concept-cd-replace algorithm: that algorithm
+          // would move to the beginning of the new text here)
+          (*marker)->setStartOffset(offset + newLength);
+        } else {
+          // Marker start was after the replaced text. Shift by length
+          // difference
+          unsigned oldStartOffset = (*marker)->startOffset();
+          (*marker)->setStartOffset(oldStartOffset + newLength - oldLength);
+        }
+      }
+
+      if ((*marker)->endOffset() > offset) {
+        // Deviation from the concept-cd-replace algorithm: < instead of <= in
+        // the next line
+        if ((*marker)->endOffset() < offset + oldLength) {
+          // Marker end was in the replaced text. Move to beginning of new text
+          (*marker)->setEndOffset(offset);
+        } else {
+          // Marker end was after the replaced text. Shift by length difference
+          unsigned oldEndOffset = (*marker)->endOffset();
+          (*marker)->setEndOffset(oldEndOffset + newLength - oldLength);
+        }
+      }
+
+      if ((*marker)->startOffset() == (*marker)->endOffset()) {

[Xiaocheng, 2017/02/22 23:39:28]

The removal condition should be start >= end instead, as L877 moves start to the
end of the new text.

[rlanday, 2017/02/23 01:27:28]

Ah, good catch

[rlanday, 2017/02/23 01:55:52]

Actually I think the case where start > end can only happen if the replaced
region contains both the start marker (in which case we move the start marker to
the end of the new text) and the end marker (in which case we move the end
marker to the beginning of the text), and also is not exactly equal to the
region spanned by a marker (since we special-case that to put the marker around
the new text). So I think this is now redundant with the block under the "if
marker is contained by but not equal to the replaced range, remove the marker"
comment. I'll remove the block at the top since it makes the logic more complex.

+        list->remove(marker - list->begin());
+        --marker;
+        continue;
+      }
+
+      didShiftMarker = true;
+    }
+  }
+
+  if (didShiftMarker) {
+    invalidateRectsForMarkersInNode(*node);
+    // repaint the affected node
+    if (node->layoutObject()) {
+      node->layoutObject()->setShouldDoFullPaintInvalidation(
+          PaintInvalidationDocumentMarkerChange);
+    }
+  }
+}
+
 }  // namespace blink
 
 #ifndef NDEBUG
 void showDocumentMarkers(const blink::DocumentMarkerController* controller) {
   if (controller)
     controller->showMarkers();
 }
 #endif