[webkit-reviews] review granted: [Bug 179218] Replace some auto* with RefPtr within WebCore/html : [Attachment 325839] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 2 21:24:08 PDT 2017


Ryosuke Niwa <rniwa at webkit.org> has granted Jiewen Tan <jiewen_tan at apple.com>'s
request for review:
Bug 179218: Replace some auto* with RefPtr within WebCore/html
https://bugs.webkit.org/show_bug.cgi?id=179218

Attachment 325839: Patch

https://bugs.webkit.org/attachment.cgi?id=325839&action=review




--- Comment #22 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 325839
  --> https://bugs.webkit.org/attachment.cgi?id=325839
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=325839&action=review

> Source/WebCore/accessibility/AccessibilityTable.cpp:396
> +	   if (RefPtr<HTMLTableCaptionElement> caption =
tableElement->caption()) {

Use auto?

> Source/WebCore/accessibility/AccessibilityTable.cpp:690
> -	   if (HTMLTableCaptionElement* caption =
downcast<HTMLTableElement>(*tableElement).caption())
> +	   if (RefPtr<HTMLTableCaptionElement> caption =
downcast<HTMLTableElement>(*tableElement).caption())

Ditto.

> Source/WebCore/html/HTMLSummaryElement.cpp:76
> +    auto parent = makeRefPtr(parentElement());

It's a bit silly to use a RefPtr here since we're just downcast'ing to another
pointer type.
I think it's okay to keep parent & host raw pointers as long as the return type
is a RefPtr.

> Source/WebCore/html/HTMLTableRowElement.cpp:59
> -    auto* parent = row.parentNode();
> +    auto parent = makeRefPtr(row.parentNode());

Again, not certain if it's worth converting these local variables to RefPtr
since they're just casted to another pointer type.


More information about the webkit-reviews mailing list