Bug 103948 – Encapsulate ElementRareData for possible future sharing

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Elliott Sprehn]

Encapsulate ElementRareData for possible future sharing

[Elliott Sprehn]

Created attachment 177372 [details]
Patch

[Elliott Sprehn]

Created attachment 177375 [details]
Patch

[Darin Adler]

Comment on attachment 177375 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=177375&action=review

> Source/WebCore/dom/NodeRareData.h:-365
>  protected:
> -    // for ElementRareData
> -    bool needsFocusAppearanceUpdateSoonAfterAttach() const { return m_needsFocusAppearanceUpdateSoonAfterAttach; }
> -    void setNeedsFocusAppearanceUpdateSoonAfterAttach(bool needs) { m_needsFocusAppearanceUpdateSoonAfterAttach = needs; }
> -    bool styleAffectedByEmpty() const { return m_styleAffectedByEmpty; }
> -    void setStyleAffectedByEmpty(bool value) { m_styleAffectedByEmpty = value; }
> -    bool isInCanvasSubtree() const { return m_isInCanvasSubtree; }
> -    void setIsInCanvasSubtree(bool value) { m_isInCanvasSubtree = value; }
> -#if ENABLE(FULLSCREEN_API)
> -    bool containsFullScreenElement() { return m_containsFullScreenElement; }
> -    void setContainsFullScreenElement(bool value) { m_containsFullScreenElement = value; }
> -#endif
> -#if ENABLE(DIALOG_ELEMENT)
> -    bool isInTopLayer() const { return m_isInTopLayer; }
> -    void setIsInTopLayer(bool value) { m_isInTopLayer = value; }
> -#endif
> -    bool childrenAffectedByHover() const { return m_childrenAffectedByHover; }
> -    void setChildrenAffectedByHover(bool value) { m_childrenAffectedByHover = value; }
> -    bool childrenAffectedByActive() const { return m_childrenAffectedByActive; }
> -    void setChildrenAffectedByActive(bool value) { m_childrenAffectedByActive = value; }
> -    bool childrenAffectedByDrag() const { return m_childrenAffectedByDrag; }
> -    void setChildrenAffectedByDrag(bool value) { m_childrenAffectedByDrag = value; }
> -
> -    bool childrenAffectedByFirstChildRules() const { return m_childrenAffectedByFirstChildRules; }
> -    void setChildrenAffectedByFirstChildRules(bool value) { m_childrenAffectedByFirstChildRules = value; }
> -    bool childrenAffectedByLastChildRules() const { return m_childrenAffectedByLastChildRules; }
> -    void setChildrenAffectedByLastChildRules(bool value) { m_childrenAffectedByLastChildRules = value; }
> -    bool childrenAffectedByDirectAdjacentRules() const { return m_childrenAffectedByDirectAdjacentRules; }
> -    void setChildrenAffectedByDirectAdjacentRules(bool value) { m_childrenAffectedByDirectAdjacentRules = value; }
> -    bool childrenAffectedByForwardPositionalRules() const { return m_childrenAffectedByForwardPositionalRules; }
> -    void setChildrenAffectedByForwardPositionalRules(bool value) { m_childrenAffectedByForwardPositionalRules = value; }
> -    bool childrenAffectedByBackwardPositionalRules() const { return m_childrenAffectedByBackwardPositionalRules; }
> -    void setChildrenAffectedByBackwardPositionalRules(bool value) { m_childrenAffectedByBackwardPositionalRules = value; }
> -    unsigned childIndex() const { return m_childIndex; }
> -    void setChildIndex(unsigned index) { m_childIndex = index; }
> -
> -private:

Why this change? Your change log does not make clear why this is desirable.

[Elliott Sprehn]

(In reply to comment #3)
> (From update of attachment 177375 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=177375&action=review
> ..
> 
> Why this change? Your change log does not make clear why this is desirable.

It's less code and is more clear. Putting all the methods in NodeRareData and then "using" them in ElementRareData means every method name is duplicated and if you're looking for a method on ElementRareData you first look there and then have to look in another file. The properties are in NodeRareData for packing reasons, but that doesn't mean the methods need to be.

[Eric Seidel (no email)]

Comment on attachment 177375 [details]
Patch

This looks great.  I don't think you meant to make everything protected, as Darin notes.

[Elliott Sprehn]

(In reply to comment #5)
> (From update of attachment 177375 [details])
> This looks great.  I don't think you meant to make everything protected, as Darin notes.

Not sure what you mean. The fields in NodeRareData need to be protected so they're accessible from ElementRareData where I moved the methods to remove the giant block of "using" statements.

[Eric Seidel (no email)]

Comment on attachment 177375 [details]
Patch

ok

[WebKit Review Bot]

Comment on attachment 177375 [details]
Patch

Clearing flags on attachment: 177375

Committed r136738: <http://trac.webkit.org/changeset/136738>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.