Bug 132363

Summary: Make Blob RawData immutable
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: WebCore Misc.Assignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
none
proposed patch none

Description Alexey Proskuryakov 2014-04-29 15:59:15 PDT
RawData is essentially immutable, but its interface exposes a way to modify it.
Comment 1 Alexey Proskuryakov 2014-04-29 16:05:35 PDT
Created attachment 230431 [details]
proposed patch
Comment 2 Build Bot 2014-04-29 17:08:11 PDT
Comment on attachment 230431 [details]
proposed patch

Attachment 230431 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6236602707738624

New failing tests:
http/tests/local/blob/send-data-blob.html
Comment 3 Build Bot 2014-04-29 17:08:13 PDT
Created attachment 230442 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 4 Build Bot 2014-04-29 19:12:41 PDT
Comment on attachment 230431 [details]
proposed patch

Attachment 230431 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6483821025296384

New failing tests:
fast/files/read-blob-async.html
http/tests/local/blob/send-data-blob.html
fast/files/workers/worker-read-blob-sync.html
fast/files/workers/worker-read-blob-async.html
http/tests/local/blob/send-hybrid-blob.html
Comment 5 Build Bot 2014-04-29 19:12:43 PDT
Created attachment 230450 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 6 Alexey Proskuryakov 2014-04-29 22:28:51 PDT
Created attachment 230458 [details]
proposed patch
Comment 7 Anders Carlsson 2014-04-30 10:13:03 PDT
Comment on attachment 230458 [details]
proposed patch

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

> Source/WebCore/fileapi/WebKitBlobBuilder.cpp:-51
> -// FIXME: Move this file to BlobBuilder.cpp

I think you should keep this FIXME but change the name to whatever we want to rename it to.

> Source/WebCore/fileapi/WebKitBlobBuilder.h:-44
> -// FIXME: Move this file to BlobBuilder.h

I think you should keep this FIXME but change the name to whatever we want to rename it to.

> Source/WebCore/platform/network/BlobData.h:45
> +    static PassRefPtr<RawData> create(Vector<char>&& data)

No need for this to take an rvalue reference, it can just take a Vector<char> by value.
Comment 8 Alexey Proskuryakov 2014-04-30 10:20:41 PDT
Comment on attachment 230458 [details]
proposed patch

> I think you should keep this FIXME but change the name to whatever we want to rename it to.

But I don't know that yet :)

> No need for this to take an rvalue reference, it can just take a Vector<char> by value.

Talked this through in person. I want to ensure that constructing RawData never copies, so this is intentional.

But also, RawData will be going away in one of the next steps. So, just landing as is to not spend more effort on it.
Comment 9 WebKit Commit Bot 2014-04-30 10:50:32 PDT
Comment on attachment 230458 [details]
proposed patch

Clearing flags on attachment: 230458

Committed r168032: <http://trac.webkit.org/changeset/168032>
Comment 10 WebKit Commit Bot 2014-04-30 10:50:35 PDT
All reviewed patches have been landed.  Closing bug.