WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
122737
CTTE: Add more node conversion helpers
https://bugs.webkit.org/show_bug.cgi?id=122737
Summary
CTTE: Add more node conversion helpers
Sam Weinig
Reported
2013-10-13 19:55:42 PDT
CTTE: Add more node conversion helpers
Attachments
Patch
(34.59 KB, patch)
2013-10-13 20:00 PDT
,
Sam Weinig
darin
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Sam Weinig
Comment 1
2013-10-13 20:00:05 PDT
Created
attachment 214122
[details]
Patch
Darin Adler
Comment 2
2013-10-13 20:37:53 PDT
Comment on
attachment 214122
[details]
Patch 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.
Sam Weinig
Comment 3
2013-10-14 09:50:44 PDT
Committed
r157405
: <
http://trac.webkit.org/changeset/157405
>
Brent Fulgham
Comment 4
2013-10-14 11:44:22 PDT
A follow-up patch to resolve a build error on Windows was landed here: Committed
r157410
: <
http://trac.webkit.org/changeset/157410
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug