Re: [w3c/ServiceWorker] Include imported scripts to byte-check (#1023)

jungkees commented on this pull request.



> +      1. If |sourceScript| is a [=classic script=], then:
+          1. If |sourceScript|'s [=source text=] is not a byte-for-byte match with |targetScript|'s [=source text=], return false.
+          1. Let |sourceMap| be |sourceScript|'s [=script resource/imported scripts map=].
+          1. [=map/For each=] |url| → |response| of |sourceMap|:
+              1. Let |request| be a new [=/request=] whose [=request/url=] is |url|, [=request/client=] is |job|'s [=job/client=], [=request/type=] is "<code>script</code>", [=request/destination=] is "<code>script</code>", [=request/parser metadata=] is "<code>not parser-inserted</code>", [=request/synchronous flag=] is set, and whose [=request/use-URL-credentials flag=] is set.
+              1. Set |request|'s [=request/cache mode=] to "<code>no-cache</code>" if any of the following are true:
+                  * |registration|'s [=service worker registration/use cache=] is false.
+                  * |job|'s [=force bypass cache flag=] is set.
+                  * |registration|'s [=last update check time=] is not null and the time difference in seconds calculated by the current time minus |registration|’s [=last update check time=] is greater than 86400.
+              1. Let |targetResponse| be the result of [=fetch|fetching=] |request|.
+              1. If |targetResponse|'s <a for="response" href="https://github.com/whatwg/fetch/issues/376">cache state</a> is not "<code>local</code>", set |registration|’s [=last update check time=] to the current time.
+              1. Let |targetResponse| be |targetResponse|'s [=unsafe response=].
+              1. If |targetResponse|'s [=response/type=] is "<code>error</code>", or |targetResponse|'s [=response/status=] is not an [=ok status=], return false.
+              1. If the result of [=UTF-8 decoding=] |response|'s [=response/body=] is not a byte-for-byte match with the result of [=UTF-8 decoding=] |targetResponse|'s [=response/body=], return false.
+      1. If |sourceScript| is a [=module script=], then:
+          1. If |sourceScript|’s [=module script/module record=]'s \[[ECMAScriptCode]] is not a byte-for-byte match with |targetScript|’s [=module script/module record=]'s \[[ECMAScriptCode]], return false.

That sounds good. Moving the source text to the base script type will help us use the source text of scripts for both classic and module script cases. I'll spec it to checking everything in the module map with that change.

Another thing that needs a solution is the lifetime of the scripts and the map for SW. With other web workers, the lifetime of the scripts is tied to the `WorkerGlobalScope` object, so it doesn't store the main script and only stores the module scripts in the module map on the global. But with SW, the lifetime of the scripts outlives those globals, so it stores the main script and the imported scripts on a service worker concept rather than on the SW global. Because of this difference, I think we need to discuss around how to make the fetch a classic worker script and the fetch a module worker script graph work more correctly with SW. There's an opened issue for this: https://github.com/w3c/ServiceWorker/issues/1013.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/ServiceWorker/pull/1023