DocumentMarkerList refactor as an interface

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

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef DocumentMarkerListEditor_h
+#define DocumentMarkerListEditor_h
+
+#include "core/editing/markers/DocumentMarkerList.h"
+
+namespace blink {
+
+class DocumentMarkerListEditor {
+ public:
+  static DocumentMarkerList::iterator
+  getPosOfFirstMarkerNotEndingBeforeInSortedList(

[rlanday, 2017/04/07 21:46:29]

This method is only used once in this file and probably won't be needed anywhere
else...maybe I should just make it a lambda?

[Xiaocheng, 2017/04/07 23:42:24]

If it's used only once and internally, making it a lambda is better.

It's always good to keep header files small.

[rlanday, 2017/04/11 01:08:59]

I stand corrected, it's also used in
TextMatchMarkerList::SetTextMatchMarkersActive()

[Xiaocheng, 2017/04/11 01:26:01]

In this case, I don't have a strong preference.

I slightly prefer hiding it because of its long name.

+      DocumentMarkerVector* markerList,
+      size_t startOffset);
+
+  static void appendMarkersToInputList(const DocumentMarkerVector& markerList,

[rlanday, 2017/04/07 21:46:29]

This method isn't really an editing method...does this not belong here?

[Xiaocheng, 2017/04/07 23:42:24]

I think this method doesn't do much. We should make the callers call
Vector::appendVector directly.

+                                       DocumentMarkerVector* inputList);
+
+  static DocumentMarkerList::DidCopyMarkerOrNot copyUnsortedMarkers(

[Xiaocheng, 2017/04/07 23:42:24]

It's better to use output to an empty DMV so that this function does not depend
on DML.

Btw, I think it's better to make it something like moveSorted/UnsortedMarkers.

+      DocumentMarkerVector* markerList,
+      unsigned startOffset,
+      int length,
+      DocumentMarkerList* dstList,
+      int delta);
+  static DocumentMarkerList::DidCopyMarkerOrNot copySortedMarkers(
+      DocumentMarkerVector* markerList,
+      unsigned startOffset,
+      int length,
+      DocumentMarkerList* dstList,
+      int delta);
+
+  static DocumentMarkerList::DidRemoveMarkerOrNot removeUnsortedMarkers(
+      DocumentMarkerVector* markerList,
+      unsigned startOffset,
+      int length,
+      bool shouldRemovePartiallyOverlappingMarkers);
+  static DocumentMarkerList::DidRemoveMarkerOrNot removeSortedMarkers(
+      DocumentMarkerVector* markerList,
+      unsigned startOffset,
+      int length,
+      bool shouldRemovePartiallyOverlappingMarkers);
+
+  static DocumentMarkerList::DidShiftMarkerOrNot shiftUnsortedMarkers(
+      DocumentMarkerVector* markerList,
+      unsigned offset,
+      unsigned oldLength,
+      unsigned newLength);
+  static DocumentMarkerList::DidShiftMarkerOrNot shiftSortedMarkers(
+      DocumentMarkerVector* markerList,
+      unsigned offset,
+      unsigned oldLength,
+      unsigned newLength);
+
+ private:
+  DocumentMarkerListEditor();
+
+  static DocumentMarkerList::DidCopyMarkerOrNot copyMarkersHelper(

[Xiaocheng, 2017/04/07 23:42:24]

It doesn't make too much sense to introduce a static private function.

Just hide it in an anonymous namespace in the .cpp file.

+      DocumentMarkerVector* markerList,
+      DocumentMarkerList::iterator beginIt,
+      DocumentMarkerList::iterator endIt,
+      unsigned startOffset,
+      int length,
+      DocumentMarkerList* dstList,
+      int delta);
+};
+
+}  // namespace blink
+
+#endif  // DocumentMarkerListEditor_h