- Revision
- 182901
- Author
- cdu...@apple.com
- Date
- 2015-04-16 12:39:50 -0700 (Thu, 16 Apr 2015)
Log Message
Regression(r182517): WebSocket::suspend() causes error event to be fired
https://bugs.webkit.org/show_bug.cgi?id=143806
<rdar://problem/20559812>
Reviewed by Alexey Proskuryakov.
Source/WebCore:
WebSocket::suspend() causes an error event to be fired after r182517.
This is not allowed as firing the event could trigger arbitrary JS
execution, which is no longer allowed at this point.
This patch delays the error event firing until after
WebSocket::resume() is called, similarly to what we already do for
the close event.
Also add assertions in WebSocket::suspend() / WebSocket::resume()
that will be hit if JS events are fired from within these functions.
The pre-existing closed-when-entering-page-cache.html test is hitting
one of these assertions without the fix above.
Tests:
- http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html
- http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler.html
* Modules/websockets/WebSocket.cpp:
(WebCore::WebSocket::suspend):
(WebCore::WebSocket::resume):
(WebCore::WebSocket::resumeTimerFired):
(WebCore::WebSocket::stop):
(WebCore::WebSocket::didReceiveMessageError):
(WebCore::WebSocket::didClose):
(WebCore::WebSocket::dispatchOrQueueEvent):
* Modules/websockets/WebSocket.h:
LayoutTests:
* http/tests/websocket/tests/hybi/closed-when-entering-page-cache-expected.txt:
* http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html:
Extend WebSocket PageCache test to make sure that the error event is
fired after restoring the page from the PageCache and before the close
Event is fired.
* http/tests/websocket/tests/hybi/resources/page-cache-websocket.html: Added.
* http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler-expected.txt: Copied from LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache-expected.txt.
* http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler.html: Copied from LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html.
Add layout test to cover the case where WebSocket::stop() is called
while firing the pending events upon restoring the page from PageCache.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (182900 => 182901)
--- trunk/LayoutTests/ChangeLog 2015-04-16 19:20:08 UTC (rev 182900)
+++ trunk/LayoutTests/ChangeLog 2015-04-16 19:39:50 UTC (rev 182901)
@@ -1,3 +1,23 @@
+2015-04-16 Chris Dumez <cdu...@apple.com>
+
+ Regression(r182517): WebSocket::suspend() causes error event to be fired
+ https://bugs.webkit.org/show_bug.cgi?id=143806
+ <rdar://problem/20559812>
+
+ Reviewed by Alexey Proskuryakov.
+
+ * http/tests/websocket/tests/hybi/closed-when-entering-page-cache-expected.txt:
+ * http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html:
+ Extend WebSocket PageCache test to make sure that the error event is
+ fired after restoring the page from the PageCache and before the close
+ Event is fired.
+
+ * http/tests/websocket/tests/hybi/resources/page-cache-websocket.html: Added.
+ * http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler-expected.txt: Copied from LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache-expected.txt.
+ * http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler.html: Copied from LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html.
+ Add layout test to cover the case where WebSocket::stop() is called
+ while firing the pending events upon restoring the page from PageCache.
+
2015-04-16 Myles C. Maxfield <mmaxfi...@apple.com>
[iOS] Delete hardcoded font fallback tables
Modified: trunk/LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache-expected.txt (182900 => 182901)
--- trunk/LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache-expected.txt 2015-04-16 19:20:08 UTC (rev 182900)
+++ trunk/LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache-expected.txt 2015-04-16 19:39:50 UTC (rev 182901)
@@ -8,8 +8,11 @@
pagehide - entering cache
pageshow - from cache
PASS Page did enter and was restored from the page cache
-PASS WebSocket was closed
+PASS error event fired
PASS wasRestoredFromPageCache is true
+PASS close event fired
+PASS wasRestoredFromPageCache is true
+PASS receivedErrorEvent is true
PASS closeEvent.wasClean is false
PASS closeEvent.code is 1006
PASS successfullyParsed is true
Modified: trunk/LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html (182900 => 182901)
--- trunk/LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html 2015-04-16 19:20:08 UTC (rev 182900)
+++ trunk/LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html 2015-04-16 19:39:50 UTC (rev 182901)
@@ -10,6 +10,7 @@
testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
var wasRestoredFromPageCache = false;
+var receivedErrorEvent = false;
window.addEventListener("pageshow", function(event) {
debug("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
@@ -30,9 +31,15 @@
window.addEventListener('load', function() {
ws = new WebSocket("ws://127.0.0.1:8880/websocket/tests/hybi/echo");
+ ws._onerror_ = function(ev) {
+ receivedErrorEvent = true;
+ testPassed("error event fired");
+ shouldBeTrue("wasRestoredFromPageCache");
+ }
ws._onclose_ = function(ev) {
- testPassed("WebSocket was closed");
+ testPassed("close event fired");
shouldBeTrue("wasRestoredFromPageCache");
+ shouldBeTrue("receivedErrorEvent");
closeEvent = ev;
shouldBeFalse("closeEvent.wasClean");
shouldBe("closeEvent.code", "1006");
Added: trunk/LayoutTests/http/tests/websocket/tests/hybi/resources/page-cache-websocket.html (0 => 182901)
--- trunk/LayoutTests/http/tests/websocket/tests/hybi/resources/page-cache-websocket.html (rev 0)
+++ trunk/LayoutTests/http/tests/websocket/tests/hybi/resources/page-cache-websocket.html 2015-04-16 19:39:50 UTC (rev 182901)
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+ws = new WebSocket("ws://127.0.0.1:8880/websocket/tests/hybi/echo");
+</script>
+</body>
+</html>
Copied: trunk/LayoutTests/http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler-expected.txt (from rev 182900, trunk/LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache-expected.txt) (0 => 182901)
--- trunk/LayoutTests/http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler-expected.txt 2015-04-16 19:39:50 UTC (rev 182901)
@@ -0,0 +1,16 @@
+CONSOLE MESSAGE: WebSocket connection to 'ws://127.0.0.1:8880/websocket/tests/hybi/echo' failed: WebSocket is closed due to suspension.
+Tests that WebSocket correctly handles being stopped in the error event handler upon restoring from PageCache.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+pageshow - not from cache
+pagehide - entering cache
+pageshow - from cache
+PASS Page did enter and was restored from the page cache
+error event fired
+PASS wasRestoredFromPageCache is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Copied: trunk/LayoutTests/http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler.html (from rev 182900, trunk/LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html) (0 => 182901)
--- trunk/LayoutTests/http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler.html (rev 0)
+++ trunk/LayoutTests/http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler.html 2015-04-16 19:39:50 UTC (rev 182901)
@@ -0,0 +1,63 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Tests that WebSocket correctly handles being stopped in the error event handler upon restoring from PageCache.");
+
+var wasRestoredFromPageCache = false;
+
+window.jsTestIsAsync = true;
+if (window.testRunner)
+ testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+
+window.addEventListener("pageshow", function(event) {
+ debug("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
+
+ if (event.persisted) {
+ testPassed("Page did enter and was restored from the page cache");
+ wasRestoredFromPageCache = true;
+ }
+}, false);
+
+window.addEventListener("pagehide", function(event) {
+ debug("pagehide - " + (event.persisted ? "" : "not ") + "entering cache");
+ if (!event.persisted) {
+ testFailed("Page did not enter the page cache.");
+ finishJSTest();
+ }
+}, false);
+
+var totalLoaded = 0;
+function loaded() {
+ totalLoaded++;
+ if (totalLoaded < 2)
+ return;
+
+ testFrame = document.getElementById("testFrame");
+ ws = testFrame.contentWindow.ws;
+ ws._onerror_ = function(ev) {
+ debug("error event fired");
+ shouldBeTrue("wasRestoredFromPageCache");
+ testFrame.remove();
+ setTimeout(function() {
+ finishJSTest();
+ }, 500);
+ }
+ ws._onclose_ = function(ev) {
+ testFailed("Close event should not have fired.");
+ }
+
+ setTimeout(function() {
+ // Force a back navigation back to this page.
+ window.location.href = ""
+ }, 0);
+}
+
+window._onload_ = loaded;
+
+</script>
+<iframe id="testFrame" src="" _onload_="loaded()"></iframe>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (182900 => 182901)
--- trunk/Source/WebCore/ChangeLog 2015-04-16 19:20:08 UTC (rev 182900)
+++ trunk/Source/WebCore/ChangeLog 2015-04-16 19:39:50 UTC (rev 182901)
@@ -1,3 +1,38 @@
+2015-04-16 Chris Dumez <cdu...@apple.com>
+
+ Regression(r182517): WebSocket::suspend() causes error event to be fired
+ https://bugs.webkit.org/show_bug.cgi?id=143806
+ <rdar://problem/20559812>
+
+ Reviewed by Alexey Proskuryakov.
+
+ WebSocket::suspend() causes an error event to be fired after r182517.
+ This is not allowed as firing the event could trigger arbitrary JS
+ execution, which is no longer allowed at this point.
+
+ This patch delays the error event firing until after
+ WebSocket::resume() is called, similarly to what we already do for
+ the close event.
+
+ Also add assertions in WebSocket::suspend() / WebSocket::resume()
+ that will be hit if JS events are fired from within these functions.
+ The pre-existing closed-when-entering-page-cache.html test is hitting
+ one of these assertions without the fix above.
+
+ Tests:
+ - http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html
+ - http/tests/websocket/tests/hybi/stop-on-resume-in-error-handler.html
+
+ * Modules/websockets/WebSocket.cpp:
+ (WebCore::WebSocket::suspend):
+ (WebCore::WebSocket::resume):
+ (WebCore::WebSocket::resumeTimerFired):
+ (WebCore::WebSocket::stop):
+ (WebCore::WebSocket::didReceiveMessageError):
+ (WebCore::WebSocket::didClose):
+ (WebCore::WebSocket::dispatchOrQueueEvent):
+ * Modules/websockets/WebSocket.h:
+
2015-04-15 Roger Fong <roger_f...@apple.com>
Adjustments to button graphics for media controls.
Modified: trunk/Source/WebCore/Modules/websockets/WebSocket.cpp (182900 => 182901)
--- trunk/Source/WebCore/Modules/websockets/WebSocket.cpp 2015-04-16 19:20:08 UTC (rev 182900)
+++ trunk/Source/WebCore/Modules/websockets/WebSocket.cpp 2015-04-16 19:39:50 UTC (rev 182901)
@@ -468,13 +468,15 @@
void WebSocket::suspend(ReasonForSuspension reason)
{
+ NoEventDispatchAssertion assertNoEventDispatch;
+
if (m_resumeTimer.isActive())
m_resumeTimer.stop();
+ m_shouldDelayEventFiring = true;
+
if (m_channel) {
if (reason == ActiveDOMObject::PageCache) {
- // The page will enter the page cache, close the channel but only fire the close event after resuming.
- m_shouldDelayCloseEvent = true;
// This will cause didClose() to be called.
m_channel->fail("WebSocket is closed due to suspension.");
} else
@@ -484,18 +486,28 @@
void WebSocket::resume()
{
+ NoEventDispatchAssertion assertNoEventDispatch;
+
if (m_channel)
m_channel->resume();
- else if (m_pendingCloseEvent && !m_resumeTimer.isActive()) {
- // Fire the close event in a timer as we are not allowed to execute arbitrary JS from resume().
+ else if (!m_pendingEvents.isEmpty() && !m_resumeTimer.isActive()) {
+ // Fire the pending events in a timer as we are not allowed to execute arbitrary JS from resume().
m_resumeTimer.startOneShot(0);
}
+
+ m_shouldDelayEventFiring = false;
}
void WebSocket::resumeTimerFired()
{
- ASSERT(m_pendingCloseEvent);
- dispatchEvent(WTF::move(m_pendingCloseEvent));
+ Ref<WebSocket> protect(*this);
+
+ ASSERT(!m_pendingEvents.isEmpty());
+
+ // Check m_shouldDelayEventFiring when iterating in case firing an event causes
+ // suspend() to be called.
+ while (!m_pendingEvents.isEmpty() && !m_shouldDelayEventFiring)
+ dispatchEvent(m_pendingEvents.takeFirst());
}
void WebSocket::stop()
@@ -505,6 +517,7 @@
m_channel->disconnect();
m_channel = 0;
m_state = CLOSED;
+ m_pendingEvents.clear();
ActiveDOMObject::stop();
if (pending)
ActiveDOMObject::unsetPendingActivity(this);
@@ -560,7 +573,7 @@
LOG(Network, "WebSocket %p didReceiveErrorMessage()", this);
m_state = CLOSED;
ASSERT(scriptExecutionContext());
- dispatchEvent(Event::create(eventNames().errorEvent, false, false));
+ dispatchOrQueueEvent(Event::create(eventNames().errorEvent, false, false));
}
void WebSocket::didUpdateBufferedAmount(unsigned long bufferedAmount)
@@ -587,12 +600,7 @@
m_bufferedAmount = unhandledBufferedAmount;
ASSERT(scriptExecutionContext());
- RefPtr<CloseEvent> event = CloseEvent::create(wasClean, code, reason);
- if (m_shouldDelayCloseEvent) {
- m_pendingCloseEvent = WTF::move(event);
- m_shouldDelayCloseEvent = false;
- } else
- dispatchEvent(event);
+ dispatchOrQueueEvent(CloseEvent::create(wasClean, code, reason));
if (m_channel) {
m_channel->disconnect();
@@ -616,6 +624,14 @@
return overhead;
}
+void WebSocket::dispatchOrQueueEvent(Ref<Event>&& event)
+{
+ if (m_shouldDelayEventFiring)
+ m_pendingEvents.append(WTF::move(event));
+ else
+ dispatchEvent(WTF::move(event));
+}
+
} // namespace WebCore
#endif
Modified: trunk/Source/WebCore/Modules/websockets/WebSocket.h (182900 => 182901)
--- trunk/Source/WebCore/Modules/websockets/WebSocket.h 2015-04-16 19:20:08 UTC (rev 182900)
+++ trunk/Source/WebCore/Modules/websockets/WebSocket.h 2015-04-16 19:39:50 UTC (rev 182901)
@@ -110,6 +110,7 @@
explicit WebSocket(ScriptExecutionContext&);
void resumeTimerFired();
+ void dispatchOrQueueEvent(Ref<Event>&&);
// ActiveDOMObject API.
void contextDestroyed() override;
@@ -140,8 +141,8 @@
String m_extensions;
Timer m_resumeTimer;
- bool m_shouldDelayCloseEvent { false };
- RefPtr<CloseEvent> m_pendingCloseEvent;
+ bool m_shouldDelayEventFiring { false };
+ Deque<Ref<Event>> m_pendingEvents;
};
} // namespace WebCore
