Open WebSockets should not prevent a page from entering PageCache
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Apr 2015 04:24:24 +0000 (04:24 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Apr 2015 04:24:24 +0000 (04:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=143505
<rdar://problem/19923085>

Reviewed by Alexey Proskuryakov.

Source/WebCore:

Open WebSockets should not prevent a page from entering PageCache. This
is currently causing mobile.nytimes.com to not be page-cacheable.

In this patch, We close open WebSockets when entering the page cache
and fire the "close" events after resuming, similarly to what we did
for XMLHttpRequest in r181480. This gives a chance for the content to
handle the 'close' event (with wasClean being false and code being
1006) in order to reopen the connection if necessary.

Test: http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html

* Modules/websockets/WebSocket.cpp:
(WebCore::WebSocket::WebSocket):
(WebCore::WebSocket::canSuspend):
(WebCore::WebSocket::suspend):
(WebCore::WebSocket::resume):
(WebCore::WebSocket::resumeTimerFired):
(WebCore::WebSocket::didClose):
* Modules/websockets/WebSocket.h:

LayoutTests:

Add a layout test to check that an open WebSocket does not prevent a
page from entering page cache and that a 'close' event is fired after
resuming (restoring from the page cache).

* http/tests/websocket/tests/hybi/closed-when-entering-page-cache-expected.txt: Added.
* http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@182517 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/websockets/WebSocket.cpp
Source/WebCore/Modules/websockets/WebSocket.h

index 264e6dcdc420897de72e11cb58eb00d1c9237f22..bc32a8dc188764b0925774f9d81910f85cf08bbd 100644 (file)
@@ -1,3 +1,18 @@
+2015-04-07  Chris Dumez  <cdumez@apple.com>
+
+        Open WebSockets should not prevent a page from entering PageCache
+        https://bugs.webkit.org/show_bug.cgi?id=143505
+        <rdar://problem/19923085>
+
+        Reviewed by Alexey Proskuryakov.
+
+        Add a layout test to check that an open WebSocket does not prevent a
+        page from entering page cache and that a 'close' event is fired after
+        resuming (restoring from the page cache).
+
+        * http/tests/websocket/tests/hybi/closed-when-entering-page-cache-expected.txt: Added.
+        * http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html: Added.
+
 2015-04-07  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r182511.
diff --git a/LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache-expected.txt b/LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache-expected.txt
new file mode 100644 (file)
index 0000000..16e2e14
--- /dev/null
@@ -0,0 +1,18 @@
+CONSOLE MESSAGE: WebSocket connection to 'ws://127.0.0.1:8880/websocket/tests/hybi/echo' failed: WebSocket is closed due to suspension.
+Test that an open WebSocket does not prevent a page from entering page cache.
+
+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
+PASS WebSocket was closed
+PASS wasRestoredFromPageCache is true
+PASS closeEvent.wasClean is false
+PASS closeEvent.code is 1006
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html b/LayoutTests/http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html
new file mode 100644 (file)
index 0000000..0429d12
--- /dev/null
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="/js-test-resources/js-test-pre.js"></script>
+<script>
+description("Test that an open WebSocket does not prevent a page from entering page cache.");
+
+window.jsTestIsAsync = true;
+if (window.testRunner)
+    testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+
+var wasRestoredFromPageCache = false;
+
+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);
+
+window.addEventListener('load', function() {
+    ws = new WebSocket("ws://127.0.0.1:8880/websocket/tests/hybi/echo");
+    ws.onclose = function(ev) {
+        testPassed("WebSocket was closed");
+        shouldBeTrue("wasRestoredFromPageCache");
+        closeEvent = ev;
+        shouldBeFalse("closeEvent.wasClean");
+        shouldBe("closeEvent.code", "1006");
+        finishJSTest();
+    };
+
+    // This needs to happen in a setTimeout because a navigation inside the onload handler would
+    // not create a history entry.
+    setTimeout(function() {
+      // Force a back navigation back to this page.
+      window.location.href = "/navigation/resources/page-cache-helper.html";
+    }, 0);
+}, false);
+
+</script>
+<script src="/js-test-resources/js-test-post.js"></script>
+</body>
+</html>
index e4a7d39dc186e26681a91e5688445263a4ae8d53..5a0a31755f498b066b7a2fde4f4a925a1f4fa19a 100644 (file)
@@ -1,3 +1,31 @@
+2015-04-07  Chris Dumez  <cdumez@apple.com>
+
+        Open WebSockets should not prevent a page from entering PageCache
+        https://bugs.webkit.org/show_bug.cgi?id=143505
+        <rdar://problem/19923085>
+
+        Reviewed by Alexey Proskuryakov.
+
+        Open WebSockets should not prevent a page from entering PageCache. This
+        is currently causing mobile.nytimes.com to not be page-cacheable.
+
+        In this patch, We close open WebSockets when entering the page cache
+        and fire the "close" events after resuming, similarly to what we did
+        for XMLHttpRequest in r181480. This gives a chance for the content to
+        handle the 'close' event (with wasClean being false and code being
+        1006) in order to reopen the connection if necessary.
+
+        Test: http/tests/websocket/tests/hybi/closed-when-entering-page-cache.html
+
+        * Modules/websockets/WebSocket.cpp:
+        (WebCore::WebSocket::WebSocket):
+        (WebCore::WebSocket::canSuspend):
+        (WebCore::WebSocket::suspend):
+        (WebCore::WebSocket::resume):
+        (WebCore::WebSocket::resumeTimerFired):
+        (WebCore::WebSocket::didClose):
+        * Modules/websockets/WebSocket.h:
+
 2015-04-07  Simon Fraser  <simon.fraser@apple.com>
 
         Add experimental code to use custom font dilation when rendering into non-opaque contexts
index 57878e762aa8fa851670c53e1e96f187053d3481..263961acf6f7deef8325211a215c971014c9b13e 100644 (file)
@@ -146,6 +146,7 @@ WebSocket::WebSocket(ScriptExecutionContext& context)
     , m_binaryType(BinaryTypeBlob)
     , m_subprotocol("")
     , m_extensions("")
+    , m_resumeTimer(*this, &WebSocket::resumeTimerFired)
 {
 }
 
@@ -462,19 +463,39 @@ void WebSocket::contextDestroyed()
 
 bool WebSocket::canSuspend() const
 {
-    return !m_channel;
+    return true;
 }
 
-void WebSocket::suspend(ReasonForSuspension)
+void WebSocket::suspend(ReasonForSuspension reason)
 {
-    if (m_channel)
-        m_channel->suspend();
+    if (m_resumeTimer.isActive())
+        m_resumeTimer.stop();
+
+    if (m_channel) {
+        if (reason == ActiveDOMObject::DocumentWillBecomeInactive) {
+            // 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
+            m_channel->suspend();
+    }
 }
 
 void WebSocket::resume()
 {
     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().
+        m_resumeTimer.startOneShot(0);
+    }
+}
+
+void WebSocket::resumeTimerFired()
+{
+    ASSERT(m_pendingCloseEvent);
+    dispatchEvent(WTF::move(m_pendingCloseEvent));
 }
 
 void WebSocket::stop()
@@ -565,11 +586,17 @@ void WebSocket::didClose(unsigned long unhandledBufferedAmount, ClosingHandshake
     m_state = CLOSED;
     m_bufferedAmount = unhandledBufferedAmount;
     ASSERT(scriptExecutionContext());
+
     RefPtr<CloseEvent> event = CloseEvent::create(wasClean, code, reason);
-    dispatchEvent(event);
+    if (m_shouldDelayCloseEvent) {
+        m_pendingCloseEvent = WTF::move(event);
+        m_shouldDelayCloseEvent = false;
+    } else
+        dispatchEvent(event);
+
     if (m_channel) {
         m_channel->disconnect();
-        m_channel = 0;
+        m_channel = nullptr;
     }
     if (hasPendingActivity())
         ActiveDOMObject::unsetPendingActivity(this);
index d3c3baa5d5add70c03246cd6a15cd816e569bcb7..9a8dcf929ef1337c23dd42ed5a4e97b3691a354e 100644 (file)
@@ -46,6 +46,7 @@
 namespace WebCore {
 
 class Blob;
+class CloseEvent;
 class ThreadableWebSocketChannel;
 
 class WebSocket final : public RefCounted<WebSocket>, public EventTargetWithInlineData, public ActiveDOMObject, public WebSocketChannelClient {
@@ -108,6 +109,8 @@ public:
 private:
     explicit WebSocket(ScriptExecutionContext&);
 
+    void resumeTimerFired();
+
     // ActiveDOMObject API.
     void contextDestroyed() override;
     bool canSuspend() const override;
@@ -135,6 +138,10 @@ private:
     BinaryType m_binaryType;
     String m_subprotocol;
     String m_extensions;
+
+    Timer m_resumeTimer;
+    bool m_shouldDelayCloseEvent { false };
+    RefPtr<CloseEvent> m_pendingCloseEvent;
 };
 
 } // namespace WebCore