RESOLVED FIXED 90050
Rename Element::rareData() to Element::elementRareData(), and Element::ensureRareData() to Element::ensureElementRareData()
https://bugs.webkit.org/show_bug.cgi?id=90050
Summary Rename Element::rareData() to Element::elementRareData(), and Element::ensure...
Kentaro Hara
Reported 2012-06-26 23:33:11 PDT
Element::rareData()/Element::ensureRareData() and Node::rareData()/Node::ensureRareData() are confusing. They are not virtual methods. For clarification, we can rename Element::rareData() to Element::elementRareData(), and Element::ensureRareData() to Element::ensureElementRareData(). c.f. SVGRareData uses SVGElement::rareSVGData() and SVGElement::ensureRareSVGData(). (We might want to rename them to SVGElement::svgRareData() and SVGElement::ensureSVGRareData() though.)
Attachments
Patch (12.76 KB, patch)
2012-06-26 23:37 PDT, Kentaro Hara
no flags
Kentaro Hara
Comment 1 2012-06-26 23:37:20 PDT
Hajime Morrita
Comment 2 2012-06-27 00:43:10 PDT
Is that confusing? How could you misuse it? Another random idea is to unify then to rareData(bool ensure);
Kentaro Hara
Comment 3 2012-06-27 00:52:40 PDT
(In reply to comment #2) > Is that confusing? How could you misuse it? Misuse would lead to a compile error (because Node::rareData() returns NodeRareData and Element::rareData() returns ElementRareData), and thus it would not cause any serious issue in consequence. That being said, distinguishing their names would be less confusing. For example, SVGElement uses rareSVGData() to retrieve SVGRareData (I am trying to rename it to svgRareData() though). Specific context: I've tried to virtualize Node::rareData() to cache NodeRareData* on Document to optimize Dromaeo. Then Element::rareData() prevented me from doing the virtualization. (Whether the optimization is acceptable or not is another topic.)
Ryosuke Niwa
Comment 4 2012-06-27 01:07:29 PDT
(In reply to comment #3) > Specific context: I've tried to virtualize Node::rareData() to cache NodeRareData* on Document to optimize Dromaeo. Then Element::rareData() prevented me from doing the virtualization. (Whether the optimization is acceptable or not is another topic.) I don't think we want to virtualize rareData(). That function is already slow. We shouldn't be making it even slower by virtualizing it.
Ryosuke Niwa
Comment 5 2012-06-27 01:11:34 PDT
Comment on attachment 149690 [details] Patch Regardless, this sounds like a good idea.
Kentaro Hara
Comment 6 2012-06-27 01:12:28 PDT
morrita-san: If you think the change is OK, I'm happy to commit it.
WebKit Review Bot
Comment 7 2012-06-27 03:22:13 PDT
Comment on attachment 149690 [details] Patch Clearing flags on attachment: 149690 Committed r121335: <http://trac.webkit.org/changeset/121335>
WebKit Review Bot
Comment 8 2012-06-27 03:22:18 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.