[webkit-reviews] review denied: [Bug 39085] Resized Images are not cached in Chromium (Skia) : [Attachment 56035] Fix for this bug.

[bugzilla-daemon at webkit.org]

Dmitry Titov <dimich at chromium.org> has denied Hin-Chung Lam
<hclam at google.com>'s request for review:
Bug 39085: Resized Images are not cached in Chromium (Skia)
https://bugs.webkit.org/show_bug.cgi?id=39085

Attachment 56035: Fix for this bug.
https://bugs.webkit.org/attachment.cgi?id=56035&action=review

------- Additional Comments from Dmitry Titov <dimich at chromium.org>
I can't check the functional validity. If Brett thinks it's ok and layout tests
with pixel compare don't fail it could be ok.

Some style issues:
The change needs a ChangeLog (hence r-)


> +		   ||
bitmap->shouldCacheResampling(static_cast<int>(destBitmapWidth),
static_cast<int>(destBitmapHeight),
> +						   
static_cast<int>(destBitmapWidth), static_cast<int>(destBitmapHeight))) ) {
I don't think the space before the closing ')' is needed.

Could I suggest all those casts to be moved to the top, initializing some local
'destWidth' and 'destHeight', so we don't have a dozen of them repeated
everywhere?

> +	       // resize() caches resized images.
> +	       resampled =
bitmap->resizedBitmap(static_cast<int>(destBitmapWidth),
> +		   static_cast<int>(destBitmapHeight));

Once the casts are gone, this line can be un-wrapped.