Don't fail update upon error fetching the incumbent worker's import s… (#1377)

* Don't fail update upon error fetching the incumbent worker's import scripts.

To include imported scripts in the byte-for-byte comparison, the Update
algorithm fetches the incumbent worker's scripts in order to
detect if there is a difference. But it would abort the update upon
MIME type error if fetching the script failed. This is undesirable
because the new worker might not use these scripts at all, e.g.,
the scripts may have legitimately been removed from the server.

I considered setting the Updated flag on failure, but it turns out Firefox
treats the failure as "no change" and Chrome's WIP implementation
was also doing that. The consensus at #1374 is to just consider
network errors to be "no change", so do that.

Author: mfalken

docs/index.bs

@@ -163,13 +163,17 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
 
     A [=/service worker=] has an associated <dfn export id="dfn-script-resource-map">script resource map</dfn> which is an <a>ordered map</a> where the keys are [=/URLs=] and the values are [=/responses=].
 
+    A [=/service worker=] has an associated  <dfn export id="dfn-set-of-used-scripts">set of used scripts</dfn> (a [=ordered set|set=]) whose [=list/item=] is a [=/URL=]. It is initially a new [=ordered set|set=].
+
+    Note: The [=set of used scripts=] is only used to prune unused resources from a new worker's map after installation, that were populated based on the old worker's map during the update check.
+
     A [=/service worker=] has an associated <dfn export id="dfn-skip-waiting-flag">skip waiting flag</dfn>. Unless stated otherwise it is unset.
 
     A [=/service worker=] has an associated <dfn export id="dfn-classic-scripts-imported-flag">classic scripts imported flag</dfn>. It is initially unset.
 
-    A [=/service worker=] has an associated <dfn export id="dfn-set-of-event-types-to-handle">set of event types to handle</dfn> (a [=ordered set|set=]) whose [=list/item=] is an <a>event listener</a>'s event type. It is initially an empty set.
+    A [=/service worker=] has an associated <dfn export id="dfn-set-of-event-types-to-handle">set of event types to handle</dfn> (a [=ordered set|set=]) whose [=list/item=] is an <a>event listener</a>'s event type. It is initially a new [=ordered set|set=].
 
-    A [=/service worker=] has an associated <dfn export id="dfn-set-of-extended-events">set of extended events</dfn> (a [=ordered set|set=]) whose [=list/item=] is an {{ExtendableEvent}}. It is initially an empty set.
+    A [=/service worker=] has an associated <dfn export id="dfn-set-of-extended-events">set of extended events</dfn> (a [=ordered set|set=]) whose [=list/item=] is an {{ExtendableEvent}}. It is initially a new [=ordered set|set=].
 
     <section>
       <h4 id="service-worker-lifetime">Lifetime</h4>
@@ -2063,8 +2067,13 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
       When the <dfn method for="ServiceWorkerGlobalScope" id="importscripts-method"><code>importScripts(|urls|)</code></dfn> method is called on a {{ServiceWorkerGlobalScope}} object, the user agent *must* <a>import scripts into worker global scope</a>, given this {{ServiceWorkerGlobalScope}} object and |urls|, and with the following steps to [=fetching scripts/perform the fetch=] given the [=/request=] |request|:
 
         1. Let |serviceWorker| be |request|'s [=request/client=]'s [=environment settings object/global object=]'s [=ServiceWorkerGlobalScope/service worker=].
-        1. If |serviceWorker|'s [=script resource map=][|request|'s [=request/url=]] [=map/exists=], return the [=map/entry=]'s [=map/value=].
-        1. If |serviceWorker|'s [=state=] is not *parsed* or *installing*, return a [=network error=].
+        1. Let |map| be |serviceWorker|'s [=script resource map=].
+        1. Let |url| be |request|'s [=request/url=].
+        1. If |serviceWorker|'s [=state=] is not *parsed* or *installing*:
+            1. Return |map|[|url|] if it [=map/exists=] and a [=network error=] otherwise.
+        1. If |map|[|url|] [=map/exists=]:
+            1. [=set/Append=] |url| to |serviceWorker|'s [=set of used scripts=].
+            1. Return |map|[|url|].
         1. Let |registration| be |serviceWorker|'s [=containing service worker registration=].
         1. Set |request|'s [=service-workers mode=] to "`none`".
         1. Set |request|'s [=request/cache mode=] to "`no-cache`" if any of the following are true:
@@ -2074,7 +2083,8 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
         1. Let |response| be the result of [=fetch|fetching=] |request|.
         1. If |response|’s [=response/cache state=] is not "`local`", set |registration|’s [=service worker registration/last update check time=] to the current time.
         1. If |response|'s [=unsafe response=] is a [=bad import script response=], then return a [=network error=].
-        1. [=map/Set=] |serviceWorker|'s [=script resource map=][|request|'s [=request/url=]] to |response|.
+        1. [=map/Set=] |map|[|url|] to |response|.
+        1. [=set/Append=] |url| to |serviceWorker|'s [=set of used scripts=].
         1. Set |serviceWorker|'s [=classic scripts imported flag=].
         1. Return |response|.
     </section>
@@ -2404,7 +2414,7 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
           1. Invoke [=Reject Job Promise=] with |job| and `TypeError`.
           1. Invoke <a>Finish Job</a> with |job| and abort these steps.
       1. Let |newestWorker| be the result of running <a>Get Newest Worker</a> algorithm passing |registration| as the argument.
-      1. If |job|'s <a>job type</a> is *update*, and |newestWorker|'s [=service worker/script url=] does not [=url/equal=] |job|'s [=job/script url=], then:
+      1. If |job|'s <a>job type</a> is *update*, and |newestWorker| is not null and its [=service worker/script url=] does not [=url/equal=] |job|'s [=job/script url=], then:
           1. Invoke [=Reject Job Promise=] with |job| and `TypeError`.
           1. Invoke <a>Finish Job</a> with |job| and abort these steps.
       1. Let |httpsState| be "<code>none</code>".
@@ -2458,9 +2468,11 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
           1. Else:
               1. Invoke [=Reject Job Promise=] with |job| and "{{SecurityError}}" {{DOMException}}.
               1. Asynchronously complete these steps with a <a>network error</a>.
-          1. Set |updatedResourceMap|[|request|'s [=request/url=]] to |response|.
+          1. Let |url| be |request|'s [=request/url=].
+          1. Set |updatedResourceMap|[|url|] to |response|.
           1. If |response|'s [=response/cache state=] is not "`local`", set |registration|'s [=last update check time=] to the current time.
-          1. If |newestWorker| is null, or |newestWorker|'s [=script resource map=][|request|'s [=request/url=]]'s [=response/body=] is not byte-for-byte identical with |response|'s [=response/body=], set |hasUpdatedResources| to true.
+          1. Let |map| be |newestWorker|'s [=script resource map=] if |newestWorker| is not null, and null otherwise.
+          1. If |map| is null or |map|[|url|]'s [=response/body=] is not byte-for-byte identical with |response|'s [=response/body=], set |hasUpdatedResources| to true.
           1. Else if |newestWorker|'s [=classic scripts imported flag=] is set, then:
 
               Note: The following checks to see if an imported script has been updated, since the main script has not changed.
@@ -2476,7 +2488,10 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
                   1. Set |updatedResourceMap|[|importRequest|'s [=request/url=]] to |fetchedResponse|.
                   1. Set |fetchedResponse| to |fetchedResponse|'s [=unsafe response=].
                   1. If |fetchedResponse|'s [=response/cache state=] is not "`local`", set |registration|’s [=last update check time=] to the current time.
-                  1. If |fetchedResponse| is a [=bad import script response=], then asynchronously complete these steps with a [=network error=].
+                  1. If |fetchedResponse| is a [=bad import script response=], continue.
+
+                    Note: Bad responses for <a>importScripts()</a> are ignored for the purpose of the byte-to-byte check. Only good responses for the incumbent worker and good responses for the potential update worker are considered. See <a href="https://github.com/w3c/ServiceWorker/issues/1374">issue #1374</a> for some rationale.
+
                   1. If |fetchedResponse|'s [=response/body=] is not byte-for-byte identical with |storedResponse|'s [=unsafe response=]'s [=response/body=], set |hasUpdatedResources| to true.
 
                       Note: The control does not break the loop in this step to continue with all the imported scripts to populate the cache.
@@ -2496,9 +2511,8 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
           1. Invoke [=Resolve Job Promise=] with |job| and |registration|.
           1. Invoke [=Finish Job=] with |job| and abort these steps.
       1. Let |worker| be a new [=/service worker=].
-      1. Set |worker|'s [=service worker/script url=] to |job|'s [=job/script url=], |worker|'s <a>script resource</a> to |script|, and |worker|'s [=service worker/type=] to |job|'s <a>worker type</a>.
-      1. [=map/For each=] |url| → |response| of |updatedResourceMap|:
-          1. Set |worker|'s [=script resource map=][|url|] to |response|.
+      1. Set |worker|'s [=service worker/script url=] to |job|'s [=job/script url=], |worker|'s [=script resource=] to |script|, |worker|'s [=service worker/type=] to |job|'s [=worker type=], and |worker|'s [=script resource map=] to |updatedResourceMap|.
+      1. Append |url| to |worker|'s [=set of used scripts=].
       1. Set |worker|'s <a>script resource</a>'s <a>HTTPS state</a> to |httpsState|.
       1. Set |worker|'s <a>script resource</a>'s [=script resource/referrer policy=] to |referrerPolicy|.
       1. Invoke [=Run Service Worker=] algorithm given |worker|, with the *force bypass cache for importscripts flag* set if |job|'s [=job/force bypass cache flag=] is set, and with the following callback steps given |evaluationStatus|:
@@ -2567,6 +2581,10 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
           1. Run the <a>Update Registration State</a> algorithm passing |registration|, "<code>installing</code>" and null as the arguments.
           1. If |newestWorker| is null, invoke <a>Clear Registration</a> algorithm passing |registration| as its argument.
           1. Invoke <a>Finish Job</a> with |job| and abort these steps.
+      1. Let |map| be |registration|'s [=installing worker=]'s [=script resource map=].
+      1. Let |usedSet| be |registration|'s [=installing worker=]'s [=set of used scripts=].
+      1. [=map/For each=] |url| of |map|:
+          1. If |usedSet| does not [=list/contain=] |url|, then [=map/remove=] |map|[|url|].
       1. If |registration|'s <a>waiting worker</a> is not null, then:
           1. [=Terminate Service Worker|Terminate=] |registration|'s [=waiting worker=].
           1. Run the [=Update Worker State=] algorithm passing |registration|'s [=waiting worker=] and *redundant* as the arguments.