Commit c4a2a8cb authored by ap@apple.com's avatar ap@apple.com

http/tests/eventsource/workers/eventsource-simple.html is a flaky crash because of

eventsource-status-error-iframe-crash.html
https://bugs.webkit.org/show_bug.cgi?id=61523

Reviewed by Nate Chapin.

Source/WebCore:

The problem here was that canceling EventSource during frame removal erroneously resulted
in event dispatch, and event handler re-entered frame destruction code.

* page/EventSource.h: Renamed endRequest() to networkRequestEnded(), because this method
doesn't end request. It implements "reestablish the connection" or "fail the connection"
algotithms from the spec, depending on current state.
Removed m_failSilently, since we can make this decision with existing data, and want to
fail silently by default (e.g. when detaching a frame cancels all loads).

* page/EventSource.cpp:
(WebCore::EventSource::EventSource): Don't initialize m_failSilently.
(WebCore::EventSource::~EventSource): Assert taht we are in a correct state.
(WebCore::EventSource::connect): Ditto.
(WebCore::EventSource::networkRequestEnded): Moved errorevent dispatch elsewhere.
(WebCore::EventSource::scheduleReconnect): Error event should always be queued when
reconnecting; firing it synchronously after starting m_reconnectTimer implements that.
(WebCore::EventSource::reconnectTimerFired): Assert that state is correct (the timer is
stopped if EventSource is stopped while waiting on the timer).
(WebCore::EventSource::close): Don't set m_state before calling cancel() - it will indirectly
call didFail(), which asserts that EventSource is not stopped yet.
(WebCore::EventSource::didReceiveResponse): Explicitly dispatch an error event, since it
is no longer dispatched when canceling, and canceling is the only way to stop a ThreadableLoader.
Removed a special case for 2xx responses, since it's no longer in the spec.
(WebCore::EventSource::didReceiveData): Assert that state is correct.
(WebCore::EventSource::didFinishLoading): Don't set state to CONNECTING after parsing remaining
response bytes - that may well result in dispatching an event whose handler calls close().
(WebCore::EventSource::didFail): It's simple now - we always reconnect unless the request
got canceled.
(WebCore::EventSource::didFailRedirectCheck): Dispatch error event explicitly, as we are
not going to attempt reconnecting.

LayoutTests:

* http/tests/eventsource/eventsource-status-code-states-expected.txt:
* http/tests/eventsource/eventsource-status-code-states.html:
2xx responses are no longer different from any other failures.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@94242 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 71a1e70a
2011-08-31 Alexey Proskuryakov <ap@apple.com>
http/tests/eventsource/workers/eventsource-simple.html is a flaky crash because of
eventsource-status-error-iframe-crash.html
https://bugs.webkit.org/show_bug.cgi?id=61523
Reviewed by Nate Chapin.
* http/tests/eventsource/eventsource-status-code-states-expected.txt:
* http/tests/eventsource/eventsource-status-code-states.html:
2xx responses are no longer different from any other failures.
2011-08-31 David Levin <levin@chromium.org>
[chromium] Fix expectations due to r94213.
Test EventSource states for different status codes. Should print a series of PASS messages followed by DONE.
PASS: status code 200 resulted in states CONNECTING, OPEN, OPEN, CONNECTING, CLOSED
PASS: status code 204 resulted in states CONNECTING, CONNECTING, CLOSED
PASS: status code 205 resulted in states CONNECTING, CONNECTING, CLOSED
PASS: status code 202 resulted in states CONNECTING, CONNECTING, CLOSED
PASS: status code 204 resulted in states CONNECTING, CLOSED, CLOSED
PASS: status code 205 resulted in states CONNECTING, CLOSED, CLOSED
PASS: status code 202 resulted in states CONNECTING, CLOSED, CLOSED
PASS: status code 301 resulted in states CONNECTING, OPEN, OPEN, CONNECTING, CLOSED
PASS: status code 302 resulted in states CONNECTING, OPEN, OPEN, CONNECTING, CLOSED
PASS: status code 303 resulted in states CONNECTING, OPEN, OPEN, CONNECTING, CLOSED
......
......@@ -21,9 +21,9 @@ for (var i in stateNames)
eval("var " + stateNames[i] + " = " + i);
var tests = [{"code": 200, "expectedStates": [CONNECTING, OPEN, OPEN, CONNECTING, CLOSED]},
{"code": 204, "expectedStates": [CONNECTING,,, CONNECTING, CLOSED]},
{"code": 205, "expectedStates": [CONNECTING,,, CONNECTING, CLOSED]},
{"code": 202, "expectedStates": [CONNECTING,,, CONNECTING, CLOSED]}, // other 2xx
{"code": 204, "expectedStates": [CONNECTING,,, CLOSED, CLOSED]},
{"code": 205, "expectedStates": [CONNECTING,,, CLOSED, CLOSED]},
{"code": 202, "expectedStates": [CONNECTING,,, CLOSED, CLOSED]}, // other 2xx
{"code": 301, "expectedStates": [CONNECTING, OPEN, OPEN, CONNECTING, CLOSED]},
{"code": 302, "expectedStates": [CONNECTING, OPEN, OPEN, CONNECTING, CLOSED]},
{"code": 303, "expectedStates": [CONNECTING, OPEN, OPEN, CONNECTING, CLOSED]},
......
2011-08-31 Alexey Proskuryakov <ap@apple.com>
http/tests/eventsource/workers/eventsource-simple.html is a flaky crash because of
eventsource-status-error-iframe-crash.html
https://bugs.webkit.org/show_bug.cgi?id=61523
Reviewed by Nate Chapin.
The problem here was that canceling EventSource during frame removal erroneously resulted
in event dispatch, and event handler re-entered frame destruction code.
* page/EventSource.h: Renamed endRequest() to networkRequestEnded(), because this method
doesn't end request. It implements "reestablish the connection" or "fail the connection"
algotithms from the spec, depending on current state.
Removed m_failSilently, since we can make this decision with existing data, and want to
fail silently by default (e.g. when detaching a frame cancels all loads).
* page/EventSource.cpp:
(WebCore::EventSource::EventSource): Don't initialize m_failSilently.
(WebCore::EventSource::~EventSource): Assert taht we are in a correct state.
(WebCore::EventSource::connect): Ditto.
(WebCore::EventSource::networkRequestEnded): Moved errorevent dispatch elsewhere.
(WebCore::EventSource::scheduleReconnect): Error event should always be queued when
reconnecting; firing it synchronously after starting m_reconnectTimer implements that.
(WebCore::EventSource::reconnectTimerFired): Assert that state is correct (the timer is
stopped if EventSource is stopped while waiting on the timer).
(WebCore::EventSource::close): Don't set m_state before calling cancel() - it will indirectly
call didFail(), which asserts that EventSource is not stopped yet.
(WebCore::EventSource::didReceiveResponse): Explicitly dispatch an error event, since it
is no longer dispatched when canceling, and canceling is the only way to stop a ThreadableLoader.
Removed a special case for 2xx responses, since it's no longer in the spec.
(WebCore::EventSource::didReceiveData): Assert that state is correct.
(WebCore::EventSource::didFinishLoading): Don't set state to CONNECTING after parsing remaining
response bytes - that may well result in dispatching an event whose handler calls close().
(WebCore::EventSource::didFail): It's simple now - we always reconnect unless the request
got canceled.
(WebCore::EventSource::didFailRedirectCheck): Dispatch error event explicitly, as we are
not going to attempt reconnecting.
2011-08-31 Sheriff Bot <webkit.review.bot@gmail.com>
Unreviewed, rolling out r94116.
......@@ -65,7 +65,6 @@ inline EventSource::EventSource(const KURL& url, ScriptExecutionContext* context
, m_decoder(TextResourceDecoder::create("text/plain", "UTF-8"))
, m_reconnectTimer(this, &EventSource::reconnectTimerFired)
, m_discardTrailingNewline(false)
, m_failSilently(false)
, m_requestInFlight(false)
, m_reconnectDelay(defaultReconnectDelay)
, m_origin(context->securityOrigin()->toString())
......@@ -101,10 +100,15 @@ PassRefPtr<EventSource> EventSource::create(const String& url, ScriptExecutionCo
EventSource::~EventSource()
{
ASSERT(m_state == CLOSED);
ASSERT(!m_requestInFlight);
}
void EventSource::connect()
{
ASSERT(m_state == CONNECTING);
ASSERT(!m_requestInFlight);
ResourceRequest request(m_url);
request.setHTTPMethod("GET");
request.setHTTPHeaderField("Accept", "text/event-stream");
......@@ -124,16 +128,13 @@ void EventSource::connect()
m_requestInFlight = true;
}
void EventSource::endRequest()
void EventSource::networkRequestEnded()
{
if (!m_requestInFlight)
return;
m_requestInFlight = false;
if (!m_failSilently)
dispatchEvent(Event::create(eventNames().errorEvent, false, false));
if (m_state != CLOSED)
scheduleReconnect();
else
......@@ -144,6 +145,7 @@ void EventSource::scheduleReconnect()
{
m_state = CONNECTING;
m_reconnectTimer.startOneShot(m_reconnectDelay / 1000);
dispatchEvent(Event::create(eventNames().errorEvent, false, false));
}
void EventSource::reconnectTimerFired(Timer<EventSource>*)
......@@ -163,19 +165,21 @@ EventSource::State EventSource::readyState() const
void EventSource::close()
{
if (m_state == CLOSED)
if (m_state == CLOSED) {
ASSERT(!m_requestInFlight);
return;
}
// Stop trying to reconnect if EventSource was explicitly closed or if ActiveDOMObject::stop() was called.
if (m_reconnectTimer.isActive()) {
m_reconnectTimer.stop();
unsetPendingActivity(this);
}
m_state = CLOSED;
m_failSilently = true;
if (m_requestInFlight)
m_loader->cancel();
m_state = CLOSED;
}
ScriptExecutionContext* EventSource::scriptExecutionContext() const
......@@ -185,13 +189,15 @@ ScriptExecutionContext* EventSource::scriptExecutionContext() const
void EventSource::didReceiveResponse(unsigned long, const ResourceResponse& response)
{
ASSERT(m_state == CONNECTING);
ASSERT(m_requestInFlight);
int statusCode = response.httpStatusCode();
bool mimeTypeIsValid = response.mimeType() == "text/event-stream";
bool responseIsValid = statusCode == 200 && mimeTypeIsValid;
if (responseIsValid) {
const String& charset = response.textEncodingName();
// If we have a charset, the only allowed value is UTF-8 (case-insensitive). This should match
// the updated EventSource standard.
// If we have a charset, the only allowed value is UTF-8 (case-insensitive).
responseIsValid = charset.isEmpty() || equalIgnoringCase(charset, "UTF-8");
if (!responseIsValid) {
String message = "EventSource's response has a charset (\"";
......@@ -215,40 +221,51 @@ void EventSource::didReceiveResponse(unsigned long, const ResourceResponse& resp
m_state = OPEN;
dispatchEvent(Event::create(eventNames().openEvent, false, false));
} else {
if (statusCode <= 200 || statusCode > 299)
m_state = CLOSED;
m_loader->cancel();
dispatchEvent(Event::create(eventNames().errorEvent, false, false));
}
}
void EventSource::didReceiveData(const char* data, int length)
{
ASSERT(m_state == OPEN);
ASSERT(m_requestInFlight);
append(m_receiveBuf, m_decoder->decode(data, length));
parseEventStream();
}
void EventSource::didFinishLoading(unsigned long, double)
{
ASSERT(m_state == OPEN);
ASSERT(m_requestInFlight);
if (m_receiveBuf.size() > 0 || m_data.size() > 0) {
append(m_receiveBuf, "\n\n");
parseEventStream();
}
m_state = CONNECTING;
endRequest();
networkRequestEnded();
}
void EventSource::didFail(const ResourceError& error)
{
int canceled = error.isCancellation();
if (((m_state == CONNECTING) && !canceled) || ((m_state == OPEN) && canceled))
ASSERT(m_state != CLOSED);
ASSERT(m_requestInFlight);
if (error.isCancellation())
m_state = CLOSED;
endRequest();
networkRequestEnded();
}
void EventSource::didFailRedirectCheck()
{
m_state = CLOSED;
ASSERT(m_state == CONNECTING);
ASSERT(m_requestInFlight);
m_loader->cancel();
ASSERT(m_state == CLOSED);
dispatchEvent(Event::create(eventNames().errorEvent, false, false));
}
void EventSource::parseEventStream()
......
......@@ -97,7 +97,7 @@ namespace WebCore {
virtual void didFailRedirectCheck();
void connect();
void endRequest();
void networkRequestEnded();
void scheduleReconnect();
void reconnectTimerFired(Timer<EventSource>*);
void parseEventStream();
......@@ -112,7 +112,6 @@ namespace WebCore {
Timer<EventSource> m_reconnectTimer;
Vector<UChar> m_receiveBuf;
bool m_discardTrailingNewline;
bool m_failSilently;
bool m_requestInFlight;
String m_eventName;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment