Add support for persisting CompositionUnderlines in InputMethodController

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 6b0a0edc9c8a29c5891c186b0a702465493e72b6..5cd3240552d113942b1c69f800ea4273b680d939 100644
--- a/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
+++ b/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
@@ -71,6 +71,8 @@ DocumentMarker::MarkerTypeIndex MarkerTypeToMarkerIndex(
       return DocumentMarker::InvisibleSpellcheckMarkerIndex;
     case DocumentMarker::Composition:
       return DocumentMarker::CompositionMarkerIndex;
+    case DocumentMarker::PersistingComposition:
+      return DocumentMarker::PersistingCompositionMarkerIndex;
   }
 
   NOTREACHED();
@@ -128,15 +130,17 @@ void DocumentMarkerController::addCompositionMarker(const Position& start,
                                                     const Position& end,
                                                     Color underlineColor,
                                                     bool thick,
-                                                    Color backgroundColor) {
+                                                    Color backgroundColor,
+                                                    bool persist) {
   DCHECK(!m_document->needsLayoutTreeUpdate());
 
   for (TextIterator markedText(start, end); !markedText.atEnd();
-       markedText.advance())
+       markedText.advance()) {
     addMarker(markedText.currentContainer(),
               DocumentMarker(markedText.startOffsetInCurrentContainer(),
                              markedText.endOffsetInCurrentContainer(),
-                             underlineColor, thick, backgroundColor));
+                             underlineColor, thick, backgroundColor, persist));
+  }
 }
 
 void DocumentMarkerController::prepareForDestruction() {
@@ -254,7 +258,8 @@ void DocumentMarkerController::addMarker(Node* node,
     list->push_back(newRenderedMarker);
   } else {
     if (newMarker.type() != DocumentMarker::TextMatch &&
-        newMarker.type() != DocumentMarker::Composition) {
+        newMarker.type() != DocumentMarker::Composition &&
+        newMarker.type() != DocumentMarker::PersistingComposition) {
       mergeOverlapping(list.get(), newRenderedMarker);
     } else {
       MarkerList::iterator pos = std::lower_bound(list->begin(), list->end(),
@@ -727,8 +732,13 @@ void DocumentMarkerController::repaintMarkers(
 }
 
 void DocumentMarkerController::shiftMarkers(Node* node,
-                                            unsigned startOffset,
-                                            int delta) {
+                                            int startOffset,
+                                            int prevLength,
+                                            int newLength) {
+  DCHECK_GE(startOffset, 0);
+  DCHECK_GE(prevLength, 0);
+  DCHECK_GE(newLength, 0);
+
   if (!possiblyHasMarkers(DocumentMarker::AllMarkers()))
     return;
   DCHECK(!m_markers.isEmpty());
@@ -744,15 +754,30 @@ void DocumentMarkerController::shiftMarkers(Node* node,
     Member<MarkerList>& list = (*markers)[markerListIndex];
     if (!list)
       continue;
+
+    // algorithm from https://dom.spec.whatwg.org/#concept-cd-replace
     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);

[rlanday, 2017/01/03 21:07:03]

The existing code here seems to not be able to deal with the amount of
generality I need for my use case. There was some sequence of editing operations
I could do that would cause the beginning of a marker range to be deleted, and
this code would try to shift the offsets so the range had a negative start
value, which was not correct.

I don't think I'm changing the behavior for anything currently supported, only
better supporting this case where a range is partially deleted/replaced.

+      if ((*marker)->startOffset() <= (unsigned)(startOffset + prevLength)) {
+        // Marker start was in the replaced text. Move to beginning of new text
+        (*marker)->setStartOffset(startOffset);
+      } else {
+        // Marker start was after the replaced text. Shift by length difference
+        unsigned oldStartOffset = (*marker)->startOffset();
+        (*marker)->setStartOffset(oldStartOffset + newLength - prevLength);
+      }
+
+      if ((*marker)->endOffset() <= (unsigned)(startOffset + prevLength)) {
+        // Marker end was in the replaced text. Move to beginning of new text
+        (*marker)->setEndOffset(startOffset);
+      } else {
+        // Marker end was after the replaced text. Shift by length difference
+        unsigned oldEndOffset = (*marker)->endOffset();
+        (*marker)->setEndOffset(oldEndOffset + newLength - prevLength);
+      }
+
       didShiftMarker = true;
     }
   }