1187472 - Candidate gathering failures on UDP or TCP results in no local candidates

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P1)

Description

[Nils Ohlmeier [:drno]]

After the landing of ICE TCP a failure to gather UDP or TCP candidates can be ignored as long as we were able to gather at least one candidate.

Comment 1

[Nils Ohlmeier [:drno]]

Attachment: MozReview Request: Bug 1187472: only log UDP and TCP candidate gathering failures. r=bwc

Bug 1187472: only log UDP and TCP candidate gathering failures

Comment 2

[Byron Campen [:bwc]]

Comment on attachment 8638798 [details]
MozReview Request: Bug 1187472: only log UDP and TCP candidate gathering failures. r=bwc

https://reviewboard.mozilla.org/r/14125/#review12727

::: media/mtransport/third_party/nICEr/src/ice/ice_component.c:601
(Diff revision 1)
> -      r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): couldn't create any valid candidates",ctx->label);
> +      r_log(LOG_ICE,LOG_WARNING,"ICE(%s): couldn't create any valid candidates",ctx->label);

Let's make this an error, since it is definitely going to cause ICE to fail.

Comment 3

[Nils Ohlmeier [:drno]]

Just manually tested with e10s on an optimized Nightly build, and it fixes the problem in bug 1185197.

mreavy: this protects us against ICE TCP (and TURN TCP) usage if e10s is on (it will just do UDP). But there is no specific indicator that e10s is causing to fail the ICE TCP part. Quick attempts from me to add warning log message about this constellation failed. Do we want/need to put in more effort for adding such an interim log message (assuming that TCP e10s support will land hopefully soon)?

Comment 4

[Nils Ohlmeier [:drno]]

Comment on attachment 8638798 [details]
MozReview Request: Bug 1187472: only log UDP and TCP candidate gathering failures. r=bwc

Bug 1187472: only log UDP and TCP candidate gathering failures. r=bwc

Comment 5

[Nils Ohlmeier [:drno]]

Comment on attachment 8638798 [details]
MozReview Request: Bug 1187472: only log UDP and TCP candidate gathering failures. r=bwc

Carrying forward r+=bwc

Comment 6

[Maire Reavy [:mreavy]]

(In reply to Nils Ohlmeier [:drno] from comment #3)
> Just manually tested with e10s on an optimized Nightly build, and it fixes
> the problem in bug 1185197.
> 
> mreavy: this protects us against ICE TCP (and TURN TCP) usage if e10s is on
> (it will just do UDP). But there is no specific indicator that e10s is
> causing to fail the ICE TCP part. Quick attempts from me to add warning log
> message about this constellation failed. Do we want/need to put in more
> effort for adding such an interim log message (assuming that TCP e10s
> support will land hopefully soon)?

No, it's not worth your time to add an interim log message.  This feature is currently pref'd off and Jesup's patch is in review -- so it should land soon (almost certainly before you are ready to pref on).

Comment 7

[Pulsebot]

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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