Currently DocumentMarker is treat as a value class. But this has following problems: - It is wasteful. We are doing unnecessary copying. - It loose its entity. A living entity of DocumentMarker object is actually a RenderedDocumentMarker, a subclass of DM. It would have benefits if we could access it. Using a DocumentMarker object as a pointer solves these problems. Concretely, DocumentMarkerController::markersForNode() and markersInRange() should return a vector of pointers.
Created attachment 94385 [details] Patch
Comment on attachment 94385 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=94385&action=review Is it possible for us to put OwnPtr in the vector to make the ownership > Source/WebCore/dom/DocumentMarkerController.cpp:317 > +Vector<DocumentMarker*> DocumentMarkerController::markersFor(Node* node) Would this work as a Vector<const DocumentMarker*>? > Source/WebCore/rendering/InlineTextBox.cpp:1092 > + Vector<DocumentMarker*>::iterator markerIt = markers.begin(); Nit: Could this be a const_iterator?
Hi Tony, thank you for giving a review! (In reply to comment #2) > (From update of attachment 94385 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=94385&action=review > > Is it possible for us to put OwnPtr in the vector to make the ownership For the return value, no it isn't. The markers are owned by DocumentMarkerController. For DocumentMarkerController::m_markers, I'm not sure. I'll try it in another patch. > > > Source/WebCore/dom/DocumentMarkerController.cpp:317 > > +Vector<DocumentMarker*> DocumentMarkerController::markersFor(Node* node) No, because we want to call some setter of returned markers. (That is a motivation of this change.) > > Would this work as a Vector<const DocumentMarker*>? > > > Source/WebCore/rendering/InlineTextBox.cpp:1092 > > + Vector<DocumentMarker*>::iterator markerIt = markers.begin(); > > Nit: Could this be a const_iterator? Sure. I'll fix this before landing.
Committed r87483: <http://trac.webkit.org/changeset/87483>