[DMC #11] Move some rendered rect method impls from DMC To TextMatchMarkerListImpl

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 ef5e4029fb55fedbc9ddf78377d328900de5380a..44ceca684b888423a86591137f476273ba45bd3f 100644
--- a/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
+++ b/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
@@ -408,7 +408,11 @@ DocumentMarkerVector DocumentMarkerController::Markers() {
 }
 
 Vector<IntRect> DocumentMarkerController::RenderedRectsForTextMatchMarkers() {
+  DCHECK(!document_->View() || !document_->View()->NeedsLayout());

[yosin_UTC9, 2017/05/22 05:24:18]

We should not call this function when |document_->View() == nullptr|.
Did you find the case of |document_->View() == nullptr|?

[rlanday, 2017/05/22 05:29:11]

I'm just moving this check up from UpdateMarkerRenderedRectIfNeeded() (since it
has to be in DMC)...do you think we should change the DCHECK to verify that
document_->View() != nullptr?

[yosin_UTC9, 2017/05/22 05:54:26]

Let's try. And change call sites not to do so.

+  DCHECK(!document_->NeedsLayoutTreeUpdate());
+
   Vector<IntRect> result;
+
   if (!PossiblyHasMarkers(DocumentMarker::kTextMatch))
     return result;
   DCHECK(!(markers_.IsEmpty()));
@@ -424,32 +428,40 @@ Vector<IntRect> DocumentMarkerController::RenderedRectsForTextMatchMarkers() {
     MarkerLists* markers = node_iterator->value.Get();
     DocumentMarkerList* const list =
         ListForType(markers, DocumentMarker::kTextMatch);
-    if (!list || list->IsEmpty())
+    if (!list)
       continue;
-
-    for (DocumentMarker* marker : list->GetMarkers()) {
-      RenderedDocumentMarker* const rendered_marker =
-          ToRenderedDocumentMarker(marker);
-      UpdateMarkerRenderedRectIfNeeded(node, *rendered_marker);
-      if (!rendered_marker->IsRendered())
-        continue;
-      result.push_back(rendered_marker->RenderedRect());
-    }
+    ToTextMatchMarkerListImpl(list)->AppendRenderedRectsToVector(node, &result);

[yosin_UTC9, 2017/05/22 05:24:18]

We can use |Vector<T>::AppendVector()| as |DocumentMarkerController::Markers()|.

[rlanday, 2017/05/22 05:29:11]

I think that would involve constructing a temporary vector and then copying,
which would involve some efficiency loss; is that OK?

[yosin_UTC9, 2017/05/22 05:54:26]

OK. Let's do cleaner implementation.
When perf testing says temporary vector makes slower, we think better way.

   }
 
   return result;
 }
 
+// TODO(rlanday): move this to TextMatchMarkerListImpl.cpp
+void TextMatchMarkerListImpl::AppendRenderedRectsToVector(
+    const Node& node,
+    Vector<IntRect>* result) {
+  if (IsEmpty())
+    return;
+
+  for (DocumentMarker* marker : markers_) {
+    RenderedDocumentMarker* const rendered_marker =
+        ToRenderedDocumentMarker(marker);
+    UpdateMarkerRenderedRectIfNeeded(node, *rendered_marker);
+    if (!rendered_marker->IsRendered())
+      continue;
+    result->push_back(rendered_marker->RenderedRect());
+  }
+}
+
 static void InvalidatePaintForTickmarks(const Node& node) {
   if (FrameView* frame_view = node.GetDocument().View())
     frame_view->InvalidatePaintForTickmarks();
 }
 
-void DocumentMarkerController::UpdateMarkerRenderedRectIfNeeded(
+// TODO(rlanday): move this to TextMatchMarkerListImpl.cpp
+void TextMatchMarkerListImpl::UpdateMarkerRenderedRectIfNeeded(
     const Node& node,
     RenderedDocumentMarker& marker) {
-  DCHECK(!document_->View() || !document_->View()->NeedsLayout());
-  DCHECK(!document_->NeedsLayoutTreeUpdate());
   if (!marker.IsValid())
     UpdateMarkerRenderedRect(node, marker);
 }