Bug 152066 – [Streams API] Expose ReadableStream and relatives to Worker

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[youenn fablet]

ReadableStream should be usable in workers.

[youenn fablet]

Created attachment 267001 [details]
WIP

[youenn fablet]

Created attachment 267094 [details]
Patch

[youenn fablet]

Created attachment 267101 [details]
Rebasing worker attribute test expectation

[youenn fablet]

Created attachment 267109 [details]
Trying to fix win build

[youenn fablet]

(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.

[Xabier Rodríguez Calvar]

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.

[Xabier Rodríguez Calvar]

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 :)

[youenn fablet]

Thanks for the feedback.

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

Sorting according the macro sounds OK.

[Darin Adler]

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.

[Xabier Rodríguez Calvar]

Created attachment 267289 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 267289 [details]
Patch for landing

Clearing flags on attachment: 267289

Committed r194033: <http://trac.webkit.org/changeset/194033>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 152465

[youenn fablet]

Created attachment 268952 [details]
Rebasing

[youenn fablet]

(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.

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[youenn fablet]

Created attachment 268957 [details]
Rebasing missing test expectations

[WebKit Commit Bot]

Comment on attachment 268957 [details]
Rebasing missing test expectations

Clearing flags on attachment: 268957

Committed r195101: <http://trac.webkit.org/changeset/195101>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.