[webkit-reviews] review denied: [Bug 66925] [WebSocket] WebSocket::close didn't handle code and reason arguments : [Attachment 105340] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Takashi Toyoshima
<toyoshim at chromium.org>'s request for review:
Bug 66925: [WebSocket] WebSocket::close didn't handle code and reason arguments
https://bugs.webkit.org/show_bug.cgi?id=66925

Attachment 105340: Patch
https://bugs.webkit.org/attachment.cgi?id=105340&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=105340&action=review


> LayoutTests/http/tests/websocket/tests/hybi/close.html:108
> +    // TODO(toyoshim): unpaired surrogates throw SYNTAX_ERR

TODO(name) is not a WebKit style.

> LayoutTests/http/tests/websocket/tests/hybi/close_wsh.py:56
> +
> +
> +# vi:sts=4 sw=4 et

Should remove such editor-specific annotation.

> LayoutTests/http/tests/websocket/tests/hybi/workers/resources/close.js:120
> +	       //shouldBeTrue("exceptionProto === DOMException.prototype");

should remove unused code.

> LayoutTests/http/tests/websocket/tests/hybi/workers/resources/close.js:160
> +	       //shouldBeTrue("exceptionProto === DOMException.prototype");

ditto.

> Source/WebCore/bindings/js/JSWebSocketCustom.cpp:93
> +    int argumentCount = static_cast<int>(exec->argumentCount());

Why don't you use unsigned or size_t for argumentCount?

> Source/WebCore/websockets/WebSocketChannel.cpp:737
> +		   startClosingHandshake(0, "");

What does "0" mean?