Title: [225997] trunk
Revision
225997
Author
cdu...@apple.com
Date
2017-12-16 08:37:11 -0800 (Sat, 16 Dec 2017)

Log Message

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.

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.

Modified Paths

Added Paths

Removed Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (225996 => 225997)


--- trunk/LayoutTests/ChangeLog	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/ChangeLog	2017-12-16 16:37:11 UTC (rev 225997)
@@ -1,3 +1,29 @@
+2017-12-16  Chris Dumez  <cdu...@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  <you...@apple.com>
 
         Service worker script fetch request should set the Service-Worker header

Modified: trunk/LayoutTests/TestExpectations (225996 => 225997)


--- trunk/LayoutTests/TestExpectations	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/TestExpectations	2017-12-16 16:37:11 UTC (rev 225997)
@@ -143,6 +143,10 @@
 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 ]

Modified: trunk/LayoutTests/http/tests/workers/service/controller-change.html (225996 => 225997)


--- trunk/LayoutTests/http/tests/workers/service/controller-change.html	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/http/tests/workers/service/controller-change.html	2017-12-16 16:37:11 UTC (rev 225997)
@@ -23,7 +23,7 @@
 
 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) {

Modified: trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-all-importScripts.html (225996 => 225997)


--- trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-all-importScripts.html	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-all-importScripts.html	2017-12-16 16:37:11 UTC (rev 225997)
@@ -23,7 +23,8 @@
         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);

Modified: trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-all.html (225996 => 225997)


--- trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-all.html	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-all.html	2017-12-16 16:37:11 UTC (rev 225997)
@@ -24,8 +24,12 @@
         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)

Modified: trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-imports-importScripts.html (225996 => 225997)


--- trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-imports-importScripts.html	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-imports-importScripts.html	2017-12-16 16:37:11 UTC (rev 225997)
@@ -23,7 +23,8 @@
         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);

Modified: trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-none-importScripts.html (225996 => 225997)


--- trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-none-importScripts.html	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-none-importScripts.html	2017-12-16 16:37:11 UTC (rev 225997)
@@ -23,7 +23,8 @@
         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);

Modified: trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-none.html (225996 => 225997)


--- trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-none.html	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/http/tests/workers/service/registration-updateViaCache-none.html	2017-12-16 16:37:11 UTC (rev 225997)
@@ -23,7 +23,8 @@
         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);

Deleted: trunk/LayoutTests/http/tests/workers/service/resources/self_registration_update-worker.js (225996 => 225997)


--- trunk/LayoutTests/http/tests/workers/service/resources/self_registration_update-worker.js	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/http/tests/workers/service/resources/self_registration_update-worker.js	2017-12-16 16:37:11 UTC (rev 225997)
@@ -1,4 +0,0 @@
-self.addEventListener("message", (event) => {
-    self.registration.update();
-});
-

Added: trunk/LayoutTests/http/tests/workers/service/resources/self_registration_update-worker.php (0 => 225997)


--- trunk/LayoutTests/http/tests/workers/service/resources/self_registration_update-worker.php	                        (rev 0)
+++ trunk/LayoutTests/http/tests/workers/service/resources/self_registration_update-worker.php	2017-12-16 16:37:11 UTC (rev 225997)
@@ -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();
+});
+

Modified: trunk/LayoutTests/http/tests/workers/service/resources/service-worker-fetch-worker.js (225996 => 225997)


--- trunk/LayoutTests/http/tests/workers/service/resources/service-worker-fetch-worker.js	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/http/tests/workers/service/resources/service-worker-fetch-worker.js	2017-12-16 16:37:11 UTC (rev 225997)
@@ -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}));

Added: trunk/LayoutTests/http/tests/workers/service/resources/updating-fetch-worker.php (0 => 225997)


--- trunk/LayoutTests/http/tests/workers/service/resources/updating-fetch-worker.php	                        (rev 0)
+++ trunk/LayoutTests/http/tests/workers/service/resources/updating-fetch-worker.php	2017-12-16 16:37:11 UTC (rev 225997)
@@ -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"}));
+});

Added: trunk/LayoutTests/http/tests/workers/service/resources/updating-worker.php (0 => 225997)


--- trunk/LayoutTests/http/tests/workers/service/resources/updating-worker.php	                        (rev 0)
+++ trunk/LayoutTests/http/tests/workers/service/resources/updating-worker.php	2017-12-16 16:37:11 UTC (rev 225997)
@@ -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 . '";';
+?>

Modified: trunk/LayoutTests/http/tests/workers/service/self_registration_update.html (225996 => 225997)


--- trunk/LayoutTests/http/tests/workers/service/self_registration_update.html	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/http/tests/workers/service/self_registration_update.html	2017-12-16 16:37:11 UTC (rev 225997)
@@ -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;
 

Modified: trunk/LayoutTests/http/tests/workers/service/service-worker-registration-gc-event.html (225996 => 225997)


--- trunk/LayoutTests/http/tests/workers/service/service-worker-registration-gc-event.html	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/http/tests/workers/service/service-worker-registration-gc-event.html	2017-12-16 16:37:11 UTC (rev 225997)
@@ -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");

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (225996 => 225997)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2017-12-16 16:37:11 UTC (rev 225997)
@@ -1,3 +1,14 @@
+2017-12-16  Chris Dumez  <cdu...@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  <cdu...@apple.com>
 
         Drop service workers stubs tests

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/registration-updateviacache.https-expected.txt (225996 => 225997)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/registration-updateviacache.https-expected.txt	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/registration-updateviacache.https-expected.txt	2017-12-16 16:37:11 UTC (rev 225997)
@@ -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-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 
 

Modified: trunk/Source/WebCore/ChangeLog (225996 => 225997)


--- trunk/Source/WebCore/ChangeLog	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/Source/WebCore/ChangeLog	2017-12-16 16:37:11 UTC (rev 225997)
@@ -1,3 +1,18 @@
+2017-12-16  Chris Dumez  <cdu...@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  <you...@apple.com>
 
         Service worker script fetch request should set the Service-Worker header

Modified: trunk/Source/WebCore/workers/service/server/SWServerJobQueue.cpp (225996 => 225997)


--- trunk/Source/WebCore/workers/service/server/SWServerJobQueue.cpp	2017-12-16 09:26:39 UTC (rev 225996)
+++ trunk/Source/WebCore/workers/service/server/SWServerJobQueue.cpp	2017-12-16 16:37:11 UTC (rev 225997)
@@ -80,12 +80,20 @@
         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);
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to