[webkit-reviews] review granted: [Bug 23042] Remove reflection repaint hack in computeAbsoluteRepaintRect() : [Attachment 26322] Patch, changelog, testcase

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 23042: Remove reflection repaint hack in computeAbsoluteRepaintRect()
https://bugs.webkit.org/show_bug.cgi?id=23042

Attachment 26322: Patch, changelog, testcase
https://bugs.webkit.org/attachment.cgi?id=26322&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    if (hasReflection()) {
> +	   IntRect rectInReflection = reflectedRect(rect);
> +	   rect.unite(rectInReflection);
>      }

I think this would read better without the local variable:

    if (hasReflection())
	rect.unite(reflectedRect(rect));

> +    IntRect result;
> +    if (!m_style->boxReflect())
> +	   return result;
> +
> +    IntRect box = borderBox();
> +    result = r;

I don't think it's so great to use that empty result in this way. I'd write it
like this:

    if (!m_style->boxReflect())
	return IntRect();

    IntRect box = borderBox();
    IntRect result = r;

r=me