1355259 - ICE TCP user pref filters out all candidates

Status: RESOLVED FIXED

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

Comment 1

[Nils Ohlmeier [:drno]]

Attachment: Bug 1355259: only filter out udp candidates if force_tcp is set.

Review commit: https://reviewboard.mozilla.org/r/128682/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128682/

Comment 2

[Michael Froman [:mjf]]

Comment on attachment 8856754 [details]
Bug 1355259: only filter out udp candidates if force_tcp is set.

https://reviewboard.mozilla.org/r/128682/#review131406

Looks good to me.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a89e8ea30f7d
only filter out udp candidates if force_tcp is set. r=mjf

Comment 4

[Iris Hsiao [:ihsiao]]

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

Comment 5

[Andrei Vaida [:avaida]]

Nils, Maire, this bug is currently blocking Desktop Release QA's pre-Beta sign off (Fx54) for ICE TCP.

Are there any plans to uplift this fix to Aurora 54 _before_ the merge of 54 to Beta, next week? 

Our main concern right now is that we won't be able to test this bug fix soon enough, for regressions. We're also not sure what risks we'd be taking by green lighting ICE TPC's release to Beta 54 _without_ this fix.

Comment 6

[Nils Ohlmeier [:drno]]

Comment on attachment 8856754 [details]
Bug 1355259: only filter out udp candidates if force_tcp is set.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1266667 landed with a broken test function.
[User impact if declined]: None for end users. It prevents QA from testing ICE TCP easily.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Same as for testing ICE TCP 1176382.
[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Not risky because it only changes a filtering logic which is intended for testing purposes only. The filter only gets turned on by flipping a hidden user pref.
[String changes made/needed]: N/A

Comment 7

[Nils Ohlmeier [:drno]]

Thanks Andrei for the follow up.

I just filled the uplift request and I hope it still can get merged before the 54 uplift.

That being said I don't consider the feature very risky per se. It will only work with certain WebRTC server based services and it will only kick in if the network environment is bad. That's why I think even if the feature in worst case turns out to crash the browser it would only affect a very small portion of our users.

I'm fine with letting this feature ride the trains with 54 up into Beta without getting tested while 54 is still Aurora. As long as the feature gets tested in 54 Beta cycle earlier we can still turn the pref back to off if the feature turns out to be problematic.

Comment 8

[Gerry Chang [:gchang]]

Comment on attachment 8856754 [details]
Bug 1355259: only filter out udp candidates if force_tcp is set.

Fix a blocker for QA testing. Aurora54+.

Hi :avaida,
Please start to test this when this is merged to aurora.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4c78da5f6dda

Comment 10

[Andrei Vaida [:avaida]]

(In reply to Gerry Chang [:gchang] from comment #8)
> Fix a blocker for QA testing. Aurora54+.
> 
> Hi :avaida,
> Please start to test this when this is merged to aurora.

This is going to be on Iulia's priority list, forwarding ni?.

Comment 11

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

Tested again the feature on the fixed aurora builds. The fix looks good till now. The sign off report is sent.

Comment 12

[Maire Reavy [:mreavy]]

I was on PTO last week, but it looks like all questions have been answered.