[webkit-reviews] review granted: [Bug 68286] [Chromium] We need an API to expose blob creation to Chromium : [Attachment 108056] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has granted Jay Civelli
<jcivelli at chromium.org>'s request for review:
Bug 68286: [Chromium] We need an API to expose blob creation to Chromium
https://bugs.webkit.org/show_bug.cgi?id=68286

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=108056&action=review


> Source/WebKit/chromium/public/WebBlob.h:51
> +    virtual ~WebBlob() { reset(); }

nit: this can be non-virtual

> Source/WebKit/chromium/public/WebBlob.h:54
> +    WebBlob(const WebBlob& n) { assign(n); }

nit: the "n" parameter name used in WebNode.h was chosen because "n" stands for
"node"
here, perhaps "b" would be a better choice.

> Source/WebKit/chromium/public/WebBlob.h:66
> +    WEBKIT_EXPORT bool equals(const WebBlob&) const;

nit: it's okay to define equals here, but you could also leave it out if you
like.
we don't universally provide this for simple WebCore wrappers.

> Source/WebKit/chromium/public/WebBlob.h:69
> +

nit: only one new line here

> Source/WebKit/chromium/public/WebBlob.h:77
> +#if WEBKIT_USING_V8

nit: we normally put WEBKIT_IMPLEMENTATION stuff last in the public section