[webkit-reviews] review granted: [Bug 122737] CTTE: Add more node conversion helpers : [Attachment 214122] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 122737: CTTE: Add more node conversion helpers
https://bugs.webkit.org/show_bug.cgi?id=122737

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

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


Way to many of these casts are being done on pointers when we know the pointer
is non-null and the cast should be on a reference. I would have made that
change in many of the call sites you touched here.

> Source/WebCore/dom/Attr.h:106
> +inline bool isAttr(const Node& node) { return node.isAttributeNode(); }
> +void isAttr(const Attr&); // Catch unnecessary runtime check of type known
at compile time.

For some reason, when adding these for the various element classes I always put
these in the opposite order, with the most specific class first and heading
down to less and less specific ones afterward.

> Source/WebCore/dom/CDATASection.h:46
> +void isCDATASection(const ContainerNode&); // Catch unnecessary runtime
check of type known at compile time.

Wonder how many other ones like this we should do? Clearly we could have a ton
of these.

> Source/WebCore/dom/Document.h:1627
> +inline bool isDocument(const Node& node) { return node.isDocumentNode(); }
> +void isDocument(const Document&); // Catch unnecessary runtime check of type
known at compile time.
>  NODE_TYPE_CASTS(Document)

In other places we left a blank line before NODE_TYPE_CASTS.

> Source/WebCore/dom/Node.h:721
> +#define TYPE_CASTS_IMPL(ToClassName, FromClassName) \

IMPL is so ugly. Maybe TYPE_CASTS_SUITE? I dunno.