Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Client type for ServiceWorkerGlobalScope environments #1074

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wanderview
Copy link
Member

@wanderview wanderview commented Feb 15, 2017

This pull request is broken up into two commits:

  1. First we rename the current 'service worker client' concept to just 'client'. I think this makes sense because the client concept is more general than just service workers and we would like to expose it to other places in the future. Also, I need the 'service worker client' name to represent my new client type.
  2. Add the service worker client type.

This PR addresses issue #1036.


Preview | Diff

This is necessary to allow us to use 'service worker client' to reference
Client objects that are actually backed by a ServiceWorkerGlobalScope
environment.
…ceWorkerGlobalScope. Fixes w3c#1036

This adds a "serviceworker" `ClientType` and the associated "service worker
client" object.  It also adds a check to avoid having `clients.matchAll()`
return a `Client` for the current environment.

A <dfn export id="dfn-service-worker-client" for="">service worker client</dfn> is a type of <a>environment</a> or <a>environment settings object</a>.
A <dfn export id="dfn-client" for="">client</dfn> is a type of <a>environment</a> or <a>environment settings object</a>.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question to @annevk in spec maintenance perspective: generally when we change definitions, changing dfn's id would be okay? (They may break existing links.) Or should we retain the ids even if they sound wrong after the change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd generally try to preserve IDs somehow since links that don't work are annoying. bikeshed has an oldids feature you could maybe use if you really wanted to change the ID.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oldids sounds good. @wanderview, please preserve the IDs. I'll at some point sort out the IDs that need changes using oldids.

@@ -1260,15 +1260,15 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
1. If the [=ServiceWorkerGlobalScope/service worker=] is not an <a>active worker</a>, return a <a>promise</a> rejected with an "{{InvalidStateError}}" exception.
1. Let |promise| be a new <a>promise</a>.
1. Run the following substeps <a>in parallel</a>:
1. For each [=/service worker client=] |client| whose [=service worker client/origin=] is the <a lt="same origin">same</a> as the [=ServiceWorkerGlobalScope/service worker=]'s [=environment settings object/origin=]:
1. For each [=/client=] |client| whose [=client/origin=] is the <a lt="same origin">same</a> as the [=ServiceWorkerGlobalScope/service worker=]'s [=environment settings object/origin=]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the service worker client itself should be excluded from claiming.

@@ -1210,7 +1215,13 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
1. If |isClientEnumerable| is true, then:
1. Let |windowClient| be the result of running <a>Create Window Client</a> algorithm with |client|, |visibilityState|, |focusState|, and |ancestorOrigins| as the arguments.
1. Add |windowClient| to |matchedClients|.
1. Else if |options|.{{ClientQueryOptions/type}} is {{ClientType/"worker"}} or {{ClientType/"all"}} and |client| is a <a>dedicated worker client</a>, or |options|.{{ClientQueryOptions/type}} is {{ClientType/"sharedworker"}} or {{ClientType/"all"}} and |client| is a <a>shared worker client</a>, then:
1. Else if |options|.{{ClientQueryOptions/type}} is {{ClientType/"worker"}} or {{ClientType/"all"}} and |client| is a <a>dedicated worker client</a>, then:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sub steps of the "else if" steps seem to be repeated here. How about we make the above window client "if" block continue to the next iteration after its own substeps. And do the following for worker blocks:

Add the result of running Create Client algorithm with |client| to |matchedClients| if any of the following are true:

  • |options|.{{ClientQueryOptions/type}} is {{ClientType/"worker"}} or {{ClientType/"all"}} and |client| is a dedicated worker client
  • |options|.{{ClientQueryOptions/type}} is {{ClientType/"sharedworker"}} or {{ClientType/"all"}} and |client| is a shared worker client
  • |options|.{{ClientQueryOptions/type}} is {{ClientType/"serviceworker"}} or {{ClientType/"all"}} and |client| is a service worker client

@jungkees
Copy link
Collaborator

Additionally, I think we need to sort out how client.reserved works for service worker clients. reserved becomes false when client's execution ready flag is set. IMO, the flag should be set during the first service worker run in Run Service Worker algorithm. If you need my help here, let me know.

@ylafon
Copy link
Member

ylafon commented Feb 16, 2017

Marked as non-substantive for IPR from ash-nazg.

<section dfn-for="service worker client">
<h3 id="service-worker-client-concept">Service Worker Client</h3>
<section dfn-for="client">
<h3 id="client-concept">Client</h3>
Copy link
Collaborator

@jungkees jungkees Feb 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preserve the old id here.


A {{Client}} object has an associated <dfn for="Client">reserved state</dfn>, which is either true or false.

A {{WindowClient}} object has an associated <dfn id="dfn-service-worker-client-visibilitystate">visibility state</dfn>, which is one of {{Document/visibilityState}} attribute value.
A {{WindowClient}} object has an associated <dfn id="dfn-client-visibilitystate">visibility state</dfn>, which is one of {{Document/visibilityState}} attribute value.
Copy link
Collaborator

@jungkees jungkees Feb 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preserve the old id here.


A {{WindowClient}} object has an associated <dfn id="dfn-service-worker-client-focusstate">focus state</dfn>, which is either true or false (initially false).
A {{WindowClient}} object has an associated <dfn id="dfn-focusstate">focus state</dfn>, which is either true or false (initially false).
Copy link
Collaborator

@jungkees jungkees Feb 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preserve the old id here.

Base automatically changed from master to main February 4, 2021 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants