[webkit-reviews] review granted: [Bug 59855] DocumentMarker: Type specific details should be separately held by other object. : [Attachment 103020] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 4 18:59:11 PDT 2011


Ryosuke Niwa <rniwa at webkit.org> has granted MORITA Hajime
<morrita at google.com>'s request for review:
Bug 59855: DocumentMarker: Type specific details should be separately held by
other object.
https://bugs.webkit.org/show_bug.cgi?id=59855

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=103020&action=review


> Source/WebCore/dom/DocumentMarker.cpp:59
> +PassRefPtr<DocumentMarkerDescription>
DocumentMarkerDescription::create(const String& description)
> +{
> +    return adoptRef(new DocumentMarkerDescription(description));
> +}

Should we make this inline?

> Source/WebCore/dom/DocumentMarker.cpp:96
> +    if (details && details->isTextMatch())
> +	   return static_cast<DocumentMarkerTextMatch*>(details);
> +    return 0;

I would have used tertiary for this.

> Source/WebCore/dom/DocumentMarker.cpp:101
> +    : m_type(Spelling), m_startOffset(0), m_endOffset(0)

Shouldn't they be on separate lines?

> Source/WebCore/dom/DocumentMarker.cpp:106
> +    : m_type(type), m_startOffset(startOffset), m_endOffset(endOffset)

Ditto.


More information about the webkit-reviews mailing list