[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 #17 from Li Yin <li.yin at intel.com>  2012-06-06 19:26:17 PST ---
(In reply to comment #16)
> (From update of attachment 145080 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=145080&action=review
> 
> I looked over this patch and left some comments.
> I should understand details, so I'll take another look tomorrow.
> 
> > Source/WebCore/Modules/websockets/WebSocketChannel.cpp:234
> > +        }
> 
> Here, you try to call didReceiveMessageError() at most once.
> But I notice that didReceiveMessageError() is invoked from many places in this source code.
> This looks another original bug on WebSocketChannel (Or, is this introduced at previous change?)

There is not a obvious language in Spec to describe whether error event should be fired more than once or not.
If WebSocket fails the connection when fire an error event, it seems that it is not necessary to fire more error events.

> 
> > Source/WebCore/Modules/websockets/WebSocketChannel.cpp:351
> > +#if PLATFORM(CHROMIUM)
> 
> Unfortunately, I have not enough understanding on other ports behaviors.
> Why do you need special handling for chromium port?

On chromium, if the network is down, both of OnError and OnClose callback are called.[net/socket_stream.cc] 
In webkit side, didFailSocketStream and didCloseSocketStream will be invoked one by one. So didFailSocketStream should not call disconnect function, because didCloseSocketsStream will do that.

But in other platform, like gtk, either didFailSocketStream or didCloseSocketStream is called when the network is down. So it is required to call disconnect function in didFailSocketStream. It is the difference between chromium and gtk, or other platforms.

> 
> > Source/WebKit/chromium/src/SocketStreamHandle.cpp:-156
> > -        m_socket.clear();
> 
> This part of change looks right.
> 
> > LayoutTests/ChangeLog:15
> > +
> 
> It'd be better to have worker version of this test.
> As you may already know, worker thread handling in WebSocketChannel is a little complicated.
Okay, I will try to add tests for worker.

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