1091290 - addIceCandidate calls queued incorrectly (already-queued callbacks may never get called)

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: queue addIceCandidate correctly to not overwrite already-queued callbacks.

Observed by code-inspection.

addIceCandidate is queued incorrectly compared to other methods, storing off onSuccess/onFailure callbacks before the queue rather than after.

This overwrites the callbacks of any addIceCandidate call already on the queue.

STR in theory:

- Call createOffer immediately followed by two addIceCandidate calls, i.e.
  don't wait for createOffer successCallback before making the two calls.

Expected result:
- All three calls should succeed (or fail if something was wrong with them).

Actual result:
- You should see createOffer and the second addIceCandidate succeed (or fail),
  but the first addIceCandidate will never end.

Comment 1

[Byron Campen [:bwc]]

Comment on attachment 8513890 [details] [diff] [review]
queue addIceCandidate correctly to not overwrite already-queued callbacks.

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

I think this will only cause problems if you make multiple addIceCandidate calls with different callbacks (including absent callbacks, potentially), but yeah that is a bug. Pretty sure this patch fixes that problem, since we end up adding the candidate and firing the callback (ie; calling callCB(_onAddIceCandidateSuccess/Failure)) synchronously.

Comment 2

[Carsten Book [:Tomcat]]

Hi, could you provide a try link? Thanks!

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Green try - https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=643adc2ac0e8

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aa148a97a974

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/aa148a97a974