Very little of it is needed for RenderText.
Created attachment 212424 [details] patch
Comment on attachment 212424 [details] patch 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?
> This sentence ends right in the middle of That's because I > 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. Yeah. > > > 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? Yes, they have overrides. > Since RenderText objects inherit their style from their parents, isn’t there something even more efficient we can do? The plan is to move m_style pointer to RenderElement and have RenderText just use the parent style. That will require a few more patches. > > 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? Surprisingly there are RenderText subclasses and there is an override for this.
https://trac.webkit.org/r156325