Re: [webrtc-pc] transceiver.stop() shouldn't block in-progress replaceTrack from succeeding (#2106)

> > RTCRtpSender.setParameters and RTCRtpSender.getStats are not checking for stopped/closed before resolving/rejecting the promise.
> 
> _getStats_ is specifically allowed on closed peer connections.

Right, so I would not assume anybody to write code in a way that handle the case of some WebRTC methods returning promises that will never reject/resolve.

> _setParameters_ is not enqueued, and is not allowed to fail with `InvalidStateError`, so I'd assume it succeeds, which seems benign enough.

So, we are asking web developers to know that setParameters is not enqueuing and replaceTrack is enqueuing, so that they properly handle the fact that a promise may never be settled in some cases for replaceTrack?

> Consistency with the other enqueued methods. Having `transceiver.stop()` cause outstanding _replaceTrack_ promises to reject with `InvalidStateError` would be novel. I'm not hearing support for that.

I expect more consistency between the methods of a given class than between methods of different classes.

As I said, I am fine with either resolve or reject.
Rejecting would be novel but web apps need to handle the InvalidStateError rejection anyway so this might not be a big deal in practice.

Resolving might indeed be less risky.
The additional bonus of resolving are that it reduces the code to write and the time to read the spec.

> I think we need to ask: Why would an app want renegotiation errors after having called `pc.close()`?

If you look at streams API which heavily relies on promises, why would an app want to get read promise errors after having called stream.abort?
An example might be that one part of the code of an app might do read.
Another part of the code might do abort but does not know about the one waiting to read or might forget to notify it.

For WebRTC, one purpose might be clean-up.
replaceTrack promise.finally might be used to know when to pause the audio context fueling an outgoing audio track, something like ref counting on the use of the audio context to decide whether to pause/resume it.
For pc, a repeating timer could be created during renegotiation and cleared at end of negotiation.
Everything would be fine until one day the app is updated to call pc.close to follow good patterns, or fix some leak issues. Now, the repeating timer might in some code paths never be cleared...


-- 
GitHub Notification of comment by youennf
Please view or discuss this issue at https://github.com/w3c/webrtc-pc/issues/2106#issuecomment-467298650 using your GitHub account

Received on Tuesday, 26 February 2019 05:16:51 UTC