-
Notifications
You must be signed in to change notification settings - Fork 311
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
Making sense of registrations #365
Comments
This is a good split, actually matches our impl more closely. I don't think this needs to be async for the current document, so a simple The Would we also need an |
I think this will simplify the implementation in Blink. Does this mean that a page can enumerate the registrations for the entire origin, or just registrations that could match the page's URL? .installing, waiting, active can be synchronous; making them Promise would not significantly simplify things. re: installing, waiting, active "updating live" I think you need to decide how many registration objects there can be for a given scope at one time. Do successive calls to register with the same scope create new registrations (and resolve to a different object) or update existing ones? In the former case you complicate getRegistration (which one should it pick while there's a new registration installing) but could simplify ServiceWorkerRegistration to have a single 'worker' property and .state can infer about as much as separate waiting, etc. properties. I think it is necessary to clarify whether getRegistration takes a scope or a URL. I assume it takes a scope and matches exactly because otherwise what is the point? |
I just wanted to point out that #357 is also confused by the single/multiple registration confusion. Separately, thinking about whether getRegistration takes a scope or a URL, I think either could be valid depending on the use case. So maybe there are actually two APIs here: For a ServiceWorker, get its registration (would use scope.) For this page, get the best matching registration (basically [[MatchScope]]. Would use a URL.) The classic navigator.serviceWorker.installing, waiting, active properties were implicitly using this. Are there higher-level use cases that you can look to for guidance about which APIs are needed? |
Agreed on the suggested direction. The only concern was whether the concept, registration, would be useful to authors not adding unnecessary complexity. However, I cannot disagree that exposing this concept explains the actual model better and reflects underlying implementation better. (@nikhilm Is it true with gecko implementation too?)
I think we should update existing ones. Once an entry was added to [[ScopeToRegistrationMap]], that registration object should be uniquely maintained until it gets deleted by [[Unregister]]. For the decision of the getRegistration() semantic, I agree we should figure out what the major use cases are. It seems it's a question of whether we'll expose [[MatchScope]] or [[GeRegistration]] to authors. Basically, getRegistration() without any argument falls on [[MatchScope]] case (implicitly with the document's URL) as it returns the registration that the document will use when navigated. If we expose getRegistration() method, this sort of usage can be dealt with for example: // Push registration with the document's default registration
navigator.serviceWorker.ready.then(() => {
navigator.push.register(/*...*/).then((pushReg) => {
});
});
// Push registration with a particular SW registration
navigator.serviceWorker.getRegistration("https://example.com/cocacola").then((registration) => {
return navigator.push.register(/*...,*/ registration).then((pushReg) => {
});
}); |
@jungkees here's a crazy idea I haven't properly thought through… what if ServiceWorker-dependant APIs such as navigator.serviceWorker.ready.then(function(reg) {
return reg.push.register();
}); Push & background sync are strongly linked to the ServiceWorker registration already. If you unregister SW, you're also unregistering push & sync. |
Is that true even if the page isn't using that registration for fetches? Eg: shift+reload, or a fresh registration before refreshing the page.
Yeah, the intent is document url. Although if we get rid of the |
Entire origin
Definitely update, even if the script URL is different. The scope is the unique part of the registration. If you call |
Here's the usecases I have for the various properties:
Basically, |
I see, overwhelmingly pages don't have a need to look at .registration. In light of that, it makes more sense to me to not have a special case sync .registration attribute and instead rely on the more general purpose async .getRegistration(docUrl) method when needed. |
I'm in favour of this change for the most part. More comments following
@michael-nordman a document's associated registration never changes, so onregistrationchange not required |
Does this change allow us to get rid of states (parsed, installing...) and onstatechange? Or is that still useful? |
I don't think allowing a page access to registrations via getRegistrations() not in it's scope is a good idea. Previously this was only allowed through register(), and the intent was you could postMessage with the worker. register() still allows doing that. On the other hand, allowing broad access is:
We should look long and hard at both the argument form of getRegistration() and getAllRegistrations() before adding them to the specification. |
@nikhilm, not sure thats true in the case of a brand new registration and existing pages that would be controlled by that upon reload. With the existing api, the existing pages would see a 'onupdatefound' event and become aware of the newly installing thing. I'm assuming window.serviceWorker.registration would be NULL in the precondition case. |
Yes, but it doesn't really matter. i.e. its just a matter of doing some different typing, so I'd be happy with either :)
Agreed about [[ScopeToRegistrationMap]] entries remaining constant between register() and unregister(). It simplifies a lot of reasoning in the implementation. |
I think |
Thanks for the commentary, particularly about use cases, and clarifications. This is useful. In general what's being proposed on this thread is implementable in Blink. Given that cross-scope communication is possible haphazardly with the existing spec, formalizing that as a simple, explicit API seems like a good rationalization. There seem to be some redundant properties now: ServiceWorker.scope vs ServiceWorkerRegistration.scope. Also, it is unclear to me how scriptURL is a property of a ServiceWorkerRegistration and not a ServiceWorker; which Service Worker's script URL does it refer to? |
Unless otherwise ServiceWorer.scope is a useful sugar for authors, I agree to drop this from ServiceWorker. Registration maps worker to scope, so it naturally belongs to registration.
In the existing spec algorithms, registration.scriptURL holds the url of the currently installing script. That is, even before the instance of installing worker is created, the algorithm used this url to fetch the target worker script. But now we want to expose the registration object to JS, I don't think this property makes much sense to authors. We can drop this from ServiceWorkerRegistration and I'll revisit the algorithms as such. |
Great idea. Those APIs can define its API space within ServiceWorkerRegistration as for example (SysApps WG's Task Scheduler API):
In the script, it can be used as long as there's a SW registration: navigator.serviceWorker.ready.then(registration => {
return registration.taskScheduler.add(Date.now() + (10 * 60000), {
message: "It's been 10 minutes, your soup is ready!"
});
}).then(onTaskAdded, onError); I'm not sure whether exposing getRegistration(url), which @nikhilm opposed to, would add some meaningful value to authors here. |
The registration has a script url that it uses for updates https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#update-algorithm. You can change the url or the registration by calling
Yeah, that makes sense. That removes confusion around
It can if the installing worker calls |
I'm probably being naive, but I can't see how Input: docURL
It doesn't use anything we don't use elsewhere, it's pretty much an extremely cut-down version of register. What's more, we're not offering anything you couldn't do in a completely crazy way with an iframe & Use-cases:
Yeah, but they can also register 1000 new scopes for an origin, which involves fetching 1000 scripts & spawning them. We should make sure we punish the origin for doing this, rather than slowing down the whole browser. |
Agreed. |
…, removing .scope from ServiceWorker objects. Part of #365
@jakearchibald I don't think scriptURL needs to be exposed to authors on ServiceWorkerRegistration. There it is simply an intermediate needed by the algorithms. |
I concede on #365 (comment) :) |
Sounds good :) @jakearchibald already changed the ts file. I'm having a vacation for a week until the 21st of JUL. I'll change the web spec accordingly next week. |
Happy to drop |
Addressed c3c3d09. |
I'm not entirely convinced exposing push on top of ServiceWorkers is a good idea. If a developer is just trying to employ window-independent network listening, then having him/her use ServiceWorkers directly seems like unnecessary development overhead. There are also some wonky constraints that ServiceWorkers implicitly impose in such a model. For example, the current Push spec only allows one registration per origin, however multiple ServiceWorkers are allowed per origin. This problem isn't necessarily unsolvable, but it is indicative of problems that services that wish to use ServiceWorkers in a similar way could have. |
I don't think I understood what "window-independent network listening" suggests. There's been no other mechanism in browser to fire/execute functional events like "push" without a browsing context before.
Defining API space in |
The spec hasn't been updated. At the moment it allows multiple registrations per origin, but it's supposed to be one registration per serviceworker. Multiple registrations per serviceworker don't make sense (make distinctions with data), and registrations without a serviceworker don't make sense (no where to fire the event). Push and background sync already had a serviceworker dependency. Moving the APIs there makes this clear. |
First of all, I wanted to introduce myself. I'm Nikhil's new intern, and I'm a bit new to these concepts, but I've been put in charge implementing the new Push API so this is all directly relevant to me. I plead for patience if I misunderstood something.
What I mean by that is that ServiceWorkers seems to me to have have its feet in two pools (for this situation in particular). You've got developers that are using them to build offline applications, and developers that are employing them to listen to server events without directly requiring the page Javascript to be executed. The thing is that there's not really all that much overlap between these two use cases. Why would developers that just want window-independent listening care about all the awesome things ServiceWorkers can do? Now, I understand that the "window independence", as I'm calling it here, is sort of a defining part of ServiceWorkers, so I can see why placing services which require window independence on ServiceWorkers is an elegant solution. However, I'm not sure it's better than just exposing a dump API to developers and implementing ServiceWorkers in manner that is transparent to the developer.
The spec hasn't been updated, and that is certainly irresponsible, but we've been developing under the one registration per origin assumption, as well as implementing a new browser-facing API that is exposed directly on navigator. Of course I'm willing to move along the track @jakearchibald suggested, I'd just like to be convinced that it's necessary first. |
Oh, I thought it was agreed that push was linked to ServiceWorker. Without that link, which ServiceWorker gets the push event, or does it go elsewhere? |
@Ndvr that assumption is wrong. Any reason why that assumption was made? You can have multiple service workers per origin... |
Hi @Ndvr, look forward to working with you on Push!
SWs are often introduced as being about offline, as that was the original motivation (and is included in the same spec). But it's more subtle than that. The core functionality of SWs is purely to be able to receive events from the browser at any time, whether or not the web app is currently open in one or more tabs. On top of this foundation, several features are being built:
These features should be treated as independent (though they happen to work well in combination), but they all rely on the ability of SWs to receive events without needing a tab to be open. This variety of use cases is why Service Workers were renamed from their old name "Navigation Controllers".
Oh, we've always been implementing one registration per SW; I thought Nikhil/Doug were on board with that?
Our prototype also did this, but we'd like to change it to be on ServiceWorkerRegistration as discussed on public-webapps. See also w3c/push-api#2 and WICG/background-sync#6. |
Okay, I'd like to apologize, I was making a silly mistake. ############# Disclaimer: Irrelevant ################ Coming at this from the push side of things, I was thinking it was ultimately unnecessary to expose ServiceWorkers to developers that were implementing push. All that is happening from a push developer's perspective is creating a registration and writing a request handler for when messages arrive, and that isn't overly complicated. In my mind, we fundamentally had this functionality for receiving events from the browser at any time, and we might as well make utilizing this as uncomplicated as possible for web devs. So I was thinking we could have a ServiceWorker in the basement, so to speak, and just interact with it via a navigator.push. navigator.push.onMessage or whatever would basically just define the onpush behavior for the ServiceWorker (it seemed weird that push.onMessage may not have access to the DOM if the page context isn't around, but if I understand correctly the web dev would have had to deal with that anyway within worker.js). Push.register would be some business logic that would essentially translate the registration request into a service worker registration, which could allow us to keep everything kosher with regards to origins. Also, it seems like we could potentially do this in a way that would avoid having to reload the page before we started receiving push events. I'm not sure if the engineering is sound on that, but it doesn't really matter. I did some thinking and even if it does simplify push in particular it seems a bit unclean to hide the ServiceWorker dependency when there are so many non-push services that could potentially employ ServiceWorkers in a similar way. ################ End Irrelevance ################ Bottom line is that I'm sorry for wasting everyone's time, and in light of my new understanding @jakearchibald 's proposal seems reasonable and elegant. |
Ah, that wouldn't have worked, because event handlers are JavaScript functions, which get destroyed when the execution context (the page) is closed. The reason it works in a Service Worker, is that -- by convention -- the SW will add event listeners for the same events each time it is run; so e.g. when the browser receives a push message and the SW isn't yet running, it will first run the SW script, which should set an event listener for the push event, then and only then the browser will be able to dispatch a push event to the SW (and if the SW misbehaves and fails to register a push event, then I guess the browser has to discard the event, or log an error to devtools).
We already can! Note that the definition of |
Okay, both of those explanations made sense. Thanks @johnmellor ! |
Blink work tracked in crbug.com/396400 (SWRegistration: done), crbug.com/399533, crbug.com/404951 ... |
At the moment
navigator.serviceWorker
doesn't really expose the concept of registrations. I think this is a mistake. It causes these problems:.register
resolves with a serviceworker that may be installing/waiting/active, this doesn't feel like the right return value for a registration.getAll
returns an array of workers in various states (getAll() resolution array #170).active
vs.controller
- one is part of the registration (along with.installing
and.waiting
) and the other is part of the document's use of a registration, but that's not really clear.onupdatefound
- this is to do with the registration that matches the document is within scope of..oncontrollerchange
is to do with the document's selection of a registrationHow about we offer up registration objects:
Which means we can tidy up
navigator.serviceWorker
:Now we don't have sync access to registration state, other than
.controller
, which is the document's use of the registration. Instead of.getAll
we have.getAllRegistrations
. Instead of installing/waiting/activate we have.getRegistration
, which can also be used to get a registration (if there is any) for a particular url.If you get a registration object, is it possible (or sensible) to have the installing/waiting/activate properties continue to update live, or is that breaking everything?
The text was updated successfully, but these errors were encountered: