-
Notifications
You must be signed in to change notification settings - Fork 39
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
PresentationReceiver: rename getConnection() and getConnections() #201
Comments
@avayvod and I talked to @jakearchibald and we came up with this proposition: interface PresentationReceiver {
Promise<> ready;
sequence<PresentationConnection> connections;
attribute EventHandler onconnectionavailable;
}; It made me wonder why we couldn't simply do: interface PresentationReceiver {
sequence<PresentationConnection> connections;
attribute EventHandler onconnectionavailable;
}; It seems that if we accept that function handshake(c) {
// ...
}
function onloadhandler() {
navigator.presentation.receiver.onconnectionavailable = handshake;
navigator.presentation.receiver.connections.forEach(handshake);
} WDYT? |
I don't have a strong opinion either way. But if we do have a synchronous .connections property, we need to define when exactly the connection which launched the page on the TV shows up there. More specifically, is .connections.length==1 when the page on the TV is first launched? @schien should probably comment on how much work it would be to make the initial connection synchronously available. |
The F2F meeting notes about the issue 19 that led to the current spec: http://www.w3.org/2015/05/19-webscreens-minutes.html#item12 The argument against having an array property was that the prototype can be changed by the web developers. It doesn't apply to Sequence though afaik. @sicking I think the timing doesn't really matter in the Mounir's example - if the connection is already initialized, it could be populated by the time the page accesses the sequence but if it's not, then the page will receive an event. I think the question now is about the easiness to support the simple use case - one presentation, one connection. We also need to keep in mind a case when the page wants to designate itself as a presentation, that's what |
Yes, a The timing matters because we need to define if something like the following should work: <html>
<head>
<script>
navigator.presentation.receiver.connections[0].send("I'm ready to rock");
</script>
<body>
... |
I think the best think the spec could say is that the code above has an undefined behaviour. Maybe the connection will be set, maybe not. The code is clearly wrong, assuming that Regarding synchronous-ity, my understanding is that the main process will notify the child process that a connection was created. Having a synchronous array with an event should make the design implementatable in that case. I'm not sure what situation would make the synchronous array not implementation. |
If the behavior of the example is undefined, then I'd rather stick to using an asynchronous promise-based API. |
The Promise-based API is to avoid race conditions when the page attempts to use the first connection before it's ready, or adds an event handler after the first connection is ready. In some sense there is a "stream" or queue of incoming connections provided asynchronously to the page, and it's crucial that the presentation be able to easily consume the first as well as subsequent elements. The desire to have a simple API for the common case of a single controller motivated the addition of the I actually like the following proposal, which is more parallel to how screen availability is handled (promise for initial state + event/attribute for updates):
The simple case is handled by calling |
I agree with @mfoltzgoogle regarding preferred API. I think the synchronous API adds more risk of races in the page. |
The reason why I started to think about another proposal than the initial one I made is because there are a few issues with it. We could solve these issues by specifying behaviour in the specification but I think it will still allow developers to shot themselves in the foot.
Also, I'm not really convinced that having an asynchronous <html>
<head>
<script>
navigator.presentation.receiver.connections.then(c => c[0].send("I'm ready to rock"););
</script>
<body>
... Which would be as wrong as the previous code from Jonas above. |
I don't think we need to couple the events "the presentation is loaded and ready" with "there's a connection made". navigator.presentation.receiver.then(function (receiver) {
if (!receiver.connections.empty) receiver.connections[0].send("I'm ready to rock."); So I don't think we should distinguish the first connection from the rest given that: Ideally we would have a way for the controller to specify if the presentation only supports one connection and then the presentation could use some simpler path but as of today, there's no way for the presentation or controller to require one persistent connection so it's up to the implementation. |
Having If we follow Anton's lead and not distinguish first connections from the others but still want to keep the one connection UC easy, what about having something as simple as: interface PresentationReceiver {
Promise<PresentationConnection> waitForNextConnection();
}; It would basically behave like a stream and will resolve the promise when a WDYT? |
@mounirlamouri Thanks for reminding me of #19 where this API was hashed out before. It looks like I was not a big fan of exposing the full sequence of connections to the presenting page (see #19 (comment)). From what I can tell, the arguments for exposing the sequence of connections were to allow the presenting page to implement a cap on the number of controllers, and possibly to simplify broadcast messaging. Either can be implemented without the sequence however. It means presentation has to manage its own Array of connections for the multi-controller case. As we are already under the assumption that the multi-controller case is uncommon, I am not too concerned. And it can be added later if needed. In the spirit of moving forward with the minimal viable API, let's go with the proposal in #201 (comment). We would need to nail down the following behaviors:
For #1 and #2 the presenting UA would maintain a queue of incoming connections and Promises and resolve them in order. For #3 we need to either specify a more complete connection handshake, or mandate buffering until the presentation is actually to receive messages. |
I'm not sure I understand the proposed behavior of Does each call to What happens if you have two parts of the page which both needs access to the connection. Does that mean that the second caller of I think we should think about what we're trying to archive with this API. My goals have been:
I think the API in #201 (comment) archives that. It means that for the common case, when an app only ever deals with a single connection, you just write
And for pages which supports multiple connections you'd do something like:
Yes, someone might try to support the single-connection usecase by writing
But there's really very little reason to do so since the correct code both looks more correct and is just as simple. We could even cover this case too by saying that the |
I believe that's the intent, yes.
Yes. Do you think that could be a common problem? Why wouldn't the page just pass the first connection it receives to its other parts? I believe that losing the single connection would be a problem and most of the presentation pages would like to allow users to reconnect, so having waitForConnection() return the new connection sounds useful (the page would call it again when the first connection changes to terminated state, for example).
I think the multiple connections case is far less common so I'm not sure why we should optimize the first iteration of the spec for that.
No, we haven't added such an API, so even if the page only cares about one connection, it can potentially get more connections but ignore them. The UA could then terminate the ignored connection after a timeout, for example. It might be harder to do though if we introduce the
I think having just one method accomplishes that better than three overlapping things. Having an event and two promises for one and many connections seems to be more likely to introduce races (e.g. event is fired for a new connection, the page calls WDYT? |
I don't know how common it will be to have multiple parts of a page which all need access to the list of connections. What I do know is that pages are often vastly more complex than we give them credit for. So I would not be surprised if it will happen more often than we think. For example I would expect people to write frameworks or libraries that take care of a bunch of the connection management. But that those frameworks/libraries won't cover all the cases that a page cares about, and so the page will also use the "raw" API we're defining here. I also know that keeping state known to the browser hidden from pages almost always ends up with developers eventually wanting access to that state. Similarly, defining "smart" APIs which try to hide details from developers more often than not just makes things harder for developers. So having an API which, after you've called
I thought reconnecting means that you're still using the same
I agree that we should definitely not optimize for multiple connections. But I do think that we should support it. If we should optimize for anything, it should be for supporting a single connection. This is actually why I don't like the current |
I meant a case when the user launches a presentation (e.g. a video) on the screen and then the controlling device loses Wi-Fi or runs out of power - the presentation should continue playing or maybe pause and wait for the user to connect from the rebooted or even another device. In this case the first PresentationConnection would definitely go to the terminated state and the page would have to wait for another one. So using only one PresentationConnection at a time doesn't mean using the one same connection during the lifetime of the presentation.
I would expect the libraries to give access to the existing connection so the page doesn't have to use the API for that esp. if the spec will say the same connection can never be obtained again via the API. I'd say that compared to the above problem of reconnecting, the problem of libraries doing the wrong thing is less important.
I don't think it's that far: the only difference is how it would behave if called twice - return the same connection or wait for the other one. Maybe we should spec that it will return the same connection until it's terminated and then it will return the next one that's not? Then we might have two methods to also support multiple connections:
I definitely agree but it depends on the algorithms defined in the spec, too, doesn't it? If the behavior of "smart" methods is well defined, it's no longer magic, it's just convenient. |
It seems that it's very easy to shot yourself in the foot in various ways with Jonas previously argued that websites might try to get the first connection at page load. I'm not sure how much we should care about that given that it would obviously be an invalid assumption to think that there is a connection available at page load. We could also propose a library simplifying the API for very basic use cases like a one connection with the receiver stopping the presentation when the connection is closed. As Anton pointed, we can't expect that to be the most common use case but we can help people trying to do exactly that by providing them the right framework in user space. At that point, I feel that we will not be able to find an API that will ideally match all possibles UC so I think we should fin an API that, used correctly, will work for all these UC even if it might require more work for some of them. |
This is a good point. Though for the case when the connection quickly transitions to But I agree with your conclusion that even in the "single controller" use case, we'll need to worry about having the API juggle multiple connections. So I think what we're looking for is an API which is focused on letting the page enumerate the set of controllers and be notified when this changes. Here's what I propose: interface PresentationReceiver {
readonly attribute Promise<PresentationConnectionList> connections;
};
interface PresentationConnectionList : EventTarget {
readonly attribute sequence<PresentationConnection> connections;
attribute EventHandler onconnectionavailable;
}; This is essentially what @mounirlamouri is suggesting. It only adds one async step which would allow the implementation to easily wait with returning the list for the first time until it has populated it with the initial connection. Or we could make it even simpler and merge partial interface Presentation {
readonly attribute Promise<PresentationConnectionList> connections;
};
interface PresentationConnectionList : EventTarget {
readonly attribute sequence<PresentationConnection> connections;
attribute EventHandler onconnectionavailable;
}; |
Your Mark and Anton, WDYT? |
I see two scenarios here:
If yes, I'm happy with having So either we have: navigator.presentation.receiver.connections.then(function(connectionsList) {
for (c : connectionsList.connections) c.send("Ready to rock!");
}; or navigator.presentation.receiver.then(function(connectionsList) {
for (c : connectionsList.connections) c.send("Ready to rock!");
}; |
Mounir, in the latter case, I believe we could do the feature detection you want and merge |
I'm not entirely sure what you meant but I think that |
FWIW, I don't think that |
In addition of the confusion, I don't understand why a long name would be a problem, it's not like there was any downside in doing |
My experience is that authors strongly prefer short names. It's one of the main feedbacks that we got on IndexedDB for example. But that's definitely also in part because names are easy to bikeshed. So I definitely don't care strongly. If |
Sorry for reviving this thread but while I was thinking about the implementation, I realised that we might need to have a better definition of whan the |connections| are expected to be. Should we keep disconnected/terminated connections in there? or should the list only reflects currently available connections? I would prefer the latter but in that case, we might want to rename the |connectionavailable| event to something like |change| because there might be also disconnections. Arguably, we can have an event for new connections and one for disconnections. I wouldn't mind either. WDYT? |
I definitely think that we should remove "terminated" connections. "disconnected" connections can transition back to "connected", right? If the other side calls If so, it might make sense to keep them in The reason I thought that we had a |
I agree with the following:
|
Ok. Sounds good. Will write the spec changes. |
Update PresentationReceiver spec to match the results from the discussions in #201
I think this is now fixed. |
CC @avayvod @mfoltzgoogle @sicking
I have looked at the receiver side of the API fairly rarely and every time I am to remember that getConnection() is actually waiting for a connection while getConnection**s** return the list of active connections. I would suggest to rename them to:
I'm dubious about
waitForConnection
because it's basically the same as theconnectionavailable
event or did I miss the subtlety here?The text was updated successfully, but these errors were encountered: