Relax terminate algorithm to deal with other states?

[tidoust]

The terminate method only has an effect on a presentation connection whose state is connected.

I think a common scenario for calling terminate is when the controlling browsing context is about to disappear or when the user clicks on a button, to clean things up, regardless of the current application state.

This is what we try to do in the test suite for instance, so that subsequent tests start from a clean "no on-going presentation" state. However, since terminate only works on connected presentations, terminating a presentation no matter what turns out to be more complex that simply calling terminate, leading to code such as:

// Try to terminate the presentation
connection.terminate();

// If state was "connecting", previous call did nothing,
// wait for the "connect" event
connection.onconnect = () => { connection.terminate(); };

// If state was "closed", we need to reconnect first. This will return a
// presentation in "connecting" state. Previous event handler will eventually
// take care of it if it's the same "connection", but it may not be.
if (connection.state === 'closed') {
  request.reconnect(connection.id).then(c => {
    c.onconnect = () => { c.terminate(); };
  });
}

I'm wondering whether the user agent could take care of some of that complexity, ideally so that developers can end up with a simple connection.terminate() call. This would require relaxing the terminate algorithm so that:

1. It also applies to connections in the connecting state. The user agent would wait for the underlying connection to become connected.
2. It also applies to connections in the closed state. The user agent would attempt to reconnect the underlying connection before it terminates it.

The latter change may be more problematic, and it seems reasonable to expect applications to deal with connections in the closed state themselves. The former change seems easier and useful. If both changes are in place, developers can just issue a call to connection.terminate() and be done with it. If only the former change is in place, developers would have to write something like:

if (connection.state === 'closed') {
  request.reconnect(connection.id).then(c => { c.terminate(); });
}
else {
  connection.terminate();
}

... which seems acceptable.

[markafoltz]

If I'm not opposed to this change, but would need to investigate the implications for our implementation to determine the scope and complexity. It does seem to improve ergonomics for the cleanup case, but isn't the expected use case to simply allow the connection to be closed by the controlling UA on page unload? The presentation can close itself when it detects the connection has goneAway.

I went ahead and thought through the implications for the API and what we signal to developers.

When script calls terminate() on a connected presentation, there's an assumption that it will succeed by default, since the presentation is known to exist and the controlling user agent has some way of communicating with it. If we allow terminate() to be called in any state, then all bets are off. Although terminate() is more or less best effort anyway, this seems to change the nature of the API.

I ask the following:

• If the presentation is known to no longer exist (i.e. its state is terminated) should we allow terminate to be called or throw an exception?

• For the closed state, should we make a best effort and report back success or failure if controller believes the presentation exists and the signal was able to be sent to the receiver? Perhaps: Promise<void> terminate()

For connecting or closed, I don't think there is a need to transition to connected then terminated, since there is no need to message a presentation that is about to be torn down.

The algorithm changes could be tricky given the fact that terminate() can be called at any time.

[markafoltz]

I think we should split this into two issues.

The first is to allow terminate() to apply to connections in the connecting state which is a smaller spec change, but possibly a tricky implementation change. I am okay with this blocking CR.

The second is to allow termination of closed connections. This is a more fundamental rethink of the API and should be a v2 feature request.

[markafoltz]

PR allows termination in a connecting state. It does not change the API signature for terminate().

Maybe an implementation note clarifying that termination is best effort would be of use? I suspect that given the implementation and network dependence, that will always be the case for many operations in the API though.

[tidoust]

Maybe an implementation note clarifying that termination is best effort would be of use? I suspect that given the implementation and network dependence, that will always be the case for many operations in the API though.

Right, close() seems best effort as well when the algorithm says "Start to signal to the destination browsing context the intention to close the corresponding PresentationConnection", for instance. If you insert a note, it should be more generic, but I'm not sure where to put it in the current spec in practice.

[markafoltz]

PR merged; I don't plan on addressing the additional note or termination via closed connections at this time. I think this can be closed then.

Feel free to file termination via closed connections as a feature request :)

[anssiko]

@tidoust, if you don't feel strongly about this, I'd give you an action to open a new issue for the feature request in question and close this issue when done.

[tidoust]

@anssiko @mfoltzgoogle

I'll close this issue without creating another one for termination via closed connections. This issue captures the possible gain that this could bring in theory and the rationale for not doing it in practice. We can always refer back to this issue later on if the idea comes back onto the table.