Re: [w3c/ServiceWorker] Skip executing fetch handlers if it is no-op. (PR #1672)

@domenic commented on this pull request.

Some suggested tweaks to be more precise, plus a suggestion that you bail out in the object case instead of potentially triggering side effects from the `handleEvent` getter.

>            1. Run the <a>responsible event loop</a> specified by |settingsObject| until it is destroyed.
           1. [=map/Clear=] |workerGlobalScope|'s [=map of active timers=].
       1. Wait for |serviceWorker| to be [=running=], or for |startFailed| to be true.
       1. If |startFailed| is true, then return *failure*.
       1. Return |serviceWorker|'s [=start status=].
   </section>
 
+  <section algorithm>
+    <h3 id="empty-handler-identification-algorithm"><dfn>Empty Handler identification</dfn></h3>
+
+      : Input
+      :: |workerGlobalScope|, a [=service worker/global object=].
+      : Output
+      :: a boolean
+
+      1. If |workerGlobalScope|'s <a>set of event types to handle</a> does not [=set/contain=] <code>fetch</code>, then returns false.
+      1. set |eventListenerList| to [=map/value=] whose [=map/key=] is <code>fetch</code> in |workerGlobalScope|'s <a>set of event types to handle</a>.
+      1. [=list/For each=] |eventListener| of |eventListenerList|:
+          1. If |eventListener|'s [=callback=] is not null:
+              1. Set |callback| to an ECMAScript object coverted from |eventListener|'s [=callback=], which is [=idl-callback-interface=].

```suggestion
              1. Set |callback| to the result of [=converting|convert to an ECMAScript value=] |eventListener|'s [=callback=] to an ECMAScript value.
```

>            1. Run the <a>responsible event loop</a> specified by |settingsObject| until it is destroyed.
           1. [=map/Clear=] |workerGlobalScope|'s [=map of active timers=].
       1. Wait for |serviceWorker| to be [=running=], or for |startFailed| to be true.
       1. If |startFailed| is true, then return *failure*.
       1. Return |serviceWorker|'s [=start status=].
   </section>
 
+  <section algorithm>
+    <h3 id="empty-handler-identification-algorithm"><dfn>Empty Handler identification</dfn></h3>
+
+      : Input
+      :: |workerGlobalScope|, a [=service worker/global object=].
+      : Output
+      :: a boolean
+
+      1. If |workerGlobalScope|'s <a>set of event types to handle</a> does not [=set/contain=] <code>fetch</code>, then returns false.
+      1. set |eventListenerList| to [=map/value=] whose [=map/key=] is <code>fetch</code> in |workerGlobalScope|'s <a>set of event types to handle</a>.
+      1. [=list/For each=] |eventListener| of |eventListenerList|:
+          1. If |eventListener|'s [=callback=] is not null:
+              1. Set |callback| to an ECMAScript object coverted from |eventListener|'s [=callback=], which is [=idl-callback-interface=].
+              1. If |callback| is a [=function=], and [=function body=] is not empty (i.e. either [=statement=] or [=declaration=] exist), then return false.

```suggestion
              1. If [$IsCallable$](|callback|), and |callback|'s [=function body=] is not empty (i.e. either a [=statement=] or [=declaration=] exist), then return false.
```

> +              1. If |callback| does not have <code>handleEvent</code> property, then return false.
+              1. If |callback|'s <code>handleEvent</code> property is not a [=function=], then return false.
+              1. If a [=function body=] of |callback|'s <code>handleEvent</code> property is not empty (i.e. either [=statement=] or [=declaration=] exist), return false.

```suggestion
              1. Return false.
```

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

Message ID: <w3c/ServiceWorker/pull/1672/review/1306579062@github.com>