Trickle ICE doesn't always send a null-candidate at the end of the candidate list

RESOLVED FIXED in Firefox 27



5 years ago
5 years ago


(Reporter: standard8, Assigned: standard8)



Firefox Tracking Flags

(firefox26 unaffected, firefox27 fixed, firefox28 fixed)


(Whiteboard: [qa-])


(1 attachment)



5 years ago
I've been looking at ICE candidate handling, and I've noticed that null candidates don't always get generated.

Doing some debugging in PeerConnection.js, I saw:

- onSetLocalDescription getting called with the ice gathering state != complete
- two ice candidates being generated
- handleIceStateChanges gets called with "IceWaiting"

at this stage, there's no null candidate being generated.

Further investigations reveal that 'this.localDescription' is never going to be valid, as PeerConnectionObserver has no such property/accessor.

This is the offending code:

Comment 1

5 years ago
Created attachment 824871 [details] [diff] [review]
Possible fix

I think this should fix it - it generates a null ICE candidate for the example I'm seeing locally, and I've confirmed it doesn't generate a null ICE candidate when the candidates are all obtained before a local description has been generated.
Attachment #824871 - Flags: review?(ekr)

Comment 2

5 years ago
Comment on attachment 824871 [details] [diff] [review]
Possible fix

This looks right to me.
Attachment #824871 - Flags: review?(ekr) → review+

Comment 3

5 years ago
Assignee: nobody → mbanner
Target Milestone: --- → mozilla28


5 years ago
status-firefox26: --- → unaffected
status-firefox27: --- → affected
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 5

5 years ago
Comment on attachment 824871 [details] [diff] [review]
Possible fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 842549 (this bug implemented the trickle ice feature)
User impact if declined: Developers connecting Firefox WebRTC to servers not implementing trickle ICE won't know when the candidate list has completed, leading to either not being able to connect, or connections taking longer than necessary. 
Testing completed (on m-c, etc.): Landed on m-c, tested fix locally
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: None
Attachment #824871 - Flags: approval-mozilla-aurora?
Comment on attachment 824871 [details] [diff] [review]
Possible fix

Approving the patch given the low risk vs the user impact. But will be helpful if we can verify this or add a testcase here.
Attachment #824871 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox27: affected → fixed
status-firefox28: --- → fixed


5 years ago
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.