Webrtc-pc: Don't fire events on a closed peer connection

[jan-ivar]

To avoid this, whenever we "Queue a task to run the following steps:", we immediately do:

• If connection's [[isClosed]] slot is true, abort these steps.

We've adopted this approach, but we're not consistent. I found at least a couple of places where we don't do this:

• The negotiationneeded event (we check [[needNegotiation]], but not [[isClosed]] [here](https://w3c.github.io/webrtc-pc/#dfn-update-the-negotiation-needed-flag)).
• Resolve promise in replaceTrack.

There may be more places.

[henbos]

Related: I found the RTCPeerConnection::close() steps a bit unclear. It's doing a bunch of things, some which may or may not imply firing events, before reaching "_8. Set connection's [[isClosed]] slot to true._"

E.g. "_4. Set the state of all RTCIceTransports to closed._" Do you fire an RTCIceTransport statechange event or not?

[jan-ivar]

"4. Set the state of all RTCIceTransports to closed." Do you fire an RTCIceTransport statechange event or not?

@henbos Event firing is explicit, so no. The above merely sets the state.

pc.close() is always user-invoked now, and by design we don't fire events on user-invoked actions.

Unless some of the algorithms invoked include steps to fire events, then no events are fired. If you find one that fires an event, then that's probably a bug, as I'm pretty sure our design intent is to not cause events on close.

[alvestrand]

There are more things that need to be fixed even after #1031 is merged.

[adam-be]

We also need to check the closed state when replaceTrack queues a task to do the actual swap of track to withTrack (replaceTrack step 9.3).

[jhaenchen]

Am I understanding this properly? If I have an RTCPeerConnection with another client and an ongoing data channel connection with that same other client and the other client then force quits their browser, oniceconnectionstatechange will fire with iceConnectionState = failed. Then, if I call .close on the data channel, dataChannel.readyState goes to closing but never transitions to closed, I assume because the PeerConnection itself is broken but the data channel doesn't realize that.

Is this intended behavior?

[jhaenchen]

Docs imply that one should not call .close and assume it's finished. We are told to wait for the .onclose event. But if the supporting PeerConnection is broken, that event is never called.

[jan-ivar]

@jhaenchen My understanding is SCTP ABORT should be instant.

With Firefox, I get a close event instantly (long before iceConnectionState = failed) when the other side is also Firefox and I refresh that one's tab. With Chrome I see issues like you describe. Sounds like https://bugs.chromium.org/p/chromium/issues/detail?id=689017

Regardless, this seems unrelated to this issue, since the peer connection itself is never closed.