[webkit-reviews] review denied: [Bug 43992] [chromium] scrolling issues when accelerated compositor is enabled : [Attachment 64544] got rid of a redundant entry in the Changelog; removed unnecessary parentheses

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Alexey Marinichev
<amarinichev at chromium.org>'s request for review:
Bug 43992: [chromium] scrolling issues when accelerated compositor is enabled
https://bugs.webkit.org/show_bug.cgi?id=43992

Attachment 64544: got rid of a redundant entry in the Changelog; removed
unnecessary parentheses
https://bugs.webkit.org/attachment.cgi?id=64544&action=review

------- Additional Comments from David Levin <levin at chromium.org>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 163f6c5..cbaecba 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,20 @@
> +2010-08-16  Alexey Marinichev  <amarinichev at chromium.org>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   [chromium] scrolling issues when accelerated compositor is enabled
> +	   https://bugs.webkit.org/show_bug.cgi?id=43992

It would be nice to have some explanation of why the int cast/floorf call is
being removed. (Is that the off-by-half bug?)
> +

Is it possible to create a layout test for this (that verifies the fix when
accelerated compositing is on)?


> diff --git a/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
b/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
> +    } else if (abs(scrollDelta.y()) > contentRect.height() ||
abs(scrollDelta.x()) > contentRect.width())
> +	   // Scrolling larger than the contentRect size does not preserve any
of the pixels, so there is
> +	   // no need to copy framebuffer pixels back into the texture.
> +	   m_scrollPosition = scrollPosition;

This should have {} around it since the else clause is more than one logical
line.