[webkit-reviews] review granted: [Bug 44383] Encapsulate document marker management into DocumentMarkerController : [Attachment 65035] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Aug 21 14:35:05 PDT 2010


Eric Seidel <eric at webkit.org> has granted Daniel Bates <dbates at webkit.org>'s
request for review:
Bug 44383: Encapsulate document marker management into DocumentMarkerController
https://bugs.webkit.org/show_bug.cgi?id=44383

Attachment 65035: Patch
https://bugs.webkit.org/attachment.cgi?id=65035&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Yay!  Thank you Dan!

WebCore/dom/Document.h:1173
 +	mutable DocumentMarkerController m_markerController;
It's better for build times if this is an OwnPtr instead.  When its a mutable
member you have to include the header in Document.h.   The only reason (I know
of) to make these mutable members is to avoid the pointer indirection in hot
code paths.  But I don't think DocumentMarkers are hot.  This is not a deal
breaker either way.

WebCore/dom/DocumentMarkerController.h:48
 +	void copyMarkers(Node *srcNode, unsigned startOffset, int length, Node
*dstNode, int delta, DocumentMarker::MarkerType = DocumentMarker::AllMarkers);
Style.

WebCore/editing/Editor.cpp:1566
 +	   
frame()->document()->markerController()->removeMarkers(selectedRange.get(),
DocumentMarker::Spelling);
I might have called the markerController() just markers().

LGTM.


More information about the webkit-reviews mailing list