According to W3C specification link: http://www.w3.org/TR/websockets/ , Websocket state should be set to closed in didReceiveMessage call back
Created attachment 237076 [details] Patch Set the Websocket state to closed in didReceiveMessage call back
Comment on attachment 237076 [details] Patch WebKit bug fixes require either a test case or an explanation of why they don’t have one. Please add a regression test.
Created attachment 237144 [details] Patch-Updated Updated patch with new layout test
Comment on attachment 237144 [details] Patch-Updated View in context: https://bugs.webkit.org/attachment.cgi?id=237144&action=review > Source/WebCore/Modules/websockets/WebSocket.cpp:536 > + m_state = CLOSED; Do we also need to call m_channel->disconnect()? Do we also need to call ActiveDOMObject::unsetPendingActivity?
(In reply to comment #4) > (From update of attachment 237144 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=237144&action=review > > > Source/WebCore/Modules/websockets/WebSocket.cpp:536 > > + m_state = CLOSED; > > Do we also need to call m_channel->disconnect()? Do we also need to call ActiveDOMObject::unsetPendingActivity? Dear Darin, Thanks for inputs, i rechecked again, i think we no need to close the channel object here, also ActiveDOMObject.
Comment on attachment 237144 [details] Patch-Updated View in context: https://bugs.webkit.org/attachment.cgi?id=237144&action=review >>> Source/WebCore/Modules/websockets/WebSocket.cpp:536 >>> + m_state = CLOSED; >> >> Do we also need to call m_channel->disconnect()? Do we also need to call ActiveDOMObject::unsetPendingActivity? > > Dear Darin, > > Thanks for inputs, i rechecked again, i think we no need to close the channel object here, also ActiveDOMObject. I’m glad you think so. Why do you think so?
(In reply to comment #6) > (From update of attachment 237144 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=237144&action=review > > >>> Source/WebCore/Modules/websockets/WebSocket.cpp:536 > >>> + m_state = CLOSED; > >> > >> Do we also need to call m_channel->disconnect()? Do we also need to call ActiveDOMObject::unsetPendingActivity? > > > > Dear Darin, > > > > Thanks for inputs, i rechecked again, i think we no need to close the channel object here, also ActiveDOMObject. > > I’m glad you think so. Why do you think so? Dear Darin, Here is my analysis, When i ran the new test cases error-event-ready-state.html, first we get onerror() call back, where the proper state will be set, after these onclose() will get called, so here the channel object will get closed and ActiveDOMObject as well. so we no need to release the resources in didReceiveMessageError() call back. I tried by closing channel object in didReceiveMessageError() call back. but it was failing some other tests, with error: FAIL: Timed out waiting for notifyDone to be called. pls share your inputs, if my understanding is not correct.
Alexey, you know this code better than I do. Could you review?
Comment on attachment 237144 [details] Patch-Updated The existing code is somewhat confusing, but I think that the patch a step in the right direction. The key consideration is that there is only one place where an error event is dispatched per the spec, and that's right before a close event.
Comment on attachment 237144 [details] Patch-Updated Clearing flags on attachment: 237144 Committed r173642: <http://trac.webkit.org/changeset/173642>
All reviewed patches have been landed. Closing bug.