Upgrade Wasm JS API, step 1

Upgrade Wasm JS API, step 1

Implements:
- WebAssembly object,
- WebAssembly.Module constructor,
- WebAssembly.Instance constructor,
- WebAssembly.compile async method,
- and Module and Instance instance objects.

Also, changes ErrorThrower to support capturing errors in a promise reject.

Since we cannot yet compile without fixing the Wasm memory, and cannot validate a module without compiling, the Module constructor and compile method don't do anything yet but checking that their argument is a suitable BufferSource. Instead of a compiled module, the hidden state of a Module object currently is just that buffer.

BUG=

Committed: https://crrev.com/386c747b8a90adbf2c2bf7161e7bbd144a5b797c
Cr-Commit-Position: refs/heads/master@{#37143}

[rossberg, 2016-06-20 13:45:53]

Description was changed from

==========
Upgrade Wasm JS API, step 1

BUG=
==========

to

==========
Upgrade Wasm JS API, step 1

Implements:
- WebAssembly object,
- WebAssembly.Module constructor,
- WebAssembly.Instance constructor,
- WebAssembly.compile async method,
- and Module and Instance instance objects.

Also, changes ErrorThrower to support capturing errors in a promise reject.

Since we cannot yet compile without fixing the Wasm memory, and cannot validate
a module without compiling, the Module constructor and compile method don't do
anything yet but checking that their argument is a suitable BufferSource.
Instead of a compiled module, the hidden state of a Module object currently is
just that buffer.

BUG=
==========

[rossberg, 2016-06-20 13:46:56]

rossberg@chromium.org changed reviewers:
+ bradnelson@chromium.org, titzer@chromium.org

[rossberg, 2016-06-20 13:55:45]

PTAL, and see CL description for limitations. For a while, I tried to split off
decoding from compilation so that at least that can happen in Module/compile,
but that proved to be tricky -- we'd need to ensure that all the intermediate
data structures that hang off a WasmModule struct are properly persistent (and
finalised!). I couldn't convince myself that the easy-enough hacks were really
safe, and since ultimately this separation isn't needed, I think it doesn't make
sense to go there. Once we have relocatable code, the proper solution can be
implemented easily.

[bradnelson, 2016-06-21 06:26:51]

lgtm

https://codereview.chromium.org/2084573002/diff/20001/src/wasm/wasm-js.cc
File src/wasm/wasm-js.cc (right):

https://codereview.chromium.org/2084573002/diff/20001/src/wasm/wasm-js.cc#new...
src/wasm/wasm-js.cc:188: i::Handle<i::JSArrayBuffer> memory =
i::Handle<i::JSArrayBuffer>::null();
Worth moving this to something like GetRawBufferSource?

https://codereview.chromium.org/2084573002/diff/20001/src/wasm/wasm-result.cc
File src/wasm/wasm-result.cc (right):

https://codereview.chromium.org/2084573002/diff/20001/src/wasm/wasm-result.cc...
src/wasm/wasm-result.cc:32: if (error()) return;
Any reason not to skip right away if there's a pending exception?
Though understood the destructor case is the minimum needed.

https://codereview.chromium.org/2084573002/diff/20001/src/wasm/wasm-result.h
File src/wasm/wasm-result.h (right):

https://codereview.chromium.org/2084573002/diff/20001/src/wasm/wasm-result.h#...
src/wasm/wasm-result.h:107: i::Handle<i::String> Reify() {
Where is this getting called?

https://codereview.chromium.org/2084573002/diff/20001/test/cctest/interpreter...
File test/cctest/interpreter/bytecode_expectations/CallRuntime.golden (right):

https://codereview.chromium.org/2084573002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/bytecode_expectations/CallRuntime.golden:83:
B(CallJSRuntime), U8(128), R(0), U8(2),
Why does this change?

https://codereview.chromium.org/2084573002/diff/20001/test/mjsunit/wasm/wasm-...
File test/mjsunit/wasm/wasm-module-builder.js (right):

https://codereview.chromium.org/2084573002/diff/20001/test/mjsunit/wasm/wasm-...
test/mjsunit/wasm/wasm-module-builder.js:5: function emit_u8(bytes, val) {
Indent seems off on these?

https://codereview.chromium.org/2084573002/diff/20001/test/mjsunit/wasm/wasm-...
test/mjsunit/wasm/wasm-module-builder.js:69: class WasmFunctionBuilder {
Maybe land this refactoring as a separate CL?

https://codereview.chromium.org/2084573002/diff/20001/test/mjsunit/wasm/wasm-...
test/mjsunit/wasm/wasm-module-builder.js:346: instantiate(...args) {
Well, except for this.
Or keep the old flavor as a fallback?

[bradn, 2016-06-21 06:28:32]

bradnelson@google.com changed reviewers:
+ bradnelson@google.com

[rossberg, 2016-06-21 12:23:48]

https://codereview.chromium.org/2084573002/diff/20001/src/wasm/wasm-js.cc
File src/wasm/wasm-js.cc (right):

https://codereview.chromium.org/2084573002/diff/20001/src/wasm/wasm-js.cc#new...
src/wasm/wasm-js.cc:188: i::Handle<i::JSArrayBuffer> memory =
i::Handle<i::JSArrayBuffer>::null();
On 2016/06/21 06:26:50, bradnelson wrote:
> Worth moving this to something like GetRawBufferSource?

Hm, this doesn't have anything to do with the source (and is logically part of
instantiation, not compilation).

https://codereview.chromium.org/2084573002/diff/20001/src/wasm/wasm-result.cc
File src/wasm/wasm-result.cc (right):

https://codereview.chromium.org/2084573002/diff/20001/src/wasm/wasm-result.cc...
src/wasm/wasm-result.cc:32: if (error()) return;
On 2016/06/21 06:26:50, bradnelson wrote:
> Any reason not to skip right away if there's a pending exception?
> Though understood the destructor case is the minimum needed.

The condition seemed very weird -- if we have a pending exception then we
shouldn't even get here.

https://codereview.chromium.org/2084573002/diff/20001/src/wasm/wasm-result.h
File src/wasm/wasm-result.h (right):

https://codereview.chromium.org/2084573002/diff/20001/src/wasm/wasm-result.h#...
src/wasm/wasm-result.h:107: i::Handle<i::String> Reify() {
On 2016/06/21 06:26:50, bradnelson wrote:
> Where is this getting called?

Nowhere right now, but it was used in the async `compile` method before I
removed the code trying to do decoding there. It's needed to resolve the promise
with an error. It will again be necessary once we actually implement the
compilation semantics properly.

https://codereview.chromium.org/2084573002/diff/20001/test/cctest/interpreter...
File test/cctest/interpreter/bytecode_expectations/CallRuntime.golden (right):

https://codereview.chromium.org/2084573002/diff/20001/test/cctest/interpreter...
test/cctest/interpreter/bytecode_expectations/CallRuntime.golden:83:
B(CallJSRuntime), U8(128), R(0), U8(2),
On 2016/06/21 06:26:50, bradnelson wrote:
> Why does this change?

I added 4 new entries to the native context, so this index seems to have
shifted.

https://codereview.chromium.org/2084573002/diff/20001/test/mjsunit/wasm/wasm-...
File test/mjsunit/wasm/wasm-module-builder.js (right):

https://codereview.chromium.org/2084573002/diff/20001/test/mjsunit/wasm/wasm-...
test/mjsunit/wasm/wasm-module-builder.js:69: class WasmFunctionBuilder {
On 2016/06/21 06:26:51, bradnelson wrote:
> Maybe land this refactoring as a separate CL?

Done.

[rossberg, 2016-06-21 12:23:58]

The CQ bit was checked by rossberg@chromium.org

[rossberg, 2016-06-21 12:23:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
bradnelson@chromium.org
Link to the patchset: https://codereview.chromium.org/2084573002/#ps40001
(title: "Separate test refactorings")

[commit-bot: I haz the power, 2016-06-21 12:24:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2084573002/40001

[commit-bot: I haz the power, 2016-06-21 12:53:05]

Description was changed from

==========
Upgrade Wasm JS API, step 1

Implements:
- WebAssembly object,
- WebAssembly.Module constructor,
- WebAssembly.Instance constructor,
- WebAssembly.compile async method,
- and Module and Instance instance objects.

Also, changes ErrorThrower to support capturing errors in a promise reject.

Since we cannot yet compile without fixing the Wasm memory, and cannot validate
a module without compiling, the Module constructor and compile method don't do
anything yet but checking that their argument is a suitable BufferSource.
Instead of a compiled module, the hidden state of a Module object currently is
just that buffer.

BUG=
==========

to

==========
Upgrade Wasm JS API, step 1

Implements:
- WebAssembly object,
- WebAssembly.Module constructor,
- WebAssembly.Instance constructor,
- WebAssembly.compile async method,
- and Module and Instance instance objects.

Also, changes ErrorThrower to support capturing errors in a promise reject.

Since we cannot yet compile without fixing the Wasm memory, and cannot validate
a module without compiling, the Module constructor and compile method don't do
anything yet but checking that their argument is a suitable BufferSource.
Instead of a compiled module, the hidden state of a Module object currently is
just that buffer.

BUG=
==========

[commit-bot: I haz the power, 2016-06-21 12:53:06]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-21 12:54:17]

Description was changed from

==========
Upgrade Wasm JS API, step 1

Implements:
- WebAssembly object,
- WebAssembly.Module constructor,
- WebAssembly.Instance constructor,
- WebAssembly.compile async method,
- and Module and Instance instance objects.

Also, changes ErrorThrower to support capturing errors in a promise reject.

Since we cannot yet compile without fixing the Wasm memory, and cannot validate
a module without compiling, the Module constructor and compile method don't do
anything yet but checking that their argument is a suitable BufferSource.
Instead of a compiled module, the hidden state of a Module object currently is
just that buffer.

BUG=
==========

to

==========
Upgrade Wasm JS API, step 1

Implements:
- WebAssembly object,
- WebAssembly.Module constructor,
- WebAssembly.Instance constructor,
- WebAssembly.compile async method,
- and Module and Instance instance objects.

Also, changes ErrorThrower to support capturing errors in a promise reject.

Since we cannot yet compile without fixing the Wasm memory, and cannot validate
a module without compiling, the Module constructor and compile method don't do
anything yet but checking that their argument is a suitable BufferSource.
Instead of a compiled module, the hidden state of a Module object currently is
just that buffer.

BUG=

Committed: https://crrev.com/386c747b8a90adbf2c2bf7161e7bbd144a5b797c
Cr-Commit-Position: refs/heads/master@{#37143}
==========

[commit-bot: I haz the power, 2016-06-21 12:54:18]

Patchset 3 (id:??) landed as
https://crrev.com/386c747b8a90adbf2c2bf7161e7bbd144a5b797c
Cr-Commit-Position: refs/heads/master@{#37143}

[Mircea Trofin, 2016-06-21 13:20:49]

mtrofin@chromium.org changed reviewers:
+ mtrofin@chromium.org

[Mircea Trofin, 2016-06-21 13:20:50]

https://codereview.chromium.org/2084573002/diff/40001/src/wasm/wasm-js.cc
File src/wasm/wasm-js.cc (right):

https://codereview.chromium.org/2084573002/diff/40001/src/wasm/wasm-js.cc#new...
src/wasm/wasm-js.cc:465: InstallFunc(isolate, wasm_object, "Instance",
WebAssemblyInstance);
Don't we want to name this "Instantiate", rather than "Instance"? The latter
suggests it is a property of the Module.

[rossberg, 2016-06-21 13:35:17]

https://codereview.chromium.org/2084573002/diff/40001/src/wasm/wasm-js.cc
File src/wasm/wasm-js.cc (right):

https://codereview.chromium.org/2084573002/diff/40001/src/wasm/wasm-js.cc#new...
src/wasm/wasm-js.cc:465: InstallFunc(isolate, wasm_object, "Instance",
WebAssemblyInstance);
On 2016/06/21 13:20:49, Mircea Trofin wrote:
> Don't we want to name this "Instantiate", rather than "Instance"? The latter
> suggests it is a property of the Module.

No, it's a constructor, i.e., represents a class. You have to use it like `new
WebAssembly.Instance(module)`. Per spec it should not be callable as a function,
although throwing an error is not yet implemented.

[Mircea Trofin, 2016-06-21 13:50:02]

On 2016/06/21 13:35:17, rossberg wrote:
> https://codereview.chromium.org/2084573002/diff/40001/src/wasm/wasm-js.cc
> File src/wasm/wasm-js.cc (right):
> 
>
https://codereview.chromium.org/2084573002/diff/40001/src/wasm/wasm-js.cc#new...
> src/wasm/wasm-js.cc:465: InstallFunc(isolate, wasm_object, "Instance",
> WebAssemblyInstance);
> On 2016/06/21 13:20:49, Mircea Trofin wrote:
> > Don't we want to name this "Instantiate", rather than "Instance"? The latter
> > suggests it is a property of the Module.
> 
> No, it's a constructor, i.e., represents a class. You have to use it like `new
> WebAssembly.Instance(module)`. Per spec it should not be callable as a
function,
> although throwing an error is not yet implemented.

I see, and you'd provide heap and ffi at that point, too. Thanks!

I'm curious, why wouldn't we also want instantiation to be asynchronous? At
minimum, 
we'll do some relocations there, which scale in time complexity with the size of
the 
compiled code.

[rossberg, 2016-06-21 14:07:15]

On 2016/06/21 13:50:02, Mircea Trofin wrote:
> I'm curious, why wouldn't we also want instantiation to be asynchronous? At
> minimum, 
> we'll do some relocations there, which scale in time complexity with the size
of
> the compiled code.

I don't care much, but the difference between compile and instantiate is
probably several orders of magnitude, so it seems much less useful. Lazily
compiling even a simple JS function that you're calling the first time can
easily take much longer.

I would rather see an asynchronous function that compiles _and_ instantiates
(like we had before). It seems silly to always have to do the double constructor
dance.

[Mircea Trofin, 2016-06-21 14:15:45]

On 2016/06/21 14:07:15, rossberg wrote:
> On 2016/06/21 13:50:02, Mircea Trofin wrote:
> > I'm curious, why wouldn't we also want instantiation to be asynchronous? At
> > minimum, 
> > we'll do some relocations there, which scale in time complexity with the
size
> of
> > the compiled code.
> 
> I don't care much, but the difference between compile and instantiate is
> probably several orders of magnitude, so it seems much less useful. Lazily
> compiling even a simple JS function that you're calling the first time can
> easily take much longer.
> 
> I would rather see an asynchronous function that compiles _and_ instantiates
> (like we had before). It seems silly to always have to do the double
constructor
> dance.

Since we have the async compile API, can't one write the all-in-one helper from
there? Or are you saying that it's not an enabler you're missing, but the
usability
aspect of not having the all-in-one helper available out of the box?

[rossberg, 2016-06-21 14:23:44]

On 2016/06/21 14:15:45, Mircea Trofin wrote:
> On 2016/06/21 14:07:15, rossberg wrote:
> > I would rather see an asynchronous function that compiles _and_ instantiates
> > (like we had before). It seems silly to always have to do the double
> constructor
> > dance.
> 
> Since we have the async compile API, can't one write the all-in-one helper
from
> there? Or are you saying that it's not an enabler you're missing, but the
> usability
> aspect of not having the all-in-one helper available out of the box?

Yep.