[webkit-reviews] review granted: [Bug 124571] Generate toHTMLFooElement() to clean up static_cast<> : [Attachment 217288] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Gyuyoung Kim
<gyuyoung.kim at samsung.com>'s request for review:
Bug 124571: Generate toHTMLFooElement() to clean up static_cast<>
https://bugs.webkit.org/show_bug.cgi?id=124571

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=217288&action=review


Patch is fine as is. I see some obvious opportunities for cleanup that could be
done as part of this or separately.

> Source/WebCore/html/HTMLTableElement.cpp:76
>      for (Node* child = firstChild(); child; child = child->nextSibling()) {
>	   if (child->hasTagName(captionTag))
> -	       return static_cast<HTMLTableCaptionElement*>(child);
> +	       return toHTMLTableCaptionElement(child);
>      }
>      return 0;

This entire function should just be:

    return childrenOfType<HTMLTableCaptionElement>(*this).first();

No need for a loop at all.

> Source/WebCore/html/HTMLTableRowElement.cpp:84
> +	       HTMLTableSectionElement* section =
toHTMLTableSectionElement(node);

Should use a reference rather than a pointer here for this local.

> Source/WebCore/html/MediaDocument.cpp:96
> +    HTMLSourceElement* source = toHTMLSourceElement(sourceElement.get());

Should use a reference rather than a pointer here for this local.

> Source/WebCore/html/shadow/DetailsMarkerControl.cpp:67
>      Element* element = shadowHost();
> -    ASSERT_WITH_SECURITY_IMPLICATION(!element ||
element->hasTagName(summaryTag));
> -    return static_cast<HTMLSummaryElement*>(element);
> +    return toHTMLSummaryElement(element);

Doesn’t seem like we need the local variable here any more.

> Source/WebCore/page/SpatialNavigation.cpp:761
> +    return candidate.isFrameOwnerElement() ?
toHTMLFrameOwnerElement(candidate.visibleNode) : 0;

nullptr