[webkit-reviews] review granted: [Bug 132533] Pass around RenderStyle by const-reference inside SVGInlineTextBox. : [Attachment 230766] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Andreas Kling <akling at apple.com>'s
request for review:
Bug 132533: Pass around RenderStyle by const-reference inside SVGInlineTextBox.
https://bugs.webkit.org/show_bug.cgi?id=132533

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

------- Additional Comments from Daniel Bates <dbates at webkit.org>
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.