[webkit-reviews] review denied: [Bug 101448] Move childrenAffectedBy bits from RenderStyle to Element : [Attachment 176265] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has denied Allan Sandfeld Jensen
<allan.jensen at digia.com>'s request for review:
Bug 101448: Move childrenAffectedBy bits from RenderStyle to Element
https://bugs.webkit.org/show_bug.cgi?id=101448

Attachment 176265: Patch
https://bugs.webkit.org/attachment.cgi?id=176265&action=review

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=176265&action=review


r-, all accessors should move up to Element. Looks good otherwise.

> Source/WebCore/ChangeLog:5
> +	   https://bugs.webkit.org/show_bug.cgi?id=101448
> +	   https://bugs.webkit.org/show_bug.cgi?id=98021

You should probably dupe one of these bugs to another.

> Source/WebCore/dom/Element.cpp:1963
> +bool Element::childrenAffectedByFirstChildRules() const
> +{
> +    return hasRareData() &&
elementRareData()->childrenAffectedByFirstChildRules();
> +}

Might make sense to inline the getters though it is ok to do it if this
actually shows up in profiles.

> Source/WebCore/dom/Element.cpp:1968
> +void Element::setChildrenAffectedByFirstChildRules()
> +{
> +    ensureElementRareData()->setChildrenAffectedByFirstChildRules(true);
> +}

I'm still slightly worried that a bad rule can make large number of Elements
have RareData. I guess we'll see if this is a real problem.

We have some room in the Node bitfield and more could be made. Perhaps the most
popular bits could go there instead of RareData.

> Source/WebCore/dom/Node.h:556
> +    bool childrenAffectedByHover() const;
> +    void setChildrenAffectedByHover(bool);
> +    bool childrenAffectedByActive() const;
> +    void setChildrenAffectedByActive(bool);
> +    bool childrenAffectedByDrag() const;
> +    void setChildrenAffectedByDrag(bool);

I don't think these bits can be set for non-elements either. The accessors
should be in Element and the clients should cast.