Bug 118235 – Adopt is/toHTMLMapElement for code cleanup

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Kangil Han]

Adopt is/toHTMLMapElement for code cleanup

[Kangil Han]

Created attachment 205796 [details]
Patch

[Andreas Kling]

Comment on attachment 205796 [details]
Patch

r=me

It's a little bit unfortunate that the isFooElement(Node*) helpers incur the cost of an isElementNode() check that wouldn't be necessary if the argument were an Element*. Perhaps we could templatize the method so the compiler would skip the check when the argument is of a narrower type. Food for thought.

[Kangil Han]

(In reply to comment #2)
> 
> It's a little bit unfortunate that the isFooElement(Node*) helpers incur the cost of an isElementNode() check that wouldn't be necessary if the argument were an Element*. Perhaps we could templatize the method so the compiler would skip the check when the argument is of a narrower type. Food for thought.

Oops, you are right. :(
IMHO, thinking about node-element inheritance has led a choice to use Node* as an argument of isFooElement. Though hasTagName() is not a virtual function. :(

How about to take Element* as an argument of is/toFooElement?
Plus why don't we make Element::hasTagName an inline function?

[Kangil Han]

(In reply to comment #3)
> Plus why don't we make Element::hasTagName an inline function?

My bad. :(
We can't make this function an inline.
Sorry to confuse you. :)

[Andreas Kling]

(In reply to comment #4)
> (In reply to comment #3)
> > Plus why don't we make Element::hasTagName an inline function?
> 
> My bad. :(
> We can't make this function an inline.
> Sorry to confuse you. :)

Element::hasTagName() is already an inline function, defined in Element.h

It could be nice to use Element* arguments instead, though I suspect that would require a lot of type checking at various call sites.

[Kangil Han]

(In reply to comment #5)
> 
> It could be nice to use Element* arguments instead, though I suspect that would require a lot of type checking at various call sites.

I guess so. :(
Anyway, I will try it and let you know. :)

[Kangil Han]

Created attachment 206047 [details]
Patch

[Kangil Han]

(In reply to comment #2)
> (From update of attachment 205796 [details])
> r=me
> 
> It's a little bit unfortunate that the isFooElement(Node*) helpers incur the cost of an isElementNode() check that wouldn't be necessary if the argument were an Element*. Perhaps we could templatize the method so the compiler would skip the check when the argument is of a narrower type. Food for thought.

Done by appending overloading isFooElement(Element*) inline function. :)
Thx!

[WebKit Commit Bot]

Comment on attachment 206047 [details]
Patch

Clearing flags on attachment: 206047

Committed r152387: <http://trac.webkit.org/changeset/152387>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.