Bug 132363 - Make Blob RawData immutable
Summary: Make Blob RawData immutable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-29 15:59 PDT by Alexey Proskuryakov
Modified: 2014-04-30 10:50 PDT (History)
3 users (show)

See Also:


Attachments
proposed patch (14.75 KB, patch)
2014-04-29 16:05 PDT, Alexey Proskuryakov
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (735.12 KB, application/zip)
2014-04-29 17:08 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (617.46 KB, application/zip)
2014-04-29 19:12 PDT, Build Bot
no flags Details
proposed patch (14.85 KB, patch)
2014-04-29 22:28 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.