[webkit-reviews] review granted: [Bug 100801] Remove the V8 custom code for WebSockets constructor : [Attachment 173234] Patch (merge changes only)

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has granted Tommy Widenflycht
<tommyw at google.com>'s request for review:
Bug 100801: Remove the V8 custom code for WebSockets constructor
https://bugs.webkit.org/show_bug.cgi?id=100801

Attachment 173234: Patch (merge changes only)
https://bugs.webkit.org/attachment.cgi?id=173234&action=review

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=173234&action=review


Looks OK with some nits. Thanks for a great patch!

> Source/WebCore/Modules/websockets/WebSocket.cpp:176
> +	   ec = SYNTAX_ERR;

This change is non-trivial but looks OK for the following reasons:

(1) This changes the current behavior. The current implementation throws
SyntaxError (one of JavaScript errors) instead of SYNTAX_ERR (one of DOM
exceptions).

(2) But the spec says it should be SYNTAX_ERR.
(http://www.w3.org/TR/2009/WD-websockets-20091222/) So the new implementation
is conformed to the spec.

(3) As no developers would care about the difference, this change won't break
compatibility.

> Source/WebCore/Modules/websockets/WebSocket.idl:43
> +    ConstructorParameters=1,

Nit: Redundant ','

> Source/WebCore/bindings/scripts/IDLParser.pm:1326
> +sub copyAttributes

copyAttributes => copyExtendedAttributes ?