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

jakearchibald requested changes on this pull request.



>  
       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}} and return a [=network error=].
+          1. [=resolve=] |eventHandled| and return null.

```suggestion
          1. [=Resolve=] |eventHandled| and 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}} and return a [=network error=].
+          1. [=resolve=] |eventHandled| and return null.
+      1. If |handleFetchFailed| is true, then [=reject=] |eventHandled| with a "{{NetworkError}}" {{DOMException}} and return a [=network error=].
+      1. [=resolve=] |eventHandled| and return |response|.

```suggestion
      1. [=Resolve=] |eventHandled| and return |response|.
```

> -          1. <a>Dispatch</a> |e| at |activeWorker|'s [=service worker/global object=].
-          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|. In following steps, [=resolve=] or [=reject=] |eventHandled| in |workerRealm| only. If |eventHandled| is null, ignore the [=resolve=] or [=reject=] operations on it.

> If |eventHandled| is null, ignore the [=resolve=] or [=reject=] operations on it.

I'm not sure we should hand-wave like this. Can we just perform a null check when we try to do stuff with this object?

> In following steps, [=resolve=] or [=reject=] |eventHandled| in |workerRealm| only.

I don't think this really works, since it doesn't matter which realm resolves/rejects the promise. The realm only matters for objects.

In this case, you'd just change the instances of

"[=reject=] |eventHandled| with a "{{NetworkError}}" {{DOMException}}"

to

"[=reject=] |eventHandled| with a "{{NetworkError}}" {{DOMException}} **in |workerRealm|**"

It's the error object that gets the realm.

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