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.

@jakearchibald, I think you're right. I can't find any step that flattens the entire imported assets. It seems each module script has module record that has the parsed source text and info to the required modules (record's [[RequestedModules]] in ECMAScript language.) Getting to [ModuleEvaluation()](https://tc39.github.io/ecma262/#sec-moduleevaluation) when running a module script, it evaluates all the related module scripts recursively finding the dependencies.

And I think we should compare all the module scripts stored in an environment settings object's module map. I'd want to confirm this with @domenic before updating this part.

@domenic, SW intends to compare all the script assets including the main script and the imported scripts for both classic and module scripts. For classic scripts, SW has it's own imported scripts map that's used for this byte-by-byte check. But for module scripts, I think I misunderstood that the main script's module record has a flattened asset that include all the imported scripts in the graph. I'd like to check with you whether this is wrong, and if so, iterating an environment setting's object's module map and compare the source texts of all the module scripts in the map would be a right way for our purpose?

-- 
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