Make DocumentMarkerListEditor::RemoveMarkers() more efficient

The current implementation of this method does a binary search to get the first
marker to be removed, then linearly walks the markers, erasing all that fall in
the specified range. This can potentially take time quadratic in the number of
markers to be removed, and always takes at least log N time.

This new implementation finds both the start point and end point of the range to
be erased in log N time, then does a single erase operation that takes time
linear in the number of markers to be removed. So the asymptotic lower bound
stays the same and the asymptotic upper bound is improved from quadratic to
linear.

I've also added a few more test cases to DocumentMarkerListEditor test to verify
the new implementation works correctly.

BUG=707867

Review-Url: https://codereview.chromium.org/2922623002
Cr-Commit-Position: refs/heads/master@{#476551}

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

@@ -52,32 +52,24 @@ bool DocumentMarkerListEditor::MoveMarkers(MarkerList* src_list,
   return didMoveMarker;
 }
 
-// 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;
   const unsigned end_offset = start_offset + length;
   MarkerList::iterator start_pos = std::upper_bound(
       list->begin(), list->end(), start_offset,
       [](size_t start_offset, const Member<DocumentMarker>& marker) {
         return start_offset < marker->EndOffset();
       });
-  for (MarkerList::iterator i = start_pos; i != list->end();) {
-    const 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;
-  }
+  MarkerList::iterator end_pos = std::lower_bound(
+      list->begin(), list->end(), end_offset,
+      [](const Member<DocumentMarker>& marker, size_t end_offset) {
+        return marker->StartOffset() < end_offset;
+      });
 
-  return doc_dirty;
+  list->erase(start_pos - list->begin(), end_pos - start_pos);
+  return start_pos != end_pos;
 }
 
 // TODO(rlanday): make this not take O(n^2) time when all the markers are

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

@@ -17,6 +17,40 @@ class DocumentMarkerListEditorTest : public ::testing::Test {
   }
 };
 
+TEST_F(DocumentMarkerListEditorTest, RemoveMarkersEmptyList) {
+  DocumentMarkerListEditor::MarkerList markers;
+  DocumentMarkerListEditor::RemoveMarkers(&markers, 0, 10);
+  EXPECT_EQ(0u, markers.size());
+}
+
+TEST_F(DocumentMarkerListEditorTest, RemoveMarkersTouchingEndpoints) {
+  DocumentMarkerListEditor::MarkerList markers;
+  markers.push_back(CreateMarker(0, 10));
+  markers.push_back(CreateMarker(10, 20));
+  markers.push_back(CreateMarker(20, 30));
+
+  DocumentMarkerListEditor::RemoveMarkers(&markers, 10, 10);
+
+  EXPECT_EQ(2u, markers.size());
+
+  EXPECT_EQ(0u, markers[0]->StartOffset());
+  EXPECT_EQ(10u, markers[0]->EndOffset());
+
+  EXPECT_EQ(20u, markers[1]->StartOffset());
+  EXPECT_EQ(30u, markers[1]->EndOffset());
+}
+
+TEST_F(DocumentMarkerListEditorTest, RemoveMarkersOneCharacterIntoInterior) {
+  DocumentMarkerListEditor::MarkerList markers;
+  markers.push_back(CreateMarker(0, 10));
+  markers.push_back(CreateMarker(10, 20));
+  markers.push_back(CreateMarker(20, 30));
+
+  DocumentMarkerListEditor::RemoveMarkers(&markers, 9, 12);
+
+  EXPECT_EQ(0u, markers.size());
+}
+
 TEST_F(DocumentMarkerListEditorTest,
        ContentDependentMarker_ReplaceStartOfMarker) {
   DocumentMarkerListEditor::MarkerList markers;