Make service workers behave correctly with regards to Page Cache
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Jan 2018 04:08:33 +0000 (04:08 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Jan 2018 04:08:33 +0000 (04:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181446
<rdar://problem/36164291>

Reviewed by Youenn Fablet.

Source/WebCore:

Make service workers behave correctly with regards to Page Cache:
1. If a document has an active service worker, do not let it go into PageCache
2. When a document goes into page cache, unregister it from the list of service worker clients
3. When a document is restored from page cache, add it nack to the list of service worker clients

Tests: http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html
       http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html
       http/tests/workers/service/no-page-cache-when-controlled.html
       http/tests/workers/service/other_resources/test.html

* dom/Document.cpp:
(WebCore::Document::suspend):
(WebCore::Document::resume):
* history/PageCache.cpp:
(WebCore::canCacheFrame):
* page/DiagnosticLoggingKeys.cpp:
(WebCore::DiagnosticLoggingKeys::serviceWorkerKey):
* page/DiagnosticLoggingKeys.h:

LayoutTests:

Add layout test coverage.

* http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache-expected.txt: Added.
* http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html: Added.
* http/tests/workers/service/client-removed-from-clients-while-in-page-cache-expected.txt: Added.
* http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html: Added.
* http/tests/workers/service/no-page-cache-when-controlled-expected.txt: Added.
* http/tests/workers/service/no-page-cache-when-controlled.html: Added.
* http/tests/workers/service/other_resources/test.html: Added.
* http/tests/workers/service/resources/getClientCount-worker.js: Added.
(event.then):

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html [new file with mode: 0644]
LayoutTests/http/tests/workers/service/client-removed-from-clients-while-in-page-cache-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html [new file with mode: 0644]
LayoutTests/http/tests/workers/service/no-page-cache-when-controlled-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/workers/service/no-page-cache-when-controlled.html [new file with mode: 0644]
LayoutTests/http/tests/workers/service/other_resources/test.html [new file with mode: 0644]
LayoutTests/http/tests/workers/service/resources/getClientCount-worker.js [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/history/PageCache.cpp
Source/WebCore/page/DiagnosticLoggingKeys.cpp
Source/WebCore/page/DiagnosticLoggingKeys.h

index 091e3f9..badf765 100644 (file)
@@ -1,5 +1,25 @@
 2018-01-09  Chris Dumez  <cdumez@apple.com>
 
+        Make service workers behave correctly with regards to Page Cache
+        https://bugs.webkit.org/show_bug.cgi?id=181446
+        <rdar://problem/36164291>
+
+        Reviewed by Youenn Fablet.
+
+        Add layout test coverage.
+
+        * http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache-expected.txt: Added.
+        * http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html: Added.
+        * http/tests/workers/service/client-removed-from-clients-while-in-page-cache-expected.txt: Added.
+        * http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html: Added.
+        * http/tests/workers/service/no-page-cache-when-controlled-expected.txt: Added.
+        * http/tests/workers/service/no-page-cache-when-controlled.html: Added.
+        * http/tests/workers/service/other_resources/test.html: Added.
+        * http/tests/workers/service/resources/getClientCount-worker.js: Added.
+        (event.then):
+
+2018-01-09  Chris Dumez  <cdumez@apple.com>
+
         We should not return undefined for most properties of a detached Window
         https://bugs.webkit.org/show_bug.cgi?id=181416
         <rdar://problem/36162489>
diff --git a/LayoutTests/http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache-expected.txt b/LayoutTests/http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache-expected.txt
new file mode 100644 (file)
index 0000000..c11925d
--- /dev/null
@@ -0,0 +1,6 @@
+* Tests that a client is re-added to the list of service worker clients when it is restored from the page cache
+
+PASS: service worker has initially 2 clients
+PASS: page is about to enter page cache
+PASS: service worker now has 2 clients again after restoring the second one from page cache
+
diff --git a/LayoutTests/http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html b/LayoutTests/http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html
new file mode 100644 (file)
index 0000000..9c17567
--- /dev/null
@@ -0,0 +1,69 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="resources/sw-test-pre.js"></script>
+<script>
+log("* Tests that a client is re-added to the list of service worker clients when it is restored from the page cache");
+log("");
+
+if (window.testRunner) {
+  testRunner.setCanOpenWindows();
+  testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+}
+
+navigator.serviceWorker.addEventListener("message", function(event) {
+  if (step == "BothClientsInitiallyActive") {
+    if (event.data != 2) {
+      log("FAIL: Wrong initial number of clients: " + event.data);
+      finishSWTest();
+      return;
+    }
+    log("PASS: service worker has initially 2 clients");
+
+    otherWindow.addEventListener("pagehide", function(event) {
+       if (!event.persisted) {
+         log("FAIL: page failed to enter page cache");
+         finishSWTest();
+         return;
+       }
+       log("PASS: page is about to enter page cache");
+    });
+
+    otherWindow.addEventListener("pageshow", function(event) {
+      if (!event.persisted) {
+         log("FAIL: page was not restored from page cache");
+         finishSWTest();
+         return;
+       }
+       setTimeout(function() {
+         step = "SecondClientRestoredFromPageCache";
+         worker.postMessage("getClientCount");
+       }, 0);
+    });
+
+    otherWindow.location.href = "/navigation/resources/page-cache-helper.html";
+    return;
+  }
+
+  if (step == "SecondClientRestoredFromPageCache") {
+    if (event.data != 2) {
+      log("FAIL: Wrong number of clients after one client was restored from page cache: " + event.data);
+      finishSWTest();
+    }
+
+    log("PASS: service worker now has 2 clients again after restoring the second one from page cache");
+    finishSWTest();
+  }
+});
+
+navigator.serviceWorker.register("resources/getClientCount-worker.js", { }).then(function(registration) {
+    worker = registration.installing;
+    otherWindow = open("other_resources/test.html");
+    otherWindow.onload = function() {
+      step = "BothClientsInitiallyActive"
+      worker.postMessage("getClientCount");
+    };
+});
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/workers/service/client-removed-from-clients-while-in-page-cache-expected.txt b/LayoutTests/http/tests/workers/service/client-removed-from-clients-while-in-page-cache-expected.txt
new file mode 100644 (file)
index 0000000..2b70514
--- /dev/null
@@ -0,0 +1,6 @@
+* Tests that a client is removed from the list of service worker clients while it is in the page cache
+
+PASS: service worker has initially 2 clients
+PASS: page is about to enter page cache
+PASS: service worker has only 1 client after 1 entered page cache
+
diff --git a/LayoutTests/http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html b/LayoutTests/http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html
new file mode 100644 (file)
index 0000000..4c2102d
--- /dev/null
@@ -0,0 +1,62 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="resources/sw-test-pre.js"></script>
+<script>
+log("* Tests that a client is removed from the list of service worker clients while it is in the page cache");
+log("");
+
+if (window.testRunner) {
+  testRunner.setCanOpenWindows();
+  testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+}
+
+navigator.serviceWorker.addEventListener("message", function(event) {
+  if (step == "BothClientsInitiallyActive") {
+    if (event.data != 2) {
+      log("FAIL: Wrong initial number of clients: " + event.data);
+      finishSWTest();
+      return;
+    }
+    log("PASS: service worker has initially 2 clients");
+
+    otherWindow.addEventListener("pagehide", function(event) {
+       if (!event.persisted) {
+         log("FAIL: page failed to enter page cache");
+         finishSWTest();
+         return;
+       }
+       log("PASS: page is about to enter page cache");
+
+       setTimeout(function() {
+         step = "OnlyOneClientRemainsActive"
+         worker.postMessage("getClientCount");
+       }, 0);
+    });
+
+    otherWindow.location.href = "about:blank";
+    return;
+  }
+
+  if (step == "OnlyOneClientRemainsActive") {
+    if (event.data != 1) {
+      log("FAIL: Wrong number of clients after one client entered page cache: " + event.data);
+      finishSWTest();
+    }
+
+    log("PASS: service worker has only 1 client after 1 entered page cache");
+    finishSWTest();
+  }
+});
+
+navigator.serviceWorker.register("resources/getClientCount-worker.js", { }).then(function(registration) {
+    worker = registration.installing;
+    otherWindow = open("other_resources/test.html");
+    otherWindow.onload = function() {
+      step = "BothClientsInitiallyActive"
+      worker.postMessage("getClientCount");
+    };
+});
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/workers/service/no-page-cache-when-controlled-expected.txt b/LayoutTests/http/tests/workers/service/no-page-cache-when-controlled-expected.txt
new file mode 100644 (file)
index 0000000..0fa7dd4
--- /dev/null
@@ -0,0 +1,3 @@
+pageshow - not from page cache
+PASS: page did not enter page cache
+
diff --git a/LayoutTests/http/tests/workers/service/no-page-cache-when-controlled.html b/LayoutTests/http/tests/workers/service/no-page-cache-when-controlled.html
new file mode 100644 (file)
index 0000000..1fa8b4d
--- /dev/null
@@ -0,0 +1,57 @@
+<html>
+<head>
+<script src="resources/sw-test-pre.js"></script>
+</head>
+<body>
+<script>
+let initialController = null;
+
+if (window.testRunner) {
+  testRunner.setCanOpenWindows();
+  testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+}
+
+window.addEventListener("pageshow", function(event) {
+  log("pageshow - " + (event.persisted ? "" : "not ") + "from page cache");
+  if (!window.sessionStorage.sw_page_cache_with_controller_test_started)
+    return;
+
+  if (event.persisted)
+    log("FAIL: page entered page cache even though its iframe has a controller");
+  else
+    log("PASS: page did not enter page cache");
+
+  finishSWTest();
+});
+
+window.addEventListener("pagehide", function(event) {
+    log("pagehide - " + (event.persisted ? "" : "not ") + "entering page cache");
+    if (event.persisted) {
+        log("FAIL: page entering page cache even though its iframe has a controller");
+        finishSWTest();
+    }
+});
+
+async function test() {
+    let scopeURL = "/workers/service/resources/";
+    await registerAndWaitForActive("resources/updating-fetch-worker.php", scopeURL);
+    let frame = await withFrame(scopeURL);
+    if (frame.contentWindow.navigator.serviceWorker.controller === null) {
+        log("FAIL: frame does not have a controller");
+        finishSWTest();
+        return;
+    }
+
+    log("PASS: frame has a controller");
+
+    gc();
+    setTimeout(function() {
+        window.sessionStorage.sw_page_cache_with_controller_test_started = true;
+        location.href = "/navigation/resources/page-cache-helper.html";
+    }, 0);
+}
+
+window.onload = test;
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/workers/service/other_resources/test.html b/LayoutTests/http/tests/workers/service/other_resources/test.html
new file mode 100644 (file)
index 0000000..2a02d41
--- /dev/null
@@ -0,0 +1 @@
+TEST
diff --git a/LayoutTests/http/tests/workers/service/resources/getClientCount-worker.js b/LayoutTests/http/tests/workers/service/resources/getClientCount-worker.js
new file mode 100644 (file)
index 0000000..8b115ac
--- /dev/null
@@ -0,0 +1,6 @@
+self.addEventListener("message", (event) => {
+    source = event.source;
+    clients.matchAll({ includeUncontrolled : true }).then(function(clientList) {
+        source.postMessage(clientList.length);
+    });
+});
index 2a525ba..b88dff1 100644 (file)
@@ -1,5 +1,32 @@
 2018-01-09  Chris Dumez  <cdumez@apple.com>
 
+        Make service workers behave correctly with regards to Page Cache
+        https://bugs.webkit.org/show_bug.cgi?id=181446
+        <rdar://problem/36164291>
+
+        Reviewed by Youenn Fablet.
+
+        Make service workers behave correctly with regards to Page Cache:
+        1. If a document has an active service worker, do not let it go into PageCache
+        2. When a document goes into page cache, unregister it from the list of service worker clients
+        3. When a document is restored from page cache, add it nack to the list of service worker clients
+
+        Tests: http/tests/workers/service/client-added-to-clients-when-restored-from-page-cache.html
+               http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html
+               http/tests/workers/service/no-page-cache-when-controlled.html
+               http/tests/workers/service/other_resources/test.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::suspend):
+        (WebCore::Document::resume):
+        * history/PageCache.cpp:
+        (WebCore::canCacheFrame):
+        * page/DiagnosticLoggingKeys.cpp:
+        (WebCore::DiagnosticLoggingKeys::serviceWorkerKey):
+        * page/DiagnosticLoggingKeys.h:
+
+2018-01-09  Chris Dumez  <cdumez@apple.com>
+
         We should not return undefined for most properties of a detached Window
         https://bugs.webkit.org/show_bug.cgi?id=181416
         <rdar://problem/36162489>
index 04400dd..c7d485b 100644 (file)
@@ -4878,6 +4878,13 @@ void Document::suspend(ActiveDOMObject::ReasonForSuspension reason)
             view->compositor().cancelCompositingLayerUpdate();
     }
 
+#if ENABLE(SERVICE_WORKER)
+    if (RuntimeEnabledFeatures::sharedFeatures().serviceWorkerEnabled() && reason == ActiveDOMObject::ReasonForSuspension::PageCache) {
+        ASSERT_WITH_MESSAGE(!activeServiceWorker(), "Documents with an active service worker should not go into PageCache in the first place");
+        setServiceWorkerConnection(nullptr);
+    }
+#endif
+
     suspendScheduledTasks(reason);
 
     ASSERT(m_frame);
@@ -4909,6 +4916,13 @@ void Document::resume(ActiveDOMObject::ReasonForSuspension reason)
 
     resumeScheduledTasks(reason);
 
+#if ENABLE(SERVICE_WORKER)
+    if (RuntimeEnabledFeatures::sharedFeatures().serviceWorkerEnabled() && reason == ActiveDOMObject::ReasonForSuspension::PageCache) {
+        ASSERT_WITH_MESSAGE(!activeServiceWorker(), "Documents with an active service worker should not go into PageCache in the first place");
+        setServiceWorkerConnection(&ServiceWorkerProvider::singleton().serviceWorkerConnectionForSession(sessionID()));
+    }
+#endif
+
     m_visualUpdatesAllowed = true;
 
     m_isSuspended = false;
index 1cf8f31..6736547 100644 (file)
@@ -156,6 +156,13 @@ static bool canCacheFrame(Frame& frame, DiagnosticLoggingClient& diagnosticLoggi
         logPageCacheFailureDiagnosticMessage(diagnosticLoggingClient, DiagnosticLoggingKeys::cannotSuspendActiveDOMObjectsKey());
         isCacheable = false;
     }
+#if ENABLE(SERVICE_WORKER)
+    if (frame.document() && frame.document()->activeServiceWorker()) {
+        PCLOG("   -The document has an active service worker");
+        logPageCacheFailureDiagnosticMessage(diagnosticLoggingClient, DiagnosticLoggingKeys::serviceWorkerKey());
+        isCacheable = false;
+    }
+#endif
     // FIXME: We should investigating caching frames that have an associated
     // application cache. <rdar://problem/5917899> tracks that work.
     if (!documentLoader->applicationCacheHost().canCacheInPageCache()) {
index 6f91d46..39a4846 100644 (file)
@@ -475,11 +475,6 @@ String DiagnosticLoggingKeys::diskCacheAfterValidationKey()
     return ASCIILiteral("diskCacheAfterValidation");
 }
 
-String DiagnosticLoggingKeys::serviceWorkerKey()
-{
-    return ASCIILiteral("serviceWorker");
-}
-
 String DiagnosticLoggingKeys::reloadKey()
 {
     return ASCIILiteral("reload");
@@ -535,6 +530,11 @@ String DiagnosticLoggingKeys::scriptKey()
     return ASCIILiteral("script");
 }
 
+String DiagnosticLoggingKeys::serviceWorkerKey()
+{
+    return ASCIILiteral("serviceWorker");
+}
+
 String DiagnosticLoggingKeys::streamingMedia()
 {
     return ASCIILiteral("streamingMedia");
index 66719f7..41dc61f 100644 (file)
@@ -50,7 +50,6 @@ public:
     static String deviceMotionKey();
     static String deviceOrientationKey();
     static String diskCacheKey();
-    static String serviceWorkerKey();
     static String diskCacheAfterValidationKey();
     static String documentLoaderStoppingKey();
     WEBCORE_EXPORT static String domainCausingCrashKey();
@@ -140,6 +139,7 @@ public:
     WEBCORE_EXPORT static String revalidatingKey();
     static String sameLoadKey();
     static String scriptKey();
+    static String serviceWorkerKey();
     WEBCORE_EXPORT static String streamingMedia();
     static String styleSheetKey();
     WEBCORE_EXPORT static String successfulSpeculativeWarmupWithRevalidationKey();