Bug 47701 – Support appending an ArrayBuffer object in BlobBuilder

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Jian Li]

We need to support appending an ArrayBuffer object in BlobBuilder per the latest File API: Writer spec:
  http://dev.w3.org/2009/dap/file-system/file-writer.html#idl-def-BlobBuilder

[Jian Li]

Created attachment 70908 [details]
Proposed Patch

[WebKit Review Bot]

Attachment 70908 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/fileapi/BlobBuilder.cpp:44:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Attachment 70908 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4388054

[Jian Li]

Created attachment 70913 [details]
Proposed Patch

[Kinuko Yasuda]

Comment on attachment 70913 [details]
Proposed Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=70913&action=review

This change looks good to me.

> WebCore/fileapi/BlobBuilder.cpp:68
> +        return false;

I've lost the track of why we've been returning bool from those append methods (I'm afraid it's from my code).  If there're no other reasons maybe we can convert all the append to return void?

[Jian Li]

(In reply to comment #5)
> (From update of attachment 70913 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=70913&action=review
> 
> This change looks good to me.
> 
> > WebCore/fileapi/BlobBuilder.cpp:68
> > +        return false;
> 
> I've lost the track of why we've been returning bool from those append methods (I'm afraid it's from my code).  If there're no other reasons maybe we can convert all the append to return void?

I think we can remove the return type because I am not seeing it is used from the append method in JS/V8BlobBuilder.cpp. I am going to submit a new patch.

[Jian Li]

Created attachment 71106 [details]
Proposed Patch

[Kinuko Yasuda]

Comment on attachment 71106 [details]
Proposed Patch

This looks good to me.  Thanks for fixing append's return types.

[David Levin]

Comment on attachment 71106 [details]
Proposed Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=71106&action=review

> WebCore/ChangeLog:7
> +

It looks like bool return values have been removed. I guess this is because all functions returned true and the return values weren't used.

It would be nice to have a small comment about this (since it really isn't covered by the title).

[Jian Li]

Committed as http://trac.webkit.org/changeset/70488.