Bug 173105 - WebAssembly: add support for stream APIs
Summary: WebAssembly: add support for stream APIs
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: GSkachkov
URL:
Keywords:
Depends on: 183443 183444 184886 184888 181600 183442
Blocks: 161709
  Show dependency treegraph
 
Reported: 2017-06-08 09:57 PDT by JF Bastien
Modified: 2019-05-10 14:58 PDT (History)
23 users (show)

See Also:


Attachments
Patch (23.54 KB, patch)
2018-01-03 12:00 PST, GSkachkov
no flags Details | Formatted Diff | Diff
WASM and CSP test cases. (13.97 KB, application/zip)
2019-05-10 14:58 PDT, Rob
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description JF Bastien 2017-06-08 09:57:55 PDT
Post-MVP APIs were added: https://github.com/WebAssembly/design/blob/master/Web.md#additional-web-embedding-api

WebAssembly.compileStreaming
WebAssembly.instantiateStreaming
Comment 1 JF Bastien 2017-06-08 09:58:43 PDT
I think this is fairly low priority. When we do it we should make sure we actually stream-compile, not just download all and then start compilation.
Comment 2 JF Bastien 2017-06-29 10:58:47 PDT
When implementing this, don't forget to test CSP. I added a FIXME in 173892.

This will be trickier than current wasm CSP because the streaming APIs refer to URLs, not just array buffers.

Also consider what we do for SRI.
Comment 3 GSkachkov 2017-12-17 01:00:34 PST
Can I take this task? I will have free time so possible I'll manage implement this.
Comment 4 JF Bastien 2017-12-17 08:05:24 PST
Sure.
Comment 5 GSkachkov 2018-01-03 12:00:28 PST
Created attachment 330407 [details]
Patch

WiP. Link WebCore and JSC, and handle response from fetch. Next steps: Implement streaming parsing, run parsing in separate thread, CSP&SPI
Comment 6 JF Bastien 2018-02-20 16:10:38 PST
Comment on attachment 330407 [details]
Patch

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

A few comments to start.

> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:114
> +void WebAssemblyPrototype::webAssemblyModuleValidateAsync(ExecState* exec, Vector<uint8_t>&& source, JSPromiseDeferred* promise)

This and the internal one can just be file-level static functions.

> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:120
> +    RETURN_IF_EXCEPTION(throwScope, void());

I don't think this makes sense here?

> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:156
> +    RETURN_IF_EXCEPTION(throwScope, JSValue::encode(jsUndefined()));

Ditto.

> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:170
> +            promise->reject(exec, createTypeError(exec, ASCIILiteral("CompileStreaming is not supported in jsc, only in browser environment.")));

I might be wrong, but I think compileStreaming can take a blob URL? In that case it probably should work in JSC. What do other streaming things do in JSC?

> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:291
> +        } else {

Add a FIXME here: if there's an importObject and it contains a Memory, then we can compile the module with the right memory type (fast or not) by looking at the memory's type.

> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:296
> +                compileAndInstantiate(vm, exec, promise, firstArgument, importObject);

The above 4 lines seem wrong per spec: https://www.w3.org/TR/2018/WD-wasm-web-api-1-20180215/#streaming-modules

> Source/WebCore/bindings/js/JSDOMWindowBase.cpp:391
> +        promise->reject(exec, createTypeError(exec, ASCIILiteral("first argument must be an Response or Promise for Response #5")));

This doesn't seem like the right error message for an OOM condition.

> Source/WebCore/bindings/js/JSDOMWindowBase.cpp:402
> +    // TODO: Implement CSP and SPI check before start consume content from Response

I think you mean SRI.
Comment 7 GSkachkov 2018-03-08 03:20:40 PST
Thanks for review!!!
During the implementation this feature, I see that is more complicated that I expected, so I split it into 3 tasks:
1) Just adding javascript API, but without streaming compilation: 
   * WebAssembly.compileStreaming
   * WebAssembly.instantiateStreaming
2) Streaming compilation, compile and verify as soon as wasm chunk is delivered over network
3) Implement allow to compile each function in separate thread.

So I hope it allow to decrease size and complexity of the patches
Comment 25 Rob 2019-05-09 16:52:58 PDT
It seems that development here has stalled. We would love to see this added, but the purpose of this comment is to verify that the stream APIs will properly support origins and SRI hashes in CSPs. The comments here seem to indicate that that is a priority, but it's not super clear.

Here is a proposal that discusses the appropriate behavior of these functions: https://github.com/WebAssembly/content-security-policy/blob/master/proposals/CSP.md#proposed-origin-bound-permission

Ideally SRI hashes would be supported by the other WebAssembly APIs as well, but getting the stream APIs right from the beginning would be a huge win.
Comment 26 Rob 2019-05-10 14:58:14 PDT
Created attachment 369602 [details]
WASM and CSP test cases.

Here is a set of tests and a summary of major browser behavior.