Add optimization when updating a SW registration results in the exact same script
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Dec 2017 16:37:11 +0000 (16:37 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Dec 2017 16:37:11 +0000 (16:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180891

Reviewed by Geoffrey Garen.

LayoutTests/imported/w3c:

Rebaseline WPT test now that all checks are passing.

* web-platform-tests/service-workers/service-worker/registration-updateviacache.https-expected.txt:

Source/WebCore:

Add optimization when updating a SW registration results in the exact same script:
- https://w3c.github.io/ServiceWorker/#update-algorithm (step 8)

No new tests, rebaselined existing test.

* workers/service/server/SWServerJobQueue.cpp:
(WebCore::SWServerJobQueue::scriptFetchFinished):

LayoutTests:

* TestExpectations:
Skip bad WPT test that is timing out for us and Firefox. I'll file an upstream PR
to fix it.

* http/tests/workers/service/controller-change.html:
* http/tests/workers/service/registration-updateViaCache-all-importScripts.html:
* http/tests/workers/service/registration-updateViaCache-all.html:
* http/tests/workers/service/registration-updateViaCache-imports-importScripts.html:
* http/tests/workers/service/registration-updateViaCache-none-importScripts.html:
* http/tests/workers/service/registration-updateViaCache-none.html:
* http/tests/workers/service/resources/self_registration_update-worker.js: Removed.
* http/tests/workers/service/resources/self_registration_update-worker.php: Added.
* http/tests/workers/service/resources/service-worker-fetch-worker.js:
* http/tests/workers/service/resources/updating-fetch-worker.php: Added.
* http/tests/workers/service/resources/updating-worker.php: Added.
* http/tests/workers/service/self_registration_update.html:
* http/tests/workers/service/service-worker-registration-gc-event.html:
Undate WebKit-specific tests to reflect behavior change.

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

19 files changed:
LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/http/tests/workers/service/controller-change.html
LayoutTests/http/tests/workers/service/registration-updateViaCache-all-importScripts.html
LayoutTests/http/tests/workers/service/registration-updateViaCache-all.html
LayoutTests/http/tests/workers/service/registration-updateViaCache-imports-importScripts.html
LayoutTests/http/tests/workers/service/registration-updateViaCache-none-importScripts.html
LayoutTests/http/tests/workers/service/registration-updateViaCache-none.html
LayoutTests/http/tests/workers/service/resources/self_registration_update-worker.js [deleted file]
LayoutTests/http/tests/workers/service/resources/self_registration_update-worker.php [new file with mode: 0644]
LayoutTests/http/tests/workers/service/resources/service-worker-fetch-worker.js
LayoutTests/http/tests/workers/service/resources/updating-fetch-worker.php [new file with mode: 0644]
LayoutTests/http/tests/workers/service/resources/updating-worker.php [new file with mode: 0644]
LayoutTests/http/tests/workers/service/self_registration_update.html
LayoutTests/http/tests/workers/service/service-worker-registration-gc-event.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/registration-updateviacache.https-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/workers/service/server/SWServerJobQueue.cpp

index af8fa6b037de3d91c6f5c53a6715b825f64c9624..eed294ffcb8d61a714d7a5d3c6a2ee5b40596436 100644 (file)
@@ -1,3 +1,29 @@
+2017-12-16  Chris Dumez  <cdumez@apple.com>
+
+        Add optimization when updating a SW registration results in the exact same script
+        https://bugs.webkit.org/show_bug.cgi?id=180891
+
+        Reviewed by Geoffrey Garen.
+
+        * TestExpectations:
+        Skip bad WPT test that is timing out for us and Firefox. I'll file an upstream PR
+        to fix it.
+
+        * http/tests/workers/service/controller-change.html:
+        * http/tests/workers/service/registration-updateViaCache-all-importScripts.html:
+        * http/tests/workers/service/registration-updateViaCache-all.html:
+        * http/tests/workers/service/registration-updateViaCache-imports-importScripts.html:
+        * http/tests/workers/service/registration-updateViaCache-none-importScripts.html:
+        * http/tests/workers/service/registration-updateViaCache-none.html:
+        * http/tests/workers/service/resources/self_registration_update-worker.js: Removed.
+        * http/tests/workers/service/resources/self_registration_update-worker.php: Added.
+        * http/tests/workers/service/resources/service-worker-fetch-worker.js:
+        * http/tests/workers/service/resources/updating-fetch-worker.php: Added.
+        * http/tests/workers/service/resources/updating-worker.php: Added.
+        * http/tests/workers/service/self_registration_update.html:
+        * http/tests/workers/service/service-worker-registration-gc-event.html:
+        Undate WebKit-specific tests to reflect behavior change.
+
 2017-12-16  Youenn Fablet  <youenn@apple.com>
 
         Service worker script fetch request should set the Service-Worker header
index e30b6bbb83ec07645beef92ed20f09807a0ad3c7..1572d22e4b935d7fff45d1c10cc24273e377e71c 100644 (file)
@@ -143,6 +143,10 @@ imported/w3c/web-platform-tests/secure-contexts/basic-shared-worker.https.html [
 imported/w3c/web-platform-tests/secure-contexts/shared-worker-insecure-first.https.html [ Skip ]
 imported/w3c/web-platform-tests/secure-contexts/shared-worker-secure-first.https.html [ Skip ]
 
+# This test seems wrong as it call update() for a script that does not change and expected an updatefound event.
+# Per specification, if the scripts are identical, we do not install the new script and return the existing registration.
+imported/w3c/web-platform-tests/service-workers/service-worker/import-scripts-redirect.https.html [ Skip ]
+
 # Skip service worker tests that are timing out.
 imported/w3c/web-platform-tests/fetch/api/abort/general-serviceworker.https.html [ Skip ]
 imported/w3c/web-platform-tests/service-workers/service-worker/extendable-event-waituntil.https.html [ Skip ]
index 161b71dc380314d0975cda3e9f73beb26e1ea854..433ae0d2a7d2f73b1952caa8e8222cccbedf9259 100644 (file)
@@ -23,7 +23,7 @@ function listenForControllerChange(frame)
 
 async function test() {
     let scopeURL = "/";
-    let registration = await registerAndWaitForActive("resources/service-worker-fetch-worker.js", scopeURL);
+    let registration = await registerAndWaitForActive("resources/updating-fetch-worker.php", scopeURL);
     let frame = await withFrame(scopeURL);
     initialController = frame.contentWindow.navigator.serviceWorker.controller;
     if (initialController === null) {
index 83bad0659362812f32921e2f9511226967d96b25..f298fc0acdf3441dabeeea73ff0275d34583d854 100644 (file)
@@ -23,7 +23,8 @@ async function test()
         await waitForState(worker1, "activated");
         let randomId1 = await getRandomIdFromWorker(worker1);
 
-        await registration.update();
+        await registration.unregister();
+        registration = await navigator.serviceWorker.register("resources/import-cacheable-script-worker.js", { updateViaCache: "all" });
         let worker2 = registration.installing;
         await waitForState(worker2, "activated");
         let randomId2 = await getRandomIdFromWorker(worker2);
index 20f76b770a9f100c43af9b58ecbb2dc6a4654ee6..0757da958b4f7757a10c49a9c298e43d528c4594 100644 (file)
@@ -24,8 +24,12 @@ async function test()
         let randomId1 = await getRandomIdFromWorker(worker1);
 
         await registration.update();
-        let worker2 = registration.installing;
-        await waitForState(worker2, "activated");
+        if (registration.installing) {
+            log("FAIL: The new script should have come from the cache and thus be identical, in which case it would not have be reinstalled");
+            finishSWTest();
+        }
+        let worker2 = registration.active;
         let randomId2 = await getRandomIdFromWorker(worker2);
 
         if (randomId1 === randomId2)
index 5a6605457095800ebf31736644f5a87f4392e427..d2e93df0b9d6df75de6382b0295e0423f30344a1 100644 (file)
@@ -23,7 +23,8 @@ async function test()
         await waitForState(worker1, "activated");
         let randomId1 = await getRandomIdFromWorker(worker1);
 
-        await registration.update();
+        await registration.unregister();
+        registration = await navigator.serviceWorker.register("resources/import-cacheable-script-worker.js", { updateViaCache: "imports" });
         let worker2 = registration.installing;
         await waitForState(worker2, "activated");
         let randomId2 = await getRandomIdFromWorker(worker2);
index b6329e752ee1991ad0c4399a3651c6eeedadca2d..30cc294044690805d44e44aca92a7b57b8bf0927 100644 (file)
@@ -23,7 +23,8 @@ async function test()
         await waitForState(worker1, "activated");
         let randomId1 = await getRandomIdFromWorker(worker1);
 
-        await registration.update();
+        await registration.unregister();
+        registration = await navigator.serviceWorker.register("resources/import-cacheable-script-worker.js", { updateViaCache: "none" });
         let worker2 = registration.installing;
         await waitForState(worker2, "activated");
         let randomId2 = await getRandomIdFromWorker(worker2);
index 1fd46fa4d77ad59f04d278251b9f14ef82834354..75226786d3e82c705829ca8da1b80aa0214a9fc4 100644 (file)
@@ -23,7 +23,8 @@ async function test()
         await waitForState(worker1, "activated");
         let randomId1 = await getRandomIdFromWorker(worker1);
 
-        await registration.update();
+        await registration.unregister();
+        registration = await navigator.serviceWorker.register("resources/cacheable-script-worker.php", { updateViaCache: "none" });
         let worker2 = registration.installing;
         await waitForState(worker2, "activated");
         let randomId2 = await getRandomIdFromWorker(worker2);
diff --git a/LayoutTests/http/tests/workers/service/resources/self_registration_update-worker.js b/LayoutTests/http/tests/workers/service/resources/self_registration_update-worker.js
deleted file mode 100644 (file)
index 2b36acb..0000000
+++ /dev/null
@@ -1,4 +0,0 @@
-self.addEventListener("message", (event) => {
-    self.registration.update();
-});
-
diff --git a/LayoutTests/http/tests/workers/service/resources/self_registration_update-worker.php b/LayoutTests/http/tests/workers/service/resources/self_registration_update-worker.php
new file mode 100644 (file)
index 0000000..12b293f
--- /dev/null
@@ -0,0 +1,15 @@
+<?php
+    header('Content-Type: text/javascript');
+
+    $randomId = '';
+    $charset = 'ABCDEFGHIKLMNOPQRSTUVWXYZ0123456789';
+    for ($i = 0; $i < 16; $i++)
+        $randomId .= $charset[rand(0, strlen($charset) - 1)];
+
+    echo 'let randomId = "' . $randomId . '";';
+?>
+
+self.addEventListener("message", (event) => {
+    self.registration.update();
+});
+
index fa6b4c548bf8810bdca61cbf2c60b39d317dbcf4..c332651db54b088f6b21979d8753f41f608457b2 100644 (file)
@@ -1,7 +1,5 @@
 var status = "no status";
 
-self.skipWaiting();
-
 self.addEventListener("fetch", (event) => {
     if (event.request.url.indexOf("status") !== -1) {
         event.respondWith(new Response(null, {status: 200, statusText: status}));
diff --git a/LayoutTests/http/tests/workers/service/resources/updating-fetch-worker.php b/LayoutTests/http/tests/workers/service/resources/updating-fetch-worker.php
new file mode 100644 (file)
index 0000000..0d01fce
--- /dev/null
@@ -0,0 +1,16 @@
+<?php
+    header('Content-Type: text/javascript');
+
+    $randomId = '';
+    $charset = 'ABCDEFGHIKLMNOPQRSTUVWXYZ0123456789';
+    for ($i = 0; $i < 16; $i++)
+        $randomId .= $charset[rand(0, strlen($charset) - 1)];
+
+    echo 'let randomId = "' . $randomId . '";';
+?>
+
+self.skipWaiting();
+
+self.addEventListener("fetch", (event) => {
+    event.respondWith(new Response(null, {status: 200, statusText: "Found"}));
+});
diff --git a/LayoutTests/http/tests/workers/service/resources/updating-worker.php b/LayoutTests/http/tests/workers/service/resources/updating-worker.php
new file mode 100644 (file)
index 0000000..73e6ba7
--- /dev/null
@@ -0,0 +1,10 @@
+<?php
+    header('Content-Type: text/javascript');
+
+    $randomId = '';
+    $charset = 'ABCDEFGHIKLMNOPQRSTUVWXYZ0123456789';
+    for ($i = 0; $i < 16; $i++)
+        $randomId .= $charset[rand(0, strlen($charset) - 1)];
+
+    echo 'let randomId = "' . $randomId . '";';
+?>
index 3c646ae903203848d36ac74e1c21087f95e26811..3e7dfb9b176e6eb5450bffff54e782e0f2b18452 100644 (file)
@@ -6,7 +6,7 @@
 log("* Add basic testing for ServiceWorkerGlobalScope.registration.update()");
 log("");
 
-navigator.serviceWorker.register("resources/self_registration_update-worker.js", { }).then(function(_registration) {
+navigator.serviceWorker.register("resources/self_registration_update-worker.php", { }).then(function(_registration) {
     registration = _registration;
     worker = registration.installing;
 
index 5898137a4ac4864eb91be2b21f772ed308345d44..36b1d47623c62fe718fcafeed2516ada7389c32c 100644 (file)
@@ -7,7 +7,7 @@
 <script>
 let updatefoundCount = 0;
 
-navigator.serviceWorker.register("resources/empty-worker.js", { }).then(function(registration) {
+navigator.serviceWorker.register("resources/updating-worker.php", { }).then(function(registration) {
     registration.addEventListener("updatefound", function() {
         gc();
         log("updatefound event fired");
index 371de1badef6bd6221e34cf3ae044df11d2fbe56..0f0f489d67a0b90e676e4ac550538f7ee740e497 100644 (file)
@@ -1,3 +1,14 @@
+2017-12-16  Chris Dumez  <cdumez@apple.com>
+
+        Add optimization when updating a SW registration results in the exact same script
+        https://bugs.webkit.org/show_bug.cgi?id=180891
+
+        Reviewed by Geoffrey Garen.
+
+        Rebaseline WPT test now that all checks are passing.
+
+        * web-platform-tests/service-workers/service-worker/registration-updateviacache.https-expected.txt:
+
 2017-12-15  Chris Dumez  <cdumez@apple.com>
 
         Drop service workers stubs tests
index fc7773d753df86cef25ba4b750879c873d286b77..8201a01505cbb1a3db30e4b5781012532e298a4a 100644 (file)
@@ -1,15 +1,15 @@
 
 PASS register-with-updateViaCache-undefined 
 PASS register-with-updateViaCache-imports 
-FAIL register-with-updateViaCache-all assert_equals: No new service worker expected null but got object "[object ServiceWorker]"
+PASS register-with-updateViaCache-all 
 PASS register-with-updateViaCache-none 
 PASS register-with-updateViaCache-undefined-then-undefined 
 PASS register-with-updateViaCache-undefined-then-imports 
-FAIL register-with-updateViaCache-undefined-then-all assert_equals: No new service worker expected null but got object "[object ServiceWorker]"
+PASS register-with-updateViaCache-undefined-then-all 
 PASS register-with-updateViaCache-undefined-then-none 
 PASS register-with-updateViaCache-imports-then-undefined 
 PASS register-with-updateViaCache-imports-then-imports 
-FAIL register-with-updateViaCache-imports-then-all assert_equals: No new service worker expected null but got object "[object ServiceWorker]"
+PASS register-with-updateViaCache-imports-then-all 
 PASS register-with-updateViaCache-imports-then-none 
 PASS register-with-updateViaCache-all-then-undefined 
 PASS register-with-updateViaCache-all-then-imports 
@@ -17,6 +17,6 @@ PASS register-with-updateViaCache-all-then-all
 PASS register-with-updateViaCache-all-then-none 
 PASS register-with-updateViaCache-none-then-undefined 
 PASS register-with-updateViaCache-none-then-imports 
-FAIL register-with-updateViaCache-none-then-all assert_equals: No new service worker expected null but got object "[object ServiceWorker]"
+PASS register-with-updateViaCache-none-then-all 
 PASS register-with-updateViaCache-none-then-none 
 
index 2ff0ffccf63119dfd26a13abfc4c3e8146a1127f..cc7abe063f666966bf0d9d914ce4d9d90499589d 100644 (file)
@@ -1,3 +1,18 @@
+2017-12-16  Chris Dumez  <cdumez@apple.com>
+
+        Add optimization when updating a SW registration results in the exact same script
+        https://bugs.webkit.org/show_bug.cgi?id=180891
+
+        Reviewed by Geoffrey Garen.
+
+        Add optimization when updating a SW registration results in the exact same script:
+        - https://w3c.github.io/ServiceWorker/#update-algorithm (step 8)
+
+        No new tests, rebaselined existing test.
+
+        * workers/service/server/SWServerJobQueue.cpp:
+        (WebCore::SWServerJobQueue::scriptFetchFinished):
+
 2017-12-16  Youenn Fablet  <youenn@apple.com>
 
         Service worker script fetch request should set the Service-Worker header
index eb2db5ba688a6c2a3e5686b50c2aeadce3c8a257..9edbe0b8da6f37be8e7740a817a8858e2474379e 100644 (file)
@@ -80,11 +80,19 @@ void SWServerJobQueue::scriptFetchFinished(SWServer::Connection& connection, con
         return;
     }
 
-    // FIXME: If newestWorker is not null, newestWorker's script url equals job's script url with the exclude fragments
+    // If newestWorker is not null, newestWorker's script url equals job's script url with the exclude fragments
     // flag set, and script's source text is a byte-for-byte match with newestWorker's script resource's source
     // text, then:
-    // - Invoke Resolve Job Promise with job and registration.
-    // - Invoke Finish Job with job and abort these steps.
+    if (newestWorker && equalIgnoringFragmentIdentifier(newestWorker->scriptURL(), job.scriptURL) && result.script == newestWorker->script()) {
+        // FIXME: for non classic scripts, check the script’s module record's [[ECMAScriptCode]].
+
+        // Invoke Resolve Job Promise with job and registration.
+        m_server.resolveRegistrationJob(job, registration->data(), ShouldNotifyWhenResolved::No);
+
+        // Invoke Finish Job with job and abort these steps.
+        finishCurrentJob();
+        return;
+    }
 
     // FIXME: Support the proper worker type (classic vs module)
     m_server.updateWorker(connection, job.identifier(), *registration, job.scriptURL, result.script, WorkerType::Classic);