Bug 90050 - Rename Element::rareData() to Element::elementRareData(), and Element::ensureRareData() to Element::ensureElementRareData()
Summary: Rename Element::rareData() to Element::elementRareData(), and Element::ensure...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-26 23:33 PDT by Kentaro Hara
Modified: 2012-06-27 03:22 PDT (History)
5 users (show)

See Also:


Attachments
Patch (12.76 KB, patch)
2012-06-26 23:37 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 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.)
Comment 1 Kentaro Hara 2012-06-26 23:37:20 PDT
Created attachment 149690 [details]
Patch
Comment 2 Hajime Morrita 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);
Comment 3 Kentaro Hara 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.)
Comment 4 Ryosuke Niwa 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.
Comment 5 Ryosuke Niwa 2012-06-27 01:11:34 PDT
Comment on attachment 149690 [details]
Patch

Regardless, this sounds like a good idea.
Comment 6 Kentaro Hara 2012-06-27 01:12:28 PDT
morrita-san: If you think the change is OK, I'm happy to commit it.
Comment 7 WebKit Review Bot 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>
Comment 8 WebKit Review Bot 2012-06-27 03:22:18 PDT
All reviewed patches have been landed.  Closing bug.