Re: [w3c/ServiceWorker] Avoid preload if there’s no fetch listener. Fixes #1058. (#1062)

jungkees requested changes on this pull request.



> @@ -2905,7 +2909,6 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
       1. Else if |request| is a <a>subresource request</a>, then:
           1. If |client|'s <a>active service worker</a> is non-null, set |registration| to |client|'s <a>active service worker</a>'s <a>containing service worker registration</a>.
           1. Else, return null.
-      1. Let |activeWorker| be |registration|'s <a>active worker</a>.

As |registration| is set in either the non-subresource request block or the subresource request block, moving this assignment up to somewhere seems not good. I think it'd be better to just use |registration|'s active worker instead of |activeWorker| in https://github.com/w3c/ServiceWorker/pull/1062/files#diff-27b79860afe28f01aed4f1f6228367faR2888.

> @@ -2884,7 +2885,10 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
           1. Set |registration| to the result of running <a>Match Service Worker Registration</a> algorithm passing |request|'s [=request/url=] as the argument.
           1. If |registration| is null or |registration|'s <a>active worker</a> is null, return null.
           1. If |request|'s [=request/destination=] is not {{RequestDestination/"report"}}, set |reservedClient|'s <a>active service worker</a> to |registration|'s <a>active worker</a>.
-          1. If |request| is a [=navigation request=] and |registration|'s [=navigation preload enabled flag=] is set, and |request|'s [=request/method=] is \`<code>GET</code>\`, then:
+          1. If |request| is a [=navigation request=], |registration|'s [=navigation preload enabled flag=] is set, |request|'s [=request/method=] is \`<code>GET</code>\`, and |activeWorker|'s <a>set of event types to handle</a> contains <code>fetch</code>, then:

Using |registration|'s active worker instead of |activeWorker| would be better here to not move |activeWorker| declaration statement. (See https://github.com/w3c/ServiceWorker/pull/1062/files#r97691892).

> @@ -2884,7 +2885,10 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
           1. Set |registration| to the result of running <a>Match Service Worker Registration</a> algorithm passing |request|'s [=request/url=] as the argument.
           1. If |registration| is null or |registration|'s <a>active worker</a> is null, return null.
           1. If |request|'s [=request/destination=] is not {{RequestDestination/"report"}}, set |reservedClient|'s <a>active service worker</a> to |registration|'s <a>active worker</a>.
-          1. If |request| is a [=navigation request=] and |registration|'s [=navigation preload enabled flag=] is set, and |request|'s [=request/method=] is \`<code>GET</code>\`, then:
+          1. If |request| is a [=navigation request=], |registration|'s [=navigation preload enabled flag=] is set, |request|'s [=request/method=] is \`<code>GET</code>\`, and |activeWorker|'s <a>set of event types to handle</a> contains <code>fetch</code>, then:
+
+              Note: If the above is true except |activeWorker|'s <a>set of event types to handle</a> **does not** contain <code>fetch</code>, then the user agent may wish to show a console warning, as the developer's intent isn't clear.

Likewise, |registration|'s active worker instead of |activeWorker| here too.

-- 
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/1062#pullrequestreview-18309335