| Summary: | Adopt is/toHTMLMapElement for code cleanup | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kangil Han <kangil.han> | ||||||
| Component: | WebCore Misc. | Assignee: | Kangil Han <kangil.han> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, esprehn+autocc, jdiggs, kling, koivisto, mario | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Kangil Han
2013-07-01 04:12:40 PDT
Created attachment 205796 [details]
Patch
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.
(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? (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. :) (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. (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. :) Created attachment 206047 [details]
Patch
(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! Comment on attachment 206047 [details] Patch Clearing flags on attachment: 206047 Committed r152387: <http://trac.webkit.org/changeset/152387> All reviewed patches have been landed. Closing bug. |