Re: [w3c/ServiceWorker] Add FetchEvent.handled (#1397) (#1496)

jakearchibald requested changes on this pull request.

I've expanded some of the lines out as it was a little tricky to work out which "if" was applying to what.

One remaining issue: |workerRealm| is out of scope when it's used.

> -          1. Invoke [=Update Service Worker Extended Events Set=] with |activeWorker| and |e|.
-          1. If |e|'s [=FetchEvent/respond-with entered flag=] is set, set |respondWithEntered| to true.
-          1. If |e|'s [=FetchEvent/wait to respond flag=] is set, then:
-              1. Wait until |e|'s [=FetchEvent/wait to respond flag=] is unset.
-              1. If |e|'s [=FetchEvent/respond-with error flag=] is set, set |handleFetchFailed| to true.
-              1. Else, set |response| to |e|'s [=FetchEvent/potential response=].
-          1. If |e|'s <a>canceled flag</a> is set, set |eventCanceled| to true.
-          1. If |fetchInstance| is [=fetch/terminated=], then [=queue a task=] to [=AbortSignal/signal abort=] on |requestObject|'s {{Request/signal}}.
-
-          If |task| is discarded, set |handleFetchFailed| to true.
-
-          The |task| *must* use |activeWorker|'s <a>event loop</a> and the <a>handle fetch task source</a>.
+      1. Else:
+          1. Let |workerRealm| be the [=relevant realm=] of the |activeWorker|'s [=service worker/global object=].
+          1. Set |eventHandled| to [=a new promise=] in |workerRealm|.
+          1. [=queue a task=] |task| to run the following substeps:

```suggestion
          1. [=Queue a task=] |task| to run the following substeps:
```

>  
       1. Wait for |task| to have executed or for |handleFetchFailed| to be true.
       1. If |shouldSoftUpdate| is true, then [=in parallel=] run the [=Soft Update=] algorithm with |registration|.
-      1. If |respondWithEntered| is false, then return a [=network error=] if |eventCanceled| is true and null otherwise.
-      1. If |handleFetchFailed| is true, then return a [=network error=].
-      1. Return |response|.
+      1. If |respondWithEntered| is false, then:
+          1. If |eventCanceled| is true, then [=reject=] |eventHandled| with a "{{NetworkError}}" {{DOMException}} in |workerRealm| if |eventHandled| is not null and return a [=network error=].

```suggestion
          1. If |eventCanceled| is true, then:
              1. If |eventHandled| is not null, then [=reject=] |eventHandled| with a "{{NetworkError}}" {{DOMException}} in |workerRealm|.
              1. Return a [=network error=].
```

>  
       1. Wait for |task| to have executed or for |handleFetchFailed| to be true.
       1. If |shouldSoftUpdate| is true, then [=in parallel=] run the [=Soft Update=] algorithm with |registration|.
-      1. If |respondWithEntered| is false, then return a [=network error=] if |eventCanceled| is true and null otherwise.
-      1. If |handleFetchFailed| is true, then return a [=network error=].
-      1. Return |response|.
+      1. If |respondWithEntered| is false, then:
+          1. If |eventCanceled| is true, then [=reject=] |eventHandled| with a "{{NetworkError}}" {{DOMException}} in |workerRealm| if |eventHandled| is not null and return a [=network error=].
+          1. [=Resolve=] |eventHandled| if |eventHandled| is not null and return null.

```suggestion
          1. If |eventHandled| is not null, then [=resolve=] |eventHandled|.
          1. Return null.
```

>  
       1. Wait for |task| to have executed or for |handleFetchFailed| to be true.
       1. If |shouldSoftUpdate| is true, then [=in parallel=] run the [=Soft Update=] algorithm with |registration|.
-      1. If |respondWithEntered| is false, then return a [=network error=] if |eventCanceled| is true and null otherwise.
-      1. If |handleFetchFailed| is true, then return a [=network error=].
-      1. Return |response|.
+      1. If |respondWithEntered| is false, then:
+          1. If |eventCanceled| is true, then [=reject=] |eventHandled| with a "{{NetworkError}}" {{DOMException}} in |workerRealm| if |eventHandled| is not null and return a [=network error=].
+          1. [=Resolve=] |eventHandled| if |eventHandled| is not null and return null.
+      1. If |handleFetchFailed| is true, then [=reject=] |eventHandled| with a "{{NetworkError}}" {{DOMException}} in |workerRealm| if |eventHandled| is not null and return a [=network error=].

```suggestion
      1. If |handleFetchFailed| is true, then:
          1. If |eventHandled| is not null, then [=reject=] |eventHandled| with a "{{NetworkError}}" {{DOMException}} in |workerRealm|.
          1. Return a [=network error=].
```

>  
       1. Wait for |task| to have executed or for |handleFetchFailed| to be true.
       1. If |shouldSoftUpdate| is true, then [=in parallel=] run the [=Soft Update=] algorithm with |registration|.
-      1. If |respondWithEntered| is false, then return a [=network error=] if |eventCanceled| is true and null otherwise.
-      1. If |handleFetchFailed| is true, then return a [=network error=].
-      1. Return |response|.
+      1. If |respondWithEntered| is false, then:
+          1. If |eventCanceled| is true, then [=reject=] |eventHandled| with a "{{NetworkError}}" {{DOMException}} in |workerRealm| if |eventHandled| is not null and return a [=network error=].
+          1. [=Resolve=] |eventHandled| if |eventHandled| is not null and return null.
+      1. If |handleFetchFailed| is true, then [=reject=] |eventHandled| with a "{{NetworkError}}" {{DOMException}} in |workerRealm| if |eventHandled| is not null and return a [=network error=].
+      1. [=Resolve=] |eventHandled| if |eventHandled| is not null and return |response|.

```suggestion
      1. If |eventHandled| is not null, then [=resolve=] |eventHandled|.
      1. Return |response|.
```

-- 
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/1496#pullrequestreview-345864435