Closed Bug 1518672 Opened Last year Closed 10 months ago

signalingstatechange event fires too soon.

Categories

(Core :: WebRTC: Signaling, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- fixed

People

(Reporter: jib, Assigned: bwc)

Details

Attachments

(1 file)

Specifically, signalingstatechange fires before currentDirection is set when going to "stable", diminishing its usefulness for tracking negotiation results.

STRs:

  1. Open https://jsfiddle.net/jib1/2s0yonrf/ and share camera (twice).
  2. Check the first Hold button only.

Expected result:

.----
|          Hold ☑
|sendonly
|sendonly
`----
           Hold ☐
recvonly
recvonly

Actual result:

.----
|          Hold ☑
|sendrecv
|sendonly
`----
           Hold ☐
sendrecv
recvonly

The relevant code emitting these being:

pc.onsignalingstatechange = async () => {
  update(pc.getTransceivers()[0].currentDirection);
  await wait(200);
  update2(pc.getTransceivers()[0].currentDirection);
}

The spec [1] says to fire this event in step 10, basically after all state has been updated (steps 10-16 solely fire events and resolve the call, whereas currentDirection is set in steps 7 or 8).

Not a regression AFAICT.

[1] http://w3c.github.io/webrtc-pc/#set-description

Note that Chrome also fires the signalingstatechange event too soon, but differently. [3]

Firefox fires it AFTER transceivers are available, but before currentDirection is updated.
Chrome fires it BEFORE transceivers are available, and before currentDirection is updated. [2]
The spec [1] says to fire it after currentDirection has been updated.

Because browser behavior differs somewhat, I doubt anyone is using this event for anything significant; a good thing since hopefully it's not too late to fix it.

[2] https://jsfiddle.net/jib1/5ugb7fmd/
[3] https://bugs.chromium.org/p/chromium/issues/detail?id=920200

Chrome has fixed their end, so I'm, up'ing rank here a bit.

This flaw becomes important when people rely on negotiationneeded vs rolling their own negotiation logic.

Rank: 12
Assignee: nobody → docfaraday
Pushed by bcampen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/df88ddfaf8e3
Sync transceivers before firing signalingstatecheanged. r=jib
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Is this something we might want to uplift or is it best left in 68?

Flags: needinfo?(docfaraday)

Let's let this ride the trains.

Flags: needinfo?(docfaraday)
You need to log in before you can comment on or make changes to this bug.