1394072 - onnegotiationneeded fires synchronously

Status: RESOLVED DUPLICATE of bug 1290948

(Core :: WebRTC, defect, P2)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

+++ This bug was initially created as a clone of Bug #1175609 +++

Dunno what happened, but it looks like bug 1175609 didn't get fixed.

STR:
- Open URL and hit [Start!]

Expected output:
  pc1.addStream 1
  pc1.addStream 2
  pc1.negotiationneeded
  Connected!

Actual output:
  pc1.addStream 1
  pc1.negotiationneeded
  pc1.addStream 2
  Connected!

The spec [1] says we need to queue a task to fire negotiationneeded and we're still not doing so.
This can probably be solved easily with Services.tm.dispatchToMainThread() in PeerConnection.js

[1] http://w3c.github.io/webrtc-pc/#dfn-update-the-negotiation-needed-flag

Comment 1

[Nils Ohlmeier [:drno]]

It looks like I'm missing something important here, but how is the implementation suppose to know that there are more addStream(), addTrack() operations waiting before it fires ONN?

Or in other words: what do you expect to happen for "to queue a task"?

It sound to me like you are expecting that all queued tasks will only be executed once the code block in the {} of the .then() has finished executing. If that is the case how to you prevent the task queue from getting executed if the addStream() call for example uses internally promises itself, which finish and cause the event loop to deliver the events?

Comment 2

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

The JavaScript event-loop and run-to-completion semantics [1] dictate what must happen here. All JavaScript runs as callbacks (tasks) on the single main thread, where they're expected to complete in microscopic time. There's no "main-loop", instead top-level JS is merely an initial callback.

Importantly, no two callbacks (tasks) ever run at the same time. Each task runs to completion without interruption.

For https://jsfiddle.net/jib1/vmo3sn3t/ this means only two tasks run (when implemented to spec):

Task A: Run these synchronous steps uninterrupted and in order:

  1. let pc = new RTCPeerConnection(), stream = new MediaStream([black(), silence()]);
  2. pc.onnegotiationneeded = someNegotiationFunction;
  3. pc.addTrack(stream.getAudioTracks()[0], stream);
  4. log("added audio");
  5. pc.addTrack(stream.getVideoTracks()[0], stream);
  6. log("added video");
  7. return;

The output at this point should be:
  added audio
  added video

Task B: Run the steps of someNegotiationFunction uninterrupted and in order:
  8. log("negotiating")
  9. return;

Total output at this point should be:
  added audio
  added video
  negotiating

Firefox's actual output (because it wrongly calls pc.onnegotiationneeded from insider addTrack):
  negotiating
  added audio
  added video

Task B is queued by the "update the negotiation-needed flag" algorithm [2], and dispatchEvent() is a synchronous function. 

Hope that helps.

[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/EventLoop
[2] https://w3c.github.io/webrtc-pc/#dfn-update-the-negotiation-needed-flag

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

I've updated the fiddle (will teach me not to rely on rawgit.com for serving up js)

Comment 4

[Bulk Bug Changes for mreavy's org]

Mass change P1->P2 to align with new Mozilla triage process

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Fixed by bug 1290948.