[Webkit-unassigned] [Bug 87336] [WebSocket] WebSocket object should fire error event when WebSocket server can't be connected.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=87336





--- Comment #3 from Yuta Kitamura <yutak at chromium.org>  2012-05-24 04:02:23 PST ---
(From update of attachment 143762)
View in context: https://bugs.webkit.org/attachment.cgi?id=143762&action=review

I recommend adding another test that calls ws.close() in onclose handler to make sure the browser won't crash in these scenarios.

> Source/WebCore/ChangeLog:17
> +        From http://tools.ietf.org/html/rfc6455#section-4.1
> +        If the connection could not be opened, either because a direct
> +        connection failed or because any proxy used returned an error,
> +        then the client MUST _Fail the WebSocket Connection_ and abort
> +        the connection attempt.
> +
> +        From http://dev.w3.org/html5/websockets/#feedback-from-the-protocol
> +        If the user agent was required to fail the websocket connection 
> +        or the WebSocket connection is closed with prejudice, fire a 
> +        simple event named error at the WebSocket object.

I'm not quite sure if these spec requirements have anything to do with the change itself. Could you add more explanations in your words?

> Source/WebCore/Modules/websockets/WebSocketChannel.cpp:232
> +            RefPtr<WebSocketChannel> protect(this); // The client can close the channel, potentially removing the last reference.

This use of "protect" is wrong, because the channel may get deleted in the end of the block and the code after the block may access to "this" which is already deleted.

If the end of a block containing a protector is reached, the function must immediately exit.

> Source/WebCore/Modules/websockets/WebSocketChannel.cpp:308
> +                RefPtr<WebSocketChannel> protect(this); // The client can close the channel, potentially removing the last reference.

This protector is not necessary because the channel is guaranteed to exist until at least the end of this function (see deref() in the end of this function).

The deref() in the end of this function is paired with ref() in connect(), and it is guaranteed that the channel is alive between these two.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.