-
Notifications
You must be signed in to change notification settings - Fork 13
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 State to Service Worker Clients. #37
Conversation
@jakearchibald as discussed offline. |
spec.bs
Outdated
}; | ||
</pre> | ||
|
||
A {{Client}} object has an associated <dfn id="dfn-service-worker-client-state" for="Client">state</dfn>, which is one of {{ClientState}} attribute value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing ever sets this value. That seems bad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I skipped adjusting the Create Client algorithms where state is passed in.. I've now added them so it should be clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is clearer. The remaining thing is to add two subsections of "Algorithms" that explains how you modify "Create Window Client" and "Create Client" to take the new argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
So the remaining big issue is workers. This PR operates on all clients, but "Get Client Lifecycle State" consults the responsible document, which only exists for window environment settings objects. (See whatwg/html#4335 and in particular whatwg/html#4335 (comment).) Maybe this is as simple as using https://wicg.github.io/page-lifecycle/#dedicatedworkerglobalscope-owning-document for dedicated workers. And I think clients can only be dedicated workers or windows, right? So that'd work? |
As discussed on w3c/ServiceWorker#1442 Unfortunately adoption of page-lifecycle is yet to be formally supported by other vendors so we need to monkey patch this in the page lifecycle spec.
- Rename state to lifecycleState
- Remove "Service Worker:" prefix. We should remove the "HTML:" prefix for HTML modifications in a separate commit. - Match service worker section titles - Use Markdown-style headings instead of mixing in HTML and Markdown - Service worker client internal slots are like "lifecycle state" not "lifecycleState" - Fix section headings for matchAll/openWindow; those are methods, not algorithms - Nest sections properly (e.g. Clients stuff goes under Clients)
They were used but not turned on, so they were appearing raw.
- Change definition of frozen to using owning document
I've adjusted it back to what I wrote before Jake asked me to change it to be the resposible document. (See w3c/ServiceWorker@a5bc071#r303010890) I prefered using owning document anyways. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the patience!
As discussed on w3c/ServiceWorker#1442
Unfortunately adoption of page-lifecycle is yet to be formally
supported by other vendors so we need to monkey patch this in the
page lifecycle spec.