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

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 390 matching lines @@
   }
   std::sort(result.begin(), result.end(),
             [](const Member<DocumentMarker>& marker1,
                const Member<DocumentMarker>& marker2) {
               return marker1->StartOffset() < marker2->StartOffset();
             });
   return result;
 }
 
 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()));
 
   // outer loop: process each node
   MarkerMap::iterator end = markers_.end();
   for (MarkerMap::iterator node_iterator = markers_.begin();
        node_iterator != end; ++node_iterator) {
     // inner loop; process each marker in this node
     const Node& node = *node_iterator->key;
     if (!node.isConnected())
       continue;
     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);
 }
 
 void DocumentMarkerController::InvalidateRectsForTextMatchMarkersInNode(
     const Node& node) {
   MarkerLists* markers = markers_.at(&node);
 
   const DocumentMarkerList* const marker_list =
       ListForType(markers, DocumentMarker::kTextMatch);
@@ skipping 281 matching lines @@
 }
 
 }  // namespace blink
 
 #ifndef NDEBUG
 void showDocumentMarkers(const blink::DocumentMarkerController* controller) {
   if (controller)
     controller->ShowMarkers();
 }
 #endif