[Webkit-unassigned] [Bug 121822] Move more style change code from RenderObject to RenderElement

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=121822


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #212424|review?                     |review+
               Flag|                            |




--- Comment #2 from Darin Adler <darin at apple.com>  2013-09-23 22:47:55 PST ---
(From update of attachment 212424)
View in context: https://bugs.webkit.org/attachment.cgi?id=212424&action=review

> Source/WebCore/ChangeLog:11
> +            RenderTexts are no longer registered as image clients. They don't need 

This sentence ends right in the middle of

> Source/WebCore/rendering/RenderElement.h:36
> +    virtual void setStyle(PassRefPtr<RenderStyle>);

I think you should mark this OVERRIDE, even though it’s "= 0" in the base class. Still useful to get an error if the function signatures don’t match. Like when we change from PassRefPtr to just RefPtr some day, if we do it wrong.

> Source/WebCore/rendering/RenderElement.h:82
> +    virtual void styleWillChange(StyleDifference, const RenderStyle*);
> +    virtual void styleDidChange(StyleDifference, const RenderStyle*);

If not OVERRIDE then does this still need to be virtual? Do derived classes override this?

> Source/WebCore/rendering/RenderElement.h:106
> +    void updateFillImages(const FillLayer*, const FillLayer*);
> +    void updateImage(StyleImage*, StyleImage*);
> +#if ENABLE(CSS_SHAPES)
> +    void updateShapeImage(const ShapeValue*, const ShapeValue*);
> +#endif
> +    StyleDifference adjustStyleDifference(StyleDifference, unsigned contextSensitiveProperties) const;

I’d like this better with a blank line after the #endif.

> Source/WebCore/rendering/RenderText.cpp:203
> +void RenderText::setStyle(PassRefPtr<RenderStyle> style)

Since RenderText objects inherit their style from their parents, isn’t there something even more efficient we can do?

> Source/WebCore/rendering/RenderText.cpp:210
> +    if (oldStyle.get())

Should not need a .get() here.

> Source/WebCore/rendering/RenderText.h:154
> +    virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);

Why is this still virtual, if it’s not an override? Are there classes derived from RenderText that override this?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.