Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify/strengthen timing invariants of candidates not firing before SLD(offer) #2199

Closed
jan-ivar opened this issue May 30, 2019 · 2 comments · Fixed by #2203
Closed

Clarify/strengthen timing invariants of candidates not firing before SLD(offer) #2199

jan-ivar opened this issue May 30, 2019 · 2 comments · Fixed by #2203
Assignees

Comments

@jan-ivar
Copy link
Member

Based on a discussion in this crbug, we've found that Firefox's behavior to avoid ice candidates racing with offer/answer is desirable, and seems to be following the broad intent of the spec and the desired API properties espoused, even if there's no explicit prose to guarantee it. Since it's not clear that the spec mandates this behavior, so we're opening an issue to enshrine it.

Firefox guarantees icecandidate always fires AFTER the SLD(offer) success callback, so even if users do:

pc.onicecandidate = e => io.send(e.candidate);

await pc.setLocalDescription(await pc.createOffer());
io.send(pc.localDescription);

We guarantee the following signaling order:

    offer candidate candidate candidate

and in the other direction:

    answer candidate candidate candidate

That is, candidates always follow the offer/answer they belong to, a nice invariant. You'll never see this in Firefox:

    candidate offer candidate candidate

Which is a good thing, as it would cause addIceCandidate to throw InvalidStateError, unless you queued the candidates, which defeats the purpose of trickle ICE in the first place.

From my limited tests with https://jsfiddle.net/jib1/vfnc26mL/ it would appear Chrome has this invariant as well? At least I was not able to provoke otherwise. I get:

SLD(offer) success
pc1 candidate
pc1 candidate
pc1 candidate
pc1 candidate
pc1 candidate
SLD(answer) success
pc2 candidate
pc2 candidate
pc2 candidate
@jan-ivar jan-ivar self-assigned this May 30, 2019
@alvestrand
Copy link
Contributor

If you can add a WPT test that verifies the behavior, I'm happy to make this change. Offhand, I can't think of a case where it would be beneficial to generate a candidate before SLD() resolves.

@fippo
Copy link
Contributor

fippo commented Jun 3, 2019

while that is mostly guaranteed by timing (gathering candidates takes time), iceCandidatePoolSize is interesting. The intent in JSEP is pretty clear, gather but surface only via onicecandidate which avoids duplicate candidates and the need to filter them as mentioned here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants