Bug 47701 - Support appending an ArrayBuffer object in BlobBuilder
Summary: Support appending an ArrayBuffer object in BlobBuilder
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Jian Li
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-14 16:51 PDT by Jian Li
Modified: 2010-10-25 13:46 PDT (History)
7 users (show)

See Also:

Attachments
Proposed Patch (12.33 KB, patch)
2010-10-15 15:52 PDT, Jian Li 		jianli: commit-queue-
Details | Formatted Diff | Diff
Proposed Patch (12.38 KB, patch)
2010-10-15 16:17 PDT, Jian Li 		jianli: commit-queue-
Details | Formatted Diff | Diff
Proposed Patch (12.90 KB, patch)
2010-10-18 17:36 PDT, Jian Li 		levin: review+
jianli: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jian Li 2010-10-14 16:51:07 PDT 
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
Comment 1 Jian Li 2010-10-15 15:52:03 PDT 
Created attachment 70908 [details]
Proposed Patch
Comment 2 WebKit Review Bot 2010-10-15 15:54:50 PDT 
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.
Comment 3 Early Warning System Bot 2010-10-15 16:08:59 PDT 
Attachment 70908 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4388054
Comment 4 Jian Li 2010-10-15 16:17:31 PDT 
Created attachment 70913 [details]
Proposed Patch
Comment 5 Kinuko Yasuda 2010-10-18 17:01:00 PDT 
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?
Comment 6 Jian Li 2010-10-18 17:19:57 PDT 
(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.
Comment 7 Jian Li 2010-10-18 17:36:43 PDT 
Created attachment 71106 [details]
Proposed Patch
Comment 8 Kinuko Yasuda 2010-10-19 16:09:20 PDT 
Comment on attachment 71106 [details]
Proposed Patch

This looks good to me.  Thanks for fixing append's return types.
Comment 9 David Levin 2010-10-21 19:54:40 PDT 
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).
Comment 10 Jian Li 2010-10-25 13:46:49 PDT 
Committed as http://trac.webkit.org/changeset/70488.