Bug 86958 – [WebSocket] WebSocket object should fire a simple event named error when it is required to fail the websocket connection.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Li Yin]

Spec: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.

[Li Yin]

Created attachment 142903 [details]
Patch

[Takashi Toyoshima]

Hi Li,
Thank you for sending a patch.
Actually, this is a bug and should be fixed.

A filed bug in chromium side issue tracker is here.
http://code.google.com/p/chromium/issues/detail?id=128057

I think original code just missed to call WebSocketChannelClient::didReceiveMessageError() in WebSocketChannel::fail().

Yuta,
I think you write this part. Could you check this?

[Yuta Kitamura]

Seems like the spec changed at http://html5.org/tools/web-apps-tracker?from=6376&to=6377
(see also https://www.w3.org/Bugs/Public/show_bug.cgi?id=13294)

As Takashi said, I think you just need to call didReceiveMessageError() in fail(). You don't have to add a new function to WebSocketChannelClient.

You probably need to have a line like "RefPtr<WebSocketChannel> protect;" before calling didReceiveMessageError(), otherwise this function may cause the channel to get removed (because users may do "ws.close()" in onerror handler).

Also, you might need to change the definition of didReceiveMessageError() to let it fire the event when the state is not OPEN or CLOSING. This is old code based on the old API spec, and I guess this restriction is leftover from the old age.

[Yuta Kitamura]

Comment on attachment 142903 [details]
Patch

r- because adding a new callback fireErrorEvent() is clearly unacceptable. fireErrorEvent() doesn't work properly in Web Workers, as the definition for workers isn't provided.

[Li Yin]

(In reply to comment #3)
> Seems like the spec changed at http://html5.org/tools/web-apps-tracker?from=6376&to=6377
> (see also https://www.w3.org/Bugs/Public/show_bug.cgi?id=13294)
> 
> As Takashi said, I think you just need to call didReceiveMessageError() in fail(). You don't have to add a new function to WebSocketChannelClient.
> 
> You probably need to have a line like "RefPtr<WebSocketChannel> protect;" before calling didReceiveMessageError(), otherwise this function may cause the channel to get removed (because users may do "ws.close()" in onerror handler).
> 
> Also, you might need to change the definition of didReceiveMessageError() to let it fire the event when the state is not OPEN or CLOSING. This is old code based on the old API spec, and I guess this restriction is leftover from the old age.

Thanks for your review and good solution, I have updated the patch, please review it again.

[Li Yin]

Created attachment 143210 [details]
Patch

[Yuta Kitamura]

Comment on attachment 143210 [details]
Patch

Looks fine. Please wait for comments from a reviewer.

[WebKit Review Bot]

Comment on attachment 143210 [details]
Patch

Clearing flags on attachment: 143210

Committed r117944: <http://trac.webkit.org/changeset/117944>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.