Closed Bug 1355259 Opened 3 years ago Closed 3 years ago

ICE TCP user pref filters out all candidates

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox52 --- unaffected
firefox53 --- unaffected
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: drno, Assigned: drno)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.
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.
Attachment #8856754 - Flags: review?(mfroman) → review+
Pushed by drno@ohlmeier.org:
https://hg.mozilla.org/integration/autoland/rev/a89e8ea30f7d
only filter out udp candidates if force_tcp is set. r=mjf
https://hg.mozilla.org/mozilla-central/rev/a89e8ea30f7d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
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.
Flags: needinfo?(mreavy)
Flags: needinfo?(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
Attachment #8856754 - Flags: approval-mozilla-aurora?
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.
Flags: needinfo?(drno)
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.
Flags: needinfo?(andrei.vaida)
Attachment #8856754 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(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?.
Flags: needinfo?(andrei.vaida) → needinfo?(iulia.cristescu)
Tested again the feature on the fixed aurora builds. The fix looks good till now. The sign off report is sent.
Flags: needinfo?(iulia.cristescu)
I was on PTO last week, but it looks like all questions have been answered.
Flags: needinfo?(mreavy)
You need to log in before you can comment on or make changes to this bug.