Bug 163699

Summary: WebAssembly API: implement exception constructors properly
Product: WebKit Reporter: JF Bastien <jfbastien>
Component: JavaScriptCoreAssignee: JF Bastien <jfbastien>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, ggaren, keith_miller, mark.lam, msaboff, sbarati, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 161709, 163768    
Attachments:
Description Flags
patch
none
patch
keith_miller: review+, keith_miller: commit-queue-
patch
commit-queue: commit-queue-
patch none

Description JF Bastien 2016-10-19 15:23:12 PDT
The current exception constructors just throw instead of doing The Right Thing.

There's a small quibble about whether just calling the constructor as a function should be a TypeError: https://github.com/WebAssembly/design/issues/825
Comment 1 JF Bastien 2016-10-20 16:55:09 PDT
Created attachment 292286 [details]
patch
Comment 2 JF Bastien 2016-10-20 16:57:10 PDT
This was trickier than I thought it was! It's almost there:

I don't think I'm setting the .name property correctly though: if you do String(new WebAssembly.CompileError()) then you get "Error", not "CompileError".

Any idea what I've missed?
Comment 3 JF Bastien 2016-10-20 17:25:28 PDT
Created attachment 292291 [details]
patch

The previous patch was missing a bunch of `#if ENABLE(WEBASSEMBLY)` which made the 32-bit builds *very* sad. Don't be a jerk: make it happy.
Comment 4 Keith Miller 2016-10-20 17:39:28 PDT
Comment on attachment 292291 [details]
patch

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

r=me with typo fix.

> JSTests/ChangeLog:9
> +         - The error constructors used to thow (e.g. `new WebAssembly.CompileError()`).

thow => throw

> Source/JavaScriptCore/ChangeLog:9
> +         - The error constructors used to thow (e.g. `new WebAssembly.CompileError()`).

Ditto.
Comment 5 JF Bastien 2016-10-20 17:42:40 PDT
Created attachment 292293 [details]
patch

Fix typo.
Comment 6 WebKit Commit Bot 2016-10-20 17:43:42 PDT
Comment on attachment 292293 [details]
patch

Rejecting attachment 292293 [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', 'validate-changelog', '--check-oops', '--non-interactive', 292293, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in JSTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/2334501
Comment 7 JF Bastien 2016-10-20 17:45:55 PDT
I filed the following to fix .name separately: https://bugs.webkit.org/show_bug.cgi?id=163768
Comment 8 JF Bastien 2016-10-20 17:47:30 PDT
Created attachment 292295 [details]
patch

Fix reviewer. I'll get used to this workflow some day...
Comment 9 WebKit Commit Bot 2016-10-20 18:22:09 PDT
Comment on attachment 292295 [details]
patch

Clearing flags on attachment: 292295

Committed r207650: <http://trac.webkit.org/changeset/207650>
Comment 10 WebKit Commit Bot 2016-10-20 18:22:14 PDT
All reviewed patches have been landed.  Closing bug.