Closed Bug 921656 Opened 7 years ago Closed 7 years ago

onicecandidate not being invoked if ICE gathering finishes before SetLocalDescription does

Categories

(Core :: WebRTC: Signaling, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: abr, Assigned: abr)

References

Details

Attachments

(1 file)

After Bug 842549 landed, the required "null onicecandidate" callback stopped being called when the SDP is already complete at the time SetLocalDescription is called.
This patch actually addresses four flaws that I unearthed while researching this bug:

- onSetLocalDescriptionSuccess was trying to use
  PeerConnectionObserver._iceGatheringState rather than
  RTCPeerConnection._iceGatheringState

- handleIceStateChanges was not reliably updating _iceGatheringState.
  In my evaluation, this was made likely by the use of a switch
  statement that replicated significant code among its branches. To
  prevent similar regressions in the future, I have converted
  handleIceStateChanges to be table-driven, so that all states share
  common code.

- foundIceCandidate rather confusingly took "cand", "mid", and
  "line" as parameters, even though only "cand" was significant (and
  was required to contain an RTCIceCandidate). The other two attributes
  were dutifully placed in a dictionary, and then cheerfully ignored
  by RTCPeerConnectionIceEventInit.

- The switch statement in onStateChange was emitting spurious error
  messages to the console due to a missing branch for kReadyState.
Attachment #811409 - Flags: review?(jib)
Blocks: 842549
Comment on attachment 811409 [details] [diff] [review]
Fix ICE state change handling and onicecandidate callback

Review of attachment 811409 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm.
Attachment #811409 - Flags: review?(jib) → review+
https://hg.mozilla.org/mozilla-central/rev/f169382a0216
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.