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 af8fa6b..eed294f 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 e30b6bb..1572d22 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 161b71d..433ae0d 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 83bad06..f298fc0 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 20f76b7..0757da9 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 5a66054..d2e93df 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 b6329e7..30cc294 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 1fd46fa..7522678 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 fa6b4c5..c332651 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 3c646ae..3e7dfb9 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 5898137..36b1d47 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 371de1b..0f0f489 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 fc7773d..8201a01 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 2ff0ffc..cc7abe0 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 eb2db5b..9edbe0b 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);