Bug 132533 – Pass around RenderStyle by const-reference inside SVGInlineTextBox.

• Component: SVG
• Status: RESOLVED CONFIGURATION CHANGED

[Andreas Kling]

Functions that immediately start by asserting that some Foo* argument is non-null are silly.

[Andreas Kling]

Created attachment 230766 [details]
Patch

[Daniel Bates]

Comment on attachment 230766 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Reviewed by NOBODY (OOPS!).

The reviewed by line should precede the description of the change log entry.

> Source/WebCore/rendering/svg/SVGInlineTextBox.cpp:204
> +    const RenderStyle* selectionStyle = &style;

This local variables isn't used in a meaningful way in this function. I mean, we only assign a value(s) to it and never reference it. Shouldn't we be referencing this variable instead of |style| in SVGInlineTextBox::selectionRectForTextFragment() and GraphicsContext::fillRect() below?

> Source/WebCore/rendering/svg/SVGInlineTextBox.cpp:273
> +    const RenderStyle* selectionStyle = &style;
>      if (hasSelection) {
>          selectionStyle = parentRenderer.getCachedPseudoStyle(SELECTION);
>          if (selectionStyle) {

This is OK as-is. We should consider simplifying this code so as to remove an extraneous assignment to selectionStyle when parentRenderer.getCachedPseudoStyle(SELECTION) (on line 272) returns null. We can do this in another patch.

[Dirk Schulze]

Will you land this kling?

[Ahmad Saleem]

It seems that this r+ patch didn't landed.

Is this needed anymore? Thanks!

[Ahmad Saleem]

It seems to be done - https://searchfox.org/wubkat/rev/7cf0e3722de99a6fc09380aae193381be6733ebb/Source/WebCore/rendering/svg/SVGInlineTextBox.h#78

Marking this as 'REOSLVED CONFIGURATION CHANGED'.