Re: [w3c/ServiceWorker] Clients.get: block on reserved clients. (#1323)

wanderview approved this pull request.

Thanks!

> -                    1. Let |visibilityState| be null.
-                    1. Let |focusState| be false.
-                    1. Let |ancestorOriginsList| be the empty list.
-                    1. If |client| is an [=environment settings object=], set |browsingContext| to |client|'s [=environment settings object/global object=]'s [=/browsing context=].
-                    1. Else, set |browsingContext| to |client|’s [=environment/target browsing context=].
-                    1. <a>Queue a task</a> |task| to run the following substeps on |browsingContext|'s <a>event loop</a> using the <a>user interaction task source</a>:
-                        1. Set |visibilityState| to |browsingContext|'s <a>active document</a>'s {{Document/visibilityState}} attribute value.
-                        1. Set |focusState| to the result of running the <a>has focus steps</a> with |browsingContext|'s <a>active document</a> as the argument.
-                        1. If |client| is a <a>window client</a>, set |ancestorOriginsList| to |browsingContext|'s <a>active document</a>'s <a>relevant global object</a>'s {{Location}} object's [=Location/ancestor origins list=]'s associated list.
-                    1. Wait for |task| to have executed.
-                    1. Let |windowClient| be the result of running <a>Create Window Client</a> algorithm with |client|, |visibilityState|, |focusState|, and |ancestorOriginsList| as the arguments.
-                    1. Resolve |promise| with |windowClient| and abort these steps.
+                1. If |client|'s [=environment/id=] is not |id|, [=continue=].
+                1. Wait for either |client|'s [=environment/execution ready flag=] to be set or for |client|'s [=discarded flag=] to be set.
+                1. If |client|'s [=environment/execution ready flag=] is set, then invoke [=Resolve Get Client Promise=] with |client| and |promise|.
+                1. Else, resolve |promise| with undefined.

nit: Technically I think you could leave this "resolve with undefined" out as the one following the loop would take effect.  What you have might be clearer about intent, though.

> +          1. If |client|’s <a>creation URL</a> is not a <a>potentially trustworthy URL</a>, reject |promise| with a "{{SecurityError}}" {{DOMException}} and return.
+      1. If |client| is an [=environment settings object=] and is not a [=window client=], then:
+          1. Let |clientObject| be the result of running <a>Create Client</a> algorithm with |client| as the argument.
+          1. Resolve |promise| with |clientObject| and return.
+      1. Else:
+          1. Let |browsingContext| be null.
+          1. Let |visibilityState| be null.
+          1. Let |focusState| be false.
+          1. Let |ancestorOriginsList| be the empty list.
+          1. If |client| is an [=environment settings object=], set |browsingContext| to |client|'s [=environment settings object/global object=]'s [=/browsing context=].
+          1. Else, set |browsingContext| to |client|’s [=environment/target browsing context=].
+          1. <a>Queue a task</a> |task| to run the following substeps on |browsingContext|'s <a>event loop</a> using the <a>user interaction task source</a>:
+              1. Set |visibilityState| to |browsingContext|'s <a>active document</a>'s {{Document/visibilityState}} attribute value.
+              1. Set |focusState| to the result of running the <a>has focus steps</a> with |browsingContext|'s <a>active document</a> as the argument.
+              1. If |client| is a <a>window client</a>, set |ancestorOriginsList| to |browsingContext|'s <a>active document</a>'s <a>relevant global object</a>'s {{Location}} object's [=Location/ancestor origins list=]'s associated list.
+          1. Wait for |task| to have executed.

I realize this part of the PR is mostly preserving the original algorithm, but should we check to see if the client was discarded or destroyed between running the task and getting the result back?  Or otherwise say that if any error occurs here we resolve undefined, etc?

-- 
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/1323#pullrequestreview-126787427