869882 - [eventsource] Check HTTP status:204,205,210,299 readyState returns CONNECTING but not CLOSED and throws unreached assertion

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[tinazhao]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.64 Safari/537.31

Steps to reproduce:

ServerSentEvent latest Editor's Draft 23 April 2013 (http://dev.w3.org/html5/eventsource/) test
- Test wiki: http://www.w3.org/wiki/Webapps/Interop/ServerSentEvents
- Test suite: https://github.com/w3c/web-platform-tests/tree/master/eventsource
- Test case: https://github.com/w3c/web-platform-tests/blob/master/eventsource/request-status-error.htm


Actual results:

In SSE spec:
-Any other HTTP response code not listed here, and any network error that prevents the HTTP connection from being established in the first place (e.g. DNS errors), must cause the user agent to fail the connection.
-When a user agent is to fail the connection, the user agent must queue a task to set the readyState attribute to CLOSED and fire a simple event named error at the EventSource object. Once the user agent has failed the connection, it does not attempt to reconnect!

When check HTTP status: 204, 205, readyState got CONNECTING; 
and check HTTP status: 210, 299, it goes to onmessage() throws assertion:assert_unreached: Reached unreachable code.


Expected results:

When check HTTP status: 204, 205, readyState should return CLOSED;
and check HTTP status: 210, 299, it should go to onerror() and the readyState return CLOSED.

Comment 1

[u279076]

I assume this applies to Nightly (24.0a1) as well? Can someone please confirm and move this to the appropriate bugzilla component?

Comment 2

[Wellington Fernando de Macedo]

Attachment: patch

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 757148 [details] [diff] [review]
patch

So we don't end up in this code when non-2xx is handled? Looks like so.
http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/HttpBaseChannel.cpp#1151

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

I guess I should push the patch first to try

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

https://tbpl.mozilla.org/?tree=Try&rev=d590c29d9186

Comment 6

[Wellington Fernando de Macedo]

> I guess I should push the patch first to try
Yes, I think it would be a good idea. Thank you Olli.

tinazhao, finishing this bug all Firefox EventSource related bugs will be finished. Please, update (or ask the responsible to update) the wiki and the test cases according the conclusions of the bugs you have opened.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 757148 [details] [diff] [review]
patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/7d29016d1c53

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7d29016d1c53

Comment 9

[tinazhao]

(In reply to Wellington Fernando de Macedo from comment #6)
> > I guess I should push the patch first to try
> Yes, I think it would be a good idea. Thank you Olli.
> 
> tinazhao, finishing this bug all Firefox EventSource related bugs will be
> finished. Please, update (or ask the responsible to update) the wiki and the
> test cases according the conclusions of the bugs you have opened.

Verified. The test case get PASS on Firefox: nightly 26.0a1 (2013-08-13).