[webkit-reviews] review granted: [Bug 108563] [Refactoring] StyleResolver::State should have methods to access its member variables. : [Attachment 187770] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Takashi Sakamoto
<tasak at google.com>'s request for review:
Bug 108563: [Refactoring] StyleResolver::State should have methods to access
its member variables.
https://bugs.webkit.org/show_bug.cgi?id=108563

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

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


> Source/WebCore/css/StyleResolver.h:464
>	   State()
> -	   : element(0)
> -	   , styledElement(0)
> -	   , parentNode(0)
> -	   , parentStyle(0)
> -	   , rootElementStyle(0)
> -	   , regionForStyling(0)
> -	   , sameOriginOnly(false)
> -	   , pseudoStyle(NOPSEUDO)
> -	   , elementLinkState(NotInsideLink)
> -	   , distributedToInsertionPoint(false)
> -	   , elementAffectedByClassRules(false)
> -	   , applyPropertyToRegularStyle(true)
> -	   , applyPropertyToVisitedLinkStyle(false)
> +	   : m_element(0)
> +	   , m_styledElement(0)
> +	   , m_parentNode(0)
> +	   , m_parentStyle(0)
> +	   , m_rootElementStyle(0)
> +	   , m_regionForStyling(0)
> +	   , m_sameOriginOnly(false)
> +	   , m_pseudoStyle(NOPSEUDO)
> +	   , m_elementLinkState(NotInsideLink)
> +	   , m_distributedToInsertionPoint(false)
> +	   , m_elementAffectedByClassRules(false)
> +	   , m_applyPropertyToRegularStyle(true)
> +	   , m_applyPropertyToVisitedLinkStyle(false)
> +#if ENABLE(CSS_SHADERS)
> +	   , m_hasPendingShaders(false)
> +#endif
> +	   , m_lineHeightValue(0)
> +	   , m_fontDirty(false)
> +	   , m_hasUAAppearance(false)
> +	   , m_backgroundData(BackgroundFillLayer) { }
> +
> +    public:
> +	   void initElement(Element*);
> +	   void initForStyleResolve(Document*, Element*, RenderStyle*
parentStyle = 0, PseudoId = NOPSEUDO, RenderRegion* regionForStyling = 0);
> +	   void clear()
> +	   {
> +	       m_element = 0;
> +	       m_styledElement = 0;
> +	       m_parentStyle = 0;
> +	       m_parentNode = 0;
> +	       m_regionForStyling = 0;
> +	       m_ruleList = 0;
> +	       m_matchedRules.clear();
> +	       m_pendingImageProperties.clear();
>  #if ENABLE(CSS_SHADERS)
> -	   , hasPendingShaders(false)
> +	       m_hasPendingShaders = false;
>  #endif
> -	   , lineHeightValue(0)
> -	   , fontDirty(false)
> -	   , hasUAAppearance(false)
> -	   , backgroundData(BackgroundFillLayer) { }
> +#if ENABLE(CSS_FILTERS) && ENABLE(SVG)
> +	       m_pendingSVGDocuments.clear();
> +#endif
> +	   }

You should move functions with more than one line to cpp (constructor, clear(),
ensureRuleList(), cacheBorderAndBackground(),..).

> Source/WebCore/css/StyleResolver.h:470
> +	   Document* document() const { return m_element->document(); }
> +	   Element* element() const { return m_element; }
> +	   StyledElement* styledElement() const { return m_styledElement; }
> +	   void setStyle(PassRefPtr<RenderStyle> style) { m_style = style; }
> +	   RenderStyle* style() const { return m_style.get(); }

Would it be possible to follow proper const here? 
const Foo* foo() const
or/and
Foo* foo() {}