[webrtc-pc] negotiationneeded may still fire when it shouldn't (#2477)

jan-ivar has just created a new issue for https://github.com/w3c/webrtc-pc:

== negotiationneeded may still fire when it shouldn't ==
In https://github.com/w3c/webrtc-pc/pull/2405 we added a "chain" step to the [update the negotiation-needed flag](https://w3c.github.io/webrtc-pc/#dfn-update-the-negotiation-needed-flag) algorithm, to avoid firing NN if negotiation had begun (but was still in `"stable"`):
> ...
> _6. [Chain](https://w3c.github.io/webrtc-pc/#dfn-chain) a step to queue a task that runs the following steps, to connection's operations chain:_
> 1. _If connection.[[IsClosed]] is true, abort these steps._
> 2. _If connection's signaling state is not "stable", abort these steps._
> 3. _If connection.[[NegotiationNeeded]] is false, abort these steps._
> 4. _Fire an event named negotiationneeded at connection._

Unfortunately, I believe this may fire twice now in an SRD(answer) edge-case:
pc.setRemoteDesciption(answer); // note: no await
pc.addTransceiver("audio"); // will fire NN once "stable"; but so will SRD; = twice!
Also, because we chain-_then_-queue, by the time the queued task eventually runs, another already queued task may have begun negotiation: 🤦
onmessage = ({data: {description}}) => pc.setRemoteDescription(description); // race me
await wait(10);
pc.addTransceiver("audio"); // queued NN task may race with queued onmessage

I see a couple ways to solve this, but let's recap **required behaviors**, so we don't break any:
1. Must defer any firing to after the current run of the event loop
2. Must not fire if closed
3. Must not fire if negotiation has begun (still in `"stable"`)
4. Must not fire if negotiation is in process (not in `"stable"`)
5. Must not fire twice
6. Must re-fire going back to `"stable"` if needed (including rollback)
7. Must not re-fire on negotiation failure

Please view or discuss this issue at https://github.com/w3c/webrtc-pc/issues/2477 using your GitHub account

Received on Tuesday, 18 February 2020 18:22:18 UTC