mkruisselbrink / navigator-connect

Apache License 2.0
25 stars 8 forks source link

Should this API be using MessagePort to represent connections #35

Closed mkruisselbrink closed 9 years ago

mkruisselbrink commented 9 years ago

There are various issues surrounding the use of MessagePort to represent a connection. Most of these are caused by MessagePort being transferable (but not copyable). A port has to exist in one and only one javascript context because of this. For short running (non persistent/stashed) connections this isn't a problem, but for long running connections this causes all kinds of confusion around APIs used to store and lookup existing connections.

One proposal around this is to always represent all connections with some other type that isn't a MessagePort but still has a postMessage method (and onmessage event to receive messages when not in a service worker), but since pretty much all problems around using MessagePort to represent a connection is around long running connections, I'm proposing to keep using MessagePort as the default representation of a connection, except StashedMessagePort (maybe with a different name?) will not derive from MessagePort anymore.

I think this solves pretty much all downsides to using MessagePort. There are no weird locking mechanisms or other complicated APIs needed to ensure only one worker can lookup a certain stashed port, the problem of where to deliver messages sent to stashed ports (#34) exists regardless of how a port is represented, and the API from regular windows/workers doesn't needlessly get to use some different type.

One benefit of using a custom type to represent any connection would be that things like the service URL and client origin could be transparently stored in this connection object, but I think think that's that great of a benefit. There will always be situations where service workers (either as client or service side of a connection) will want to store extra data with their connections to identify them, so I propose to have tag and data attributes similar to Notification (set at the time of persisting) that can be used to look up connections (tag) and store read-only data like service URL or client origin (data).

So this API would look something like this:

[Exposed=ServiceWorker]
interface StashedConnection {
  readonly attribute DOMString tag;
  readonly attribute any data;
  void postMessage(any data, optional sequence<Transferable> transfer);
};

partial interface ServiceWorkerGlobalScope {
  readonly attribute StashedConnectionCollection connections;
};

[Exposed=ServiceWorker]
interface StashedConnectionCollection : EventTarget {
  StashedConnection add(MessagePort port, optional StashedConnectionOptions options);
  promise<sequence<StashedConnection>> matchAll(optional StashedConnectionMatchOptions options);
  attribute EventHandler onmessage;
};

dictionary StashedConnectionOptions {
  DOMString tag;
  any data;
};

dictionary StashedConnectionMatchOptions {
  DOMString tag;
  // maybe a flag to indicate if connections for all workers at this origin should be returned
};
mkruisselbrink commented 9 years ago

39 takes this idea even further by completely eliminating usage of MessagePort. Also preliminary data shows that pretty much no websites use MessagePort as a communication channel, so I don't think there really was any value in trying to make this API use MessagePort directly. So closing this issue, since I think the new proposal is clearer/better.