Bug 175866 – Add support for ReadableStream storage in FetchBody

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

This will allow better support of request/cache API stream manipulation.

[youenn fablet]

Created attachment 318829 [details]
Patch

[Build Bot]

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


ERROR: Source/WebCore/bindings/js/ReadableStream.h:42:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 14 files


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

[youenn fablet]

Created attachment 318834 [details]
Patch

[Build Bot]

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


ERROR: Source/WebCore/bindings/js/ReadableStream.h:42:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 14 files


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

[Build Bot]

Comment on attachment 318834 [details]
Patch

Attachment 318834 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4365417

New failing tests:
media/track/track-element-dom-change-crash.html

[Build Bot]

Created attachment 318846 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[youenn fablet]

Failure is unrelated to this patch.
I went with the approach of having IDLInterface<ReadableStream>, which works and has the benefit to keep the union conversion semantics the same.
This also automatically creates a Dom guarded object which is probably always wanted since processing ReadableStream can only be done asynchronously.

The alternative might be to introduce something like IDLReadableStream, but we might need to special case the binding generator for it and we would have to update the checks for the union conversion so that IDLReadableStream is part of the IDLInterface conversion.

[Sam Weinig]

Comment on attachment 318834 [details]
Patch

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

> Source/WebCore/Modules/fetch/FetchBody.cpp:67
> +    }, [&](RefPtr<ReadableStream>&) mutable {
> +        FetchBody body;
> +        body.m_isReadableStream = true;
> +        return body;

Now that ReadableStream has a form, can FetchBody store it? The 'm_isReadableStream' has always been a bit confusing.

> Source/WebCore/Modules/fetch/FetchRequest.idl:34
> -// FIXME: Should include ReadableStream
>  typedef (Blob or BufferSource or DOMFormData or URLSearchParams or USVString) BodyInit;

Should this not include ReadableStream?

[youenn fablet]

Thanks for the review.

> View in context:
> https://bugs.webkit.org/attachment.cgi?id=318834&action=review
> 
> > Source/WebCore/Modules/fetch/FetchBody.cpp:67
> > +    }, [&](RefPtr<ReadableStream>&) mutable {
> > +        FetchBody body;
> > +        body.m_isReadableStream = true;
> > +        return body;
> 
> Now that ReadableStream has a form, can FetchBody store it? The
> 'm_isReadableStream' has always been a bit confusing.

Yes, that is the plan.

> 
> > Source/WebCore/Modules/fetch/FetchRequest.idl:34
> > -// FIXME: Should include ReadableStream
> >  typedef (Blob or BufferSource or DOMFormData or URLSearchParams or USVString) BodyInit;
> 
> Should this not include ReadableStream?

Will fix it.

[youenn fablet]

Created attachment 318987 [details]
Patch for landing

[Build Bot]

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


ERROR: Source/WebCore/bindings/js/ReadableStream.h:42:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 15 files


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

[Build Bot]

Comment on attachment 318987 [details]
Patch for landing

Attachment 318987 [details] did not pass bindings-ews (mac):
Output: http://webkit-queues.webkit.org/results/4375353

New failing tests:
Please see test output for results

[youenn fablet]

Created attachment 318988 [details]
Patch for landing

[Build Bot]

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


ERROR: Source/WebCore/bindings/js/ReadableStream.h:42:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 15 files


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

[WebKit Commit Bot]

Comment on attachment 318988 [details]
Patch for landing

Rejecting attachment 318988 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 318988, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
.webkit.org/git/WebKit
   60a3be7..786241c  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 221196 = 60a3be7be0314d49e5410e88f8f612aa61843bd3
r221197 = 786241ced815bc6a0eae8b6b2281848520660e1a
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/4382158

[WebKit Commit Bot]

Comment on attachment 318988 [details]
Patch for landing

Clearing flags on attachment: 318988

Committed r221201: <http://trac.webkit.org/changeset/221201>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34087334>