Sync the WebSocket interface with the spec

Source/modules/websockets/WebSocket.idl

Index: Source/modules/websockets/WebSocket.idl
diff --git a/Source/modules/websockets/WebSocket.idl b/Source/modules/websockets/WebSocket.idl
index c2a343cd22eff344cdf80919caf67e452b138cb3..8b22abbc694eb1b627962feb2d03030391fad257 100644
--- a/Source/modules/websockets/WebSocket.idl
+++ b/Source/modules/websockets/WebSocket.idl
@@ -30,16 +30,14 @@
  */
 
 // http://dev.w3.org/html5/websockets/#websocket

[philipj_slow, 2015/02/26 03:02:53]

I'm pretty sure this copy of the spec is abandoned, see the date. You can also
tell from the indentation of the IDL block that it isn't being kept in sync with
the source document. In other words, drop this link?

[jsbell, 2015/02/26 19:56:54]

Done.

-// http://www.whatwg.org/specs/web-apps/current-work/multipage/network.html#websocket
+// https://html.spec.whatwg.org/multipage/comms.html#websocket

[philipj_slow, 2015/02/26 03:02:53]

Can you link to
https://html.spec.whatwg.org/multipage/comms.html#the-websocket-interface
instead? Not all interfaces have such a section handy, but when there is one, it
means you get to see the constructor and any enums without scrolling.

[jsbell, 2015/02/26 19:56:53]

Done.

 
 // FIXME: use BinaryType in binaryType

[jsbell, 2015/02/25 18:51:11]

See https://codereview.chromium.org/871013007/ by Paritosh Kumar for a fix for
this

 enum BinaryType { "blob", "arraybuffer" };
 
 [
     ActiveDOMObject,
-    // FIXME: should be optional union type http://crbug.com/240176
-    Constructor(DOMString url, optional DOMString protocol),
-    Constructor(DOMString url, sequence<DOMString> protocols),
+    Constructor(DOMString url, optional (DOMString or sequence<DOMString>) protocols),

[jsbell, 2015/02/25 18:51:11]

Spec has (DOMString or DOMString[]) but it's unclear why T[] would be used
instead of sequence<T>.

[jsbell, 2015/02/25 23:18:55]

I filed this against the HTML spec:

https://www.w3.org/Bugs/Public/show_bug.cgi?id=28102

[philipj_slow, 2015/02/26 03:02:53]

Can you add a FIXME here linking to that spec bug, just in case it should be
rejected? It can just be removed if Hixie does what you ask.

[jsbell, 2015/02/26 19:56:54]

Done.

     ConstructorCallWith=ExecutionContext,
     Exposed=(Window,Worker),
     RaisesException=Constructor,
@@ -58,17 +56,17 @@ enum BinaryType { "blob", "arraybuffer" };
     readonly attribute unsigned long bufferedAmount;
 
     // networking
-             attribute EventHandler onopen;
-             attribute EventHandler onerror;
-             attribute EventHandler onclose;
+    attribute EventHandler onopen;
+    attribute EventHandler onerror;
+    attribute EventHandler onclose;
     [TreatReturnedNullStringAs=Undefined] readonly attribute DOMString extensions;

[philipj_slow, 2015/02/26 03:02:53]

Add FIXMEs for these two [TreatReturnedNullStringAs=Undefined], per spec it
looks like these should always be strings. If there are cases where a null
string is returned internally, we should probably just return an empty string.

[jsbell, 2015/02/26 19:56:54]

Done.

I traced the code back through from Chromium, and they should never be null. But
left the FIXMEs in there for now.

     [TreatReturnedNullStringAs=Undefined] readonly attribute DOMString protocol;
-    [RaisesException] void close([Clamp] optional unsigned short code, optional DOMString reason);
+    [RaisesException] void close([Clamp] optional unsigned short code, optional USVString reason);
 
     // messaging
-             attribute EventHandler onmessage;
-             attribute DOMString binaryType;
-    [RaisesException] void send(DOMString data);
+    attribute EventHandler onmessage;
+    attribute DOMString binaryType;
+    [RaisesException] void send(USVString data);
     [RaisesException] void send(Blob data);

[philipj_slow, 2015/02/26 03:02:53]

Note that without [TypeChecking=Interface], some non-Blob object or null will
result in null being seen on the C++ side. That should never happen per the
spec's IDL. There's some risk to that change, but if you want to add it in a
separate (easily reverted) CL that would be great!

[philipj_slow, 2015/02/26 03:05:59]

Hmm, I'm actually not sure what happens when there are same-name methods with
argument type overloading, there's no single C++ function to call with a null
argument. Maybe I'm just wrong, but it would be worth checking if
[TypeChecking=Interface] changes the generated code.

[jsbell, 2015/02/26 05:45:18]

I'll take a look at the generated code and report back.

[jsbell, 2015/02/26 19:56:54]

The generated code does change, but there's no behavior difference.

With the overload, the type checks are done before dispatching using
V8$TYPE$::hasInstance(), falling back to the string overload. Then in the
dispatched calls, the conversion is done with data =
V8$TYPE$::toImplWithTypeCheck(). If [TypeChecking=Interface] is specified, this
is followed by `if (!data) { ... throwTypeError ... }`. But that should never be
true.

I can add in [TypeChecking=Interface] since we ideally would have that
everywhere, fix the cases where it causes problems, then make it the default. Or
leave it for now to avoid clutter - I have no strong preference.

[philipj_slow, 2015/02/27 04:23:09]

Oh, overloading also changes things a bit. [TypeChecking=Interface] on the
interface would be great, in this CL or later as you see fit.

     [RaisesException] void send(ArrayBuffer data);
     [RaisesException] void send(ArrayBufferView data);