Bug 152066

Summary: [Streams API] Expose ReadableStream and relatives to Worker
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, calvaris, commit-queue, darin, rniwa
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 152465, 152535    
Bug Blocks:    
Attachments:
Description Flags
WIP
none
Patch
none
Rebasing worker attribute test expectation
none
Trying to fix win build
none
Patch for landing
none
Rebasing
none
Archive of layout-test-results from ews103 for mac-yosemite
none
Archive of layout-test-results from ews107 for mac-yosemite-wk2
none
Archive of layout-test-results from ews112 for mac-yosemite
none
Rebasing missing test expectations none

Description youenn fablet 2015-12-09 07:07:38 PST
ReadableStream should be usable in workers.
Comment 1 youenn fablet 2015-12-09 07:08:57 PST
Created attachment 267001 [details]
WIP
Comment 2 youenn fablet 2015-12-10 05:31:23 PST
Created attachment 267094 [details]
Patch
Comment 3 youenn fablet 2015-12-10 06:10:20 PST
Created attachment 267101 [details]
Rebasing worker attribute test expectation
Comment 4 youenn fablet 2015-12-10 08:04:28 PST
Created attachment 267109 [details]
Trying to fix win build
Comment 5 youenn fablet 2015-12-10 08:09:05 PST
(In reply to comment #4)
> Created attachment 267109 [details]
> Trying to fix win build

Patch enables streams API in worker.
It also refactors built-in code to extract in a separate file code that the builtin generator should produce once bug 150482 is landed.

Tests seem to pass well on Mac bots.
One test is timing out on my machine for WebKitGTK (templated.js).
I did not have yet time to study this issue.
Comment 6 Xabier Rodríguez Calvar 2015-12-10 08:31:57 PST
Comment on attachment 267109 [details]
Trying to fix win build

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

As usual, my informal review. Nice work.

Really nice to see all those tests passing now.

> Source/WebCore/bindings/js/JSDOMGlobalObject.cpp:49
>      , m_world(world)
>      , m_worldIsNormal(m_world->isNormal())
> +    , m_internalFunctions(vm)

Alphabetic order?

> Source/WebCore/bindings/js/JSDOMGlobalObject.cpp:71
> +        GlobalPropertyInfo(clientData.builtinNames().streamClosedPrivateName(), jsNumber(1), DontDelete | ReadOnly),
> +        GlobalPropertyInfo(clientData.builtinNames().streamClosingPrivateName(), jsNumber(2), DontDelete | ReadOnly),
> +        GlobalPropertyInfo(clientData.builtinNames().streamErroredPrivateName(), jsNumber(3), DontDelete | ReadOnly),
> +        GlobalPropertyInfo(clientData.builtinNames().streamReadablePrivateName(), jsNumber(4), DontDelete | ReadOnly),
> +        GlobalPropertyInfo(clientData.builtinNames().streamWaitingPrivateName(), jsNumber(5), DontDelete | ReadOnly),
> +        GlobalPropertyInfo(clientData.builtinNames().streamWritablePrivateName(), jsNumber(6), DontDelete | ReadOnly),
> +        GlobalPropertyInfo(clientData.builtinNames().ReadableStreamControllerPrivateName(), createReadableStreamControllerPrivateConstructor(vm, *this), DontDelete | ReadOnly),

I wonder if we should create an enumeration for the numbers.

> Source/WebCore/bindings/js/WebCoreJSBuiltinInternals.cpp:49
> +#if ENABLE(STREAMS_API)
> +    , m_readableStreamInternalsFunctions(vm)
> +    , m_streamInternalsFunctions(vm)
> +    , m_writableStreamInternalsFunctions(vm)
> +#endif
> +#if ENABLE(MEDIA_STREAM)
> +    , m_rTCPeerConnectionInternalsFunctions(vm)
> +#endif

Since we are breaking the global alphabetic order to group the attributes by condition, we should sort the conditions and put STREAMS_API after MEDIA_STREAM

> Source/WebCore/bindings/js/WebCoreJSBuiltinInternals.cpp:65
> +#if ENABLE(STREAMS_API)
> +    m_readableStreamInternalsFunctions.visit(visitor);
> +    m_streamInternalsFunctions.visit(visitor);
> +    m_writableStreamInternalsFunctions.visit(visitor);
> +#endif
> +#if ENABLE(MEDIA_STREAM)
> +    m_rTCPeerConnectionInternalsFunctions.visit(visitor);
> +#endif
> +#ifndef SKIP_UNUSED_PARAM
> +    UNUSED_PARAM(visitor);
> +#endif

Ditto.

> Source/WebCore/bindings/js/WebCoreJSBuiltinInternals.cpp:77
> +#if ENABLE(STREAMS_API)
> +    m_readableStreamInternalsFunctions.init(globalObject);
> +    m_streamInternalsFunctions.init(globalObject);
> +    m_writableStreamInternalsFunctions.init(globalObject);
> +#endif
> +#if ENABLE(MEDIA_STREAM)
> +    m_rTCPeerConnectionInternalsFunctions.init(globalObject);
> +#endif

Ditto.

> Source/WebCore/bindings/js/WebCoreJSBuiltinInternals.cpp:106
> +#if ENABLE(STREAMS_API)
> +#define DECLARE_GLOBAL_STATIC(name)\
> +        JSDOMGlobalObject::GlobalPropertyInfo(\
> +            clientData.builtinFunctions().readableStreamInternalsBuiltins().name##PrivateName(), readableStreamInternals().m_##name##Function.get() , DontDelete | ReadOnly),
> +        WEBCORE_FOREACH_READABLESTREAMINTERNALS_BUILTIN_FUNCTION_NAME(DECLARE_GLOBAL_STATIC)
> +#undef DECLARE_GLOBAL_STATIC
> +#define DECLARE_GLOBAL_STATIC(name)\
> +        JSDOMGlobalObject::GlobalPropertyInfo(\
> +            clientData.builtinFunctions().streamInternalsBuiltins().name##PrivateName(), streamInternals().m_##name##Function.get() , DontDelete | ReadOnly),
> +        WEBCORE_FOREACH_STREAMINTERNALS_BUILTIN_FUNCTION_NAME(DECLARE_GLOBAL_STATIC)
> +#undef DECLARE_GLOBAL_STATIC
> +#define DECLARE_GLOBAL_STATIC(name)\
> +        JSDOMGlobalObject::GlobalPropertyInfo(\
> +            clientData.builtinFunctions().writableStreamInternalsBuiltins().name##PrivateName(), writableStreamInternals().m_##name##Function.get() , DontDelete | ReadOnly),
> +        WEBCORE_FOREACH_WRITABLESTREAMINTERNALS_BUILTIN_FUNCTION_NAME(DECLARE_GLOBAL_STATIC)
> +#undef DECLARE_GLOBAL_STATIC
> +#endif
> +#if ENABLE(MEDIA_STREAM)
> +#define DECLARE_GLOBAL_STATIC(name)\
> +        JSDOMGlobalObject::GlobalPropertyInfo(\
> +            clientData.builtinFunctions().rTCPeerConnectionInternalsBuiltins().name##PrivateName(), rTCPeerConnectionInternals().m_##name##Function.get() , DontDelete | ReadOnly),
> +        WEBCORE_FOREACH_RTCPEERCONNECTIONINTERNALS_BUILTIN_FUNCTION_NAME(DECLARE_GLOBAL_STATIC)
> +#undef DECLARE_GLOBAL_STATIC
> +#endif

Ditto.
Comment 7 Xabier Rodríguez Calvar 2015-12-10 08:35:06 PST
Comment on attachment 267109 [details]
Trying to fix win build

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

> Source/WebCore/bindings/js/JSDOMGlobalObject.h:93
> +        JSBuiltinInternalFunctions m_internalFunctions;

Of course, if you're planning to keep this separation, forget about the order in the constructor initialization :)
Comment 8 youenn fablet 2015-12-10 09:59:52 PST
Thanks for the feedback.

I am not yet sure how would look the generated code.

Sorting according the macro sounds OK.
Comment 9 Darin Adler 2015-12-13 15:09:34 PST
Comment on attachment 267109 [details]
Trying to fix win build

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

>> Source/WebCore/bindings/js/WebCoreJSBuiltinInternals.cpp:49
>> +#endif
> 
> Since we are breaking the global alphabetic order to group the attributes by condition, we should sort the conditions and put STREAMS_API after MEDIA_STREAM

I agree.
Comment 10 Xabier Rodríguez Calvar 2015-12-14 05:12:03 PST
Created attachment 267289 [details]
Patch for landing
Comment 11 WebKit Commit Bot 2015-12-14 07:58:15 PST
Comment on attachment 267289 [details]
Patch for landing

Clearing flags on attachment: 267289

Committed r194033: <http://trac.webkit.org/changeset/194033>
Comment 12 WebKit Commit Bot 2015-12-14 07:58:19 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 WebKit Commit Bot 2015-12-20 02:23:56 PST
Re-opened since this is blocked by bug 152465
Comment 14 youenn fablet 2016-01-14 03:50:21 PST
Created attachment 268952 [details]
Rebasing
Comment 15 youenn fablet 2016-01-14 03:53:26 PST
(In reply to comment #14)
> Created attachment 268952 [details]
> Rebasing

imported/w3c/web-platform-tests/streams-api/readable-streams/templated.html is still timing out in my GTK release build, probably related with bug 152340.
Let's see what EWS bots think.
Comment 16 Build Bot 2016-01-14 04:42:33 PST
Comment on attachment 268952 [details]
Rebasing

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

New failing tests:
imported/w3c/web-platform-tests/streams-api/readable-streams/cancel.html
imported/w3c/web-platform-tests/streams-api/readable-streams/bad-strategies.html
Comment 17 Build Bot 2016-01-14 04:42:35 PST
Created attachment 268954 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 18 Build Bot 2016-01-14 04:45:58 PST
Comment on attachment 268952 [details]
Rebasing

Attachment 268952 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/690021

New failing tests:
imported/w3c/web-platform-tests/streams-api/readable-streams/cancel.html
imported/w3c/web-platform-tests/streams-api/readable-streams/bad-strategies.html
Comment 19 Build Bot 2016-01-14 04:46:01 PST
Created attachment 268955 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 20 Build Bot 2016-01-14 04:56:34 PST
Comment on attachment 268952 [details]
Rebasing

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

New failing tests:
imported/w3c/web-platform-tests/streams-api/readable-streams/cancel.html
imported/w3c/web-platform-tests/streams-api/readable-streams/bad-strategies.html
Comment 21 Build Bot 2016-01-14 04:56:36 PST
Created attachment 268956 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 22 youenn fablet 2016-01-14 05:05:56 PST
Created attachment 268957 [details]
Rebasing missing test expectations
Comment 23 WebKit Commit Bot 2016-01-15 01:14:50 PST
Comment on attachment 268957 [details]
Rebasing missing test expectations

Clearing flags on attachment: 268957

Committed r195101: <http://trac.webkit.org/changeset/195101>
Comment 24 WebKit Commit Bot 2016-01-15 01:14:53 PST
All reviewed patches have been landed.  Closing bug.