onnegotiationneeded fires synchronously

RESOLVED DUPLICATE of bug 1290948

Status

()

Core
WebRTC
P2
normal
Rank:
15
RESOLVED DUPLICATE of bug 1290948
5 months ago
a month ago

People

(Reporter: jib, Unassigned)

Tracking

(Blocks: 2 bugs)

40 Branch
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

+++ 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
(Reporter)

Updated

5 months ago
Blocks: 1386143

Comment 1

5 months ago
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?
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
I've updated the fiddle (will teach me not to rely on rawgit.com for serving up js)

Updated

4 months ago
Rank: 15
Mass change P1->P2 to align with new Mozilla triage process
Priority: P1 → P2
Fixed by bug 1290948.
Status: NEW → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1290948
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.