[webkit-reviews] review denied: [Bug 40018] Image Resizer Patch 1: Adding basic classes to perform the resizing : [Attachment 58265] Fixing build errors

[bugzilla-daemon at webkit.org]

Dmitry Titov <dimich at chromium.org> has denied Sterling Swigart
<sswigart at google.com>'s request for review:
Bug 40018: Image Resizer Patch 1: Adding basic classes to perform the resizing
https://bugs.webkit.org/show_bug.cgi?id=40018

Attachment 58265: Fixing build errors
https://bugs.webkit.org/attachment.cgi?id=58265&action=review

------- Additional Comments from Dmitry Titov <dimich at chromium.org>
I believe those can be final ones: 

WebCore/html/AsyncImageResizer.h:51
 +  class AsyncImageResizer : public CachedResourceClient, public
RefCounted<AsyncImageResizer> {
I think AsyncImageResizer should be ThreadSafeShared rather the RefCounted
since its refcounted from 2 threads and the code is not structured specifically
to provide safe RefCounted operation.

WebCore/html/ImageResizerThread.h:46
 +	struct CallbackInfo {
For this struct, I'd have a constructor to make sure all fields are
initialized. Also, it seems 'bool error' is not necessary since blob==0 could
do the same. Also, it's unclear what error==true, blob!=0 would mean. Could we
just remove error and use !blob as indicator of error? At least for now, before
we actually need it.

WebCore/html/ImageResizerThread.h:52
 +	static bool start(RefPtr<SharedBuffer> imageData,
RefPtr<AsyncImageResizer>, AsyncImageResizer::OutputType, IntSize
desiredBounds, float quality, AsyncImageResizer::AspectRatioOption,
AsyncImageResizer::OrientationOption);
usually parameters are PassRefPtr<T> to avoi creating RefPtrs and churning
refcount.

WebCore/html/ImageResizerThread.h:56
 +	ImageResizerThread(RefPtr<AsyncImageResizer>,
AsyncImageResizer::OutputType, IntSize desiredBounds, float quality,
AsyncImageResizer::AspectRatioOption, AsyncImageResizer::OrientationOption);
Ditto about PassRefPtr