w3c / ServiceWorker

Service Workers
https://w3c.github.io/ServiceWorker/
Other
3.63k stars 312 forks source link

Clients & postMessage #609

Closed jakearchibald closed 9 years ago

jakearchibald commented 9 years ago

A client can message a particular ServiceWorker via:

navigator.serviceWorker.controller.postMessage("Hello");

I guess messageEvent.source would be a client, but where client.postMessage goes isn't properly defined. This is also a problem for clients created by clients.getAll.

In my ignorance, I thought client.postMessage would go to global.onmessage, but SharedWorker doesn't have that, and I hadn't really appreciated that window.onmessage was only for window-to-window communication.

So yeah, if a ServiceWorker sends a client a message, where does it land?

We could have navigator.serviceWorker.onmessage as the target for messages from a SW to this specific client, where messageEvent.source is an instance of ServiceWorker, allowing state inspection & postMessage back.

jakearchibald commented 9 years ago

@mvano @johnmellor @beverloo @mounirlamouri - in case you care from a push point of view

annevk commented 9 years ago

I think on both sides we want the postMessage() method and onmessage event handler to reside on the same object. That symmetry has been maintained for all message channel solutions to date.

That puts into question whether Client should be a snapshot object at all.

jakearchibald commented 9 years ago

I don't think the snapshot nature of Client is an issue here. Windows & workers have no access to the client object, so they can't listen to its message events.

The client is a static representation of the window/sharedworker/dedicatedworker. Client exists because WindowProxy can't be used for obvious reasons, that's why I thought window.onmessage made sense as a message destination.

Now that getAll() only returns window clients by default, we could have getAll({type: 'all'}) return a mix of WindowClient, DedicatedWorker & SharedWorker. WindowClient would postMessage to window.onmessage, because it's a worker-safe WindowProxy.

annevk commented 9 years ago

The problem is that if Client is the place from which you message towards client, it should also be the place from which you receive messages from the client. Similar to ServiceWorkerRegistration being the proposed place to send messages towards the service worker and receiving messages from the service worker.

(Having getAll() not actually return all by default is confusing. Name it get() then.)

annevk commented 9 years ago

Idea: we provide no built-in messaging API and require usage of BroadcastChannel. (We might still provide built-in messaging for individual fetches at a later point.)

jakearchibald commented 9 years ago

Hah, I had the same thought. Does anyone implement BroadcastChannel yet?

Even with BroadcastChannel, there's going to be asymmetry in messaging. You can use it to broadcast to multiple windows/workers, but due to the way ServiceWorker terminates when not in use, you'll need to use something like navigator.serviceWorker.controller.postMessage to reliably talk back.

I take it you're dead against WindowClient (as a worker-friendly WindowProxy) posting to window.onmessage?

annevk commented 9 years ago

Firefox implements it.

If there's some form of buffering it could work for service workers I think although that might be somewhat tricky. (Though that needs to be solved either way.)

I'm not sure what you mean by WindowProxy in this context. I don't really see how WindowClient can work if we keep it static. And posting to the Window object seems odd.

jakearchibald commented 9 years ago

I'm not sure what you mean by WindowProxy in this context.

I see WindowClient as a worker-safe version of WindowProxy. It's a worker's connection to the window. That's why I thought posting to window.onmessage was ok.

jakearchibald commented 9 years ago

@annevk can you sum up why having windowClient.postMessage proxying to the associated Window's .postMessage is a bad idea?

annevk commented 9 years ago
jakearchibald commented 9 years ago

I disagree that the postMessage model we have with Window is confusing compared to what you end up doing with SharedWorker. I think "requires all existing library code to be rewritten" is an exaggeration, but messageEvent.source being not-a-Window is an issue.

Will arrange a call to go through this.

annevk commented 9 years ago

I think it would make more sense for someone to sit down with @Hixie and sort this through.

jakearchibald commented 9 years ago

@Hixie some background:

A page can postMessage to a ServiceWorker via:

navigator.serviceWorker.controller.postMessage("Hello");

navigator.serviceWorker.controller is the ServiceWorker instance controlling the page (and likely other pages/Workers/SharedWorkers). Posted messages land in self.onmessage in the ServiceWorker's global scope.

In the ServiceWorker, self.clients gives the ServiceWorker insight into clients on the origin, where clients are Windows, Workers and SharedWorkers. The current API allows:

clients.matchAll().then(clients => {
  clients[0].postMessage("Hello");
});

…as a method for the ServiceWorker to communicate with a client. This is where there's disagreement. My initial thinking was, for windows, client.postMessage would land in window.onmessage, because a window client should behave like WindowProxy but be safe to use in a worker. @annevk disagrees (see https://github.com/slightlyoff/ServiceWorker/issues/609#issuecomment-71622477). Furthermore, there isn't an onmessage in SharedWorker's global, so it doesn't really make sense.

Alternatively, client.postMessage could land on navigator.serviceWorker.onmessage in the client's global. This works for windows & workers, although you wouldn't have a symmetrical navigator.serviceWorker.postMessage as there's nowhere for that to go. I'm not particularly bothered by this. Responses would be sent through messageEvent.source.

Other possibilities

@annevk suggests dropping postMessage and instead relying on BroadcastChannel which Mozilla recently implemented. I'm fond of BroadcastChannel, but it doesn't allow communication with a specific client.

There's also the idea of stashing message ports within ServiceWorker registrations https://gist.github.com/mkruisselbrink/536632fcd99d45005064 - but this comes with all the GC observation of PortCollection, along with potential leaks if both ports are stashed. Plus it's pretty complicated compared to client.postMessage.

jakearchibald commented 9 years ago

We're still stuck here. In case it helps, here are the requirements:

Use cases:

annevk commented 9 years ago

A client (C) is associated with zero or more ServiceWorker objects. A service worker (SW) is associated with zero or more Client objects. These are the entities that need to be able to communicate with each other.

I think we need to reconsider making these objects live (and therefore unique per global). When live message listeners could be registered at SW and C creation time and things would work. (This would mean SW is not always woken up by an event dispatched on the global.)

Making them live would require some kind of synchronization event when a new C or SW presents itself to the other side. It would also mean that any state on Client and ServiceWorker objects should probably be behind promises if it can easily change.

User agents can still optimize by creating these collections lazily when they are accessed (or assumed to be accessed based on experience running the code).

mfalken commented 9 years ago

Blink has already shipped Client.postMessage. We plan to add a console message warning the developer that the API may change.

jakearchibald commented 9 years ago

Update:

We're circling on a model where serviceWorkerInstance.postMessage lands at self.onmessage within the ServiceWorker. The messageEvent.source will be an instance of WindowClient or Client depending on the sender.

clientInstance.postMessage will land at navigator.serviceWorker.onmessage. The messageEvent.source will be an instance of ServiceWorker representing the sender.

This changes what Chrome has already shipped, but it gives us a sensible place to land messages in SharedWorkers, and doesn't overload window.onmessage.

I don't think this can be specced in terms of ports, as it isn't 1:1 (both onmessage destinations can receive messages from multiple sources). The spec for BroadcastChannel will be a good reference, as it does something similar, although less specific.

@jungkees do you have any time to have a go at specing this? I'm snowed under with talk-writing :(

inexorabletash commented 9 years ago

Just to inform implementation, do we have thoughts (to pass on to @Hixie) about updating the IDL definition for MessageEvent's source?

Currently it is (WindowProxy or MessagePort)?; Chrome's implementation has it as EventTarget? and we were planning to do something hacky until the HTML spec was clarified here.

(The spec for BroadcastChannel appears to leave source unset for messages.)

mfalken commented 9 years ago

Tracked at https://code.google.com/p/chromium/issues/detail?id=459413 for Blink.

annevk commented 9 years ago

It doesn't really sit well with me that there's a lack of consistency in how service workers are exposed to clients vs how clients are exposed to service workers.

An alternative I proposed was that we make them all live (rather than just one side), updates based on events, and let the message events bubble up to navigator.serviceWorker and self.clients to make usage a bit easier.

jakearchibald commented 9 years ago

Code like https://github.com/slightlyoff/ServiceWorker/issues/588#issuecomment-66985639 becomes unworkable if we start putting values behind promises.

and let the message events bubble up to navigator.serviceWorker

From where? SW instances aside from the one controlling the document are behind promises.

jakearchibald commented 9 years ago

@mounirlamouri Anne is suggesting the client objects should be 'live'. We can't put their state properties behind promises, else iterating over them to find which client to focus becomes unmanageable. Having client properties update live sounds like a performance issue to me, or is it not as bad as I think?

We could make the values update lazily, making only rough guarantees about accuracy.

Or we could stick with the snapshots and give clients an id for comparison purposes.

jakearchibald commented 9 years ago

Additionally, I think we should make postMessage return a promise which rejects on unsuccessful delivery. After a push message, a SW may find a focused & visible client and post a message to it, but that fails because the tab as since been closed. Developers should be able to catch that case and show a notification instead.

This also allows us to reject posts to "asleep" tabs. https://github.com/slightlyoff/ServiceWorker/issues/626

annevk commented 9 years ago

Code like #588 (comment) becomes unworkable if we start putting values behind promises.

I was suggesting we keep that live as well, updating based on tasks.

From where? SW instances aside from the one controlling the document are behind promises.

How does that work if they are live?

mounirlamouri commented 9 years ago

Making the clients live is going to have a lot of implications on the current design. .focus() would no longer return a new Client object because it wouldn't really make sense. Also, implementations will have to keep track of window objects that are clients and update the service workers that know about them - which, as you pointed - might be a performance issue.

Also, with Chrome shipping the current status of the specification, going backward might come with its share of compat issues.

annevk commented 9 years ago

Well, the problem is that currently there's not much design to speak of. E.g. elsewhere @jakearchibald argues how useful it is to === ServiceWorker instances, the same obviously goes for Client instances on the other side.

jakearchibald commented 9 years ago

@annevk @slightlyoff & I had a meeting last night about this stuff. The conclusions were:

The postMessage stuff will need to be specced more like BroadcastChannel than ports, since they're not 1:1.

jakearchibald commented 9 years ago

@inexorabletash

Just to inform implementation, do we have thoughts (to pass on to @Hixie) about updating the IDL definition for MessageEvent's source?

I think we will have to subclass or create our own event type for this.

annevk commented 9 years ago

I think the postMessage() stuff is 1:1 in a way, but the mechanics are different. There's no way to reach more than one service worker or client environment with this setup.

annevk commented 9 years ago

I think we will have to subclass or create our own event type for this.

Why not simply get that changed? If @Hixie doesn't deal with the extensibility he could even allow "any" there. Doesn't matter much.

catalinb commented 9 years ago

@jakearchibald

serviceWorkerInstance.postMessage lands at self.onmessage within the ServiceWorker. The >messageEvent.source will be an instance of WindowClient or Client depending on the sender

What about the case when the sender is not controlled by the service worker? Should the source still be an instance of WindowClient/Client? For example:

navigator.serviceWorker.register({some options}).then(function(registration) {
    registration.active.postMessage();
});

Assuming the registration has an active worker, the registering window can post messages to the worker without being controlled by it.

KenjiBaheux commented 9 years ago

@jakearchibald you mentioned that it's desirable to have postmessage return a promise in https://github.com/slightlyoff/ServiceWorker/issues/609#issuecomment-74920652

It's unclear to me if this is indeed part of the consensus reached in https://github.com/slightlyoff/ServiceWorker/issues/609#issuecomment-75739069

jungkees commented 9 years ago

What about the case when the sender is not controlled by the service worker? Should the source still be an instance of WindowClient/Client?

I think any instantiated ServiceWorker object can postMessage to the service worker represented by it regardless of the state of the service worker and the state it being controlled or not. registration.installing.postMessage() should also worker.

Yes, the event.source should be an instance of WindowClient/Client. Note that self.clients.matchAll({ includeUncontrolled: true }) resolves with uncontrolled client in the shape of WindowClient/Client as well.

jungkees commented 9 years ago

@jakearchibald In case of the uncontrolled clients gotten from self.clients.matchAll({ includeUncontrolled: true }),client.postMessage should still land in the service worker client's global.navigator.serviceWorker.onmessage as long as the ServiceWorkerContainer object exists. Are we on the same page?

annevk commented 9 years ago

@KenjiBaheux we did not discuss a return value for postMessage(). Wouldn't exposing one expose GC? Seems like that might be a bad idea potentially.

johnmellor commented 9 years ago

@annevk wrote:

@KenjiBaheux we did not discuss a return value for postMessage(). Wouldn't exposing one expose GC?

Not really, there are many reasons why the postMessage could fail, such as the user having closed the tab, or JS having navigated the tab to a non-same-origin URL.

It's #626 which would arguably expose evicted tabs (which I think is what you refer to by GC).

jakearchibald commented 9 years ago

I don't think this exposes GC, it's the closing of a window it exposes, and that seems fine to expose.

On Wed, 25 Feb 2015 11:02 John Mellor notifications@github.com wrote:

@annevk https://github.com/annevk wrote:

@KenjiBaheux https://github.com/KenjiBaheux we did not discuss a return value for postMessage(). Wouldn't exposing one expose GC?

Not really, there are many reasons why the postMessage could fail, such as the user having closed the tab, or JS having navigated the tab to a non-same-origin URL.

It's #626 https://github.com/slightlyoff/ServiceWorker/issues/626 which would arguably expose evicted tabs (which I think is what you refer to by GC).

— Reply to this email directly or view it on GitHub https://github.com/slightlyoff/ServiceWorker/issues/609#issuecomment-75942060 .

annevk commented 9 years ago

What if the user agent implements "fast back"?

jakearchibald commented 9 years ago

Good point. But as @johnmellor says, this doesn't give you the difference between a salvageable but unloaded document and a user-closed one, so I don't think it counts as GC exposure.

jakearchibald commented 9 years ago

@catalinb

What about the case when the sender is not controlled by the service worker? Should the source still be an instance of WindowClient/Client?

Yes. You can already get hold of these clients via clients.matchAll({ includeUncontrolled: true }) - oops I see @jungkees already said the same thing. I should read everything before responding.

jakearchibald commented 9 years ago

@jungkees

In case of the uncontrolled clients gotten from self.clients.matchAll({ includeUncontrolled: true }),client.postMessage should still land in the service worker client's global.navigator.serviceWorker.onmessage as long as the ServiceWorkerContainer object exists. Are we on the same page?

Yep! And global.navigator.serviceWorker is present in all clients' globals.

jungkees commented 9 years ago

Spec'ed ServiceWorker.postMessage() and Client.postMessage(): 6dfce0aacceeef41c813c4680c7f67b96f80c4b6. Just done it in Friday evening and may need to continue the work next week. ;-)

Notes:

ghost commented 9 years ago

I had a hard time following the decision making process to its conclusion here.

Will it be possible to get reference to, and message (unicast not broadcast) the client which originated the fetch event? It's my most important use case.

jungkees commented 9 years ago

Will it be possible to get reference to, and message (unicast not broadcast) the client which originated the fetch event? It's my most important use case.

Yes, you can do this:

self.addEventListener("fetch", function(e) {
  e.client.postMessage("To the client initiated this fetch."); 
  // It lands on the client's global object's navigator.serviceWorker.onmessage
});

And this messaging is a unicast.

jungkees commented 9 years ago

Closing.