Closed Bug 1391857 Opened 3 years ago Closed 3 years ago
[e10s] Gathering option 2 is broken if e10s is on
59 bytes, text/x-review-board-request
The trickle ICE sample page https://webrtc.github.io/samples/src/content/peerconnection/trickle-ice/ shows only the IP interface of one interface if e10s is turned off, but all interfaces are gathered if e10s is on. This breaks our default Option 2 of the gathering only the default route if the user hasn't given any gUM consent.
I'm going to look into this, if you don't mind.
The check for NR_ICE_CTX_FLAGS_ONLY_DEFAULT_ADDRS at  is different when on e10s and non-e10s. Since this is almost certainly my fault, I'll keep digging.  https://dxr.mozilla.org/mozilla-central/rev/7dddbd85047c6dc73ddbe1e423cd643a217845b3/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c#749
Assignee: nobody → mfroman
Comment on attachment 8899939 [details] Bug 1391857 - fixing ctx flags for e10s stun addr gathering. https://reviewboard.mozilla.org/r/171270/#review176502 ::: media/mtransport/third_party/nICEr/src/ice/ice_ctx.c:749 (Diff revision 1) > + r_log(LOG_ICE, LOG_DEBUG, > + "ICE(%s): (ctx->flags & NR_ICE_CTX_FLAGS_ONLY_DEFAULT_ADDRS): %d\n", > + ctx->label, > + (int)(ctx->flags & NR_ICE_CTX_FLAGS_ONLY_DEFAULT_ADDRS?1:0)); Debug message? If not we should probably make it less obscure.
Attachment #8899939 - Flags: review?(drno) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/ef2f00ebda5c fixing ctx flags for e10s stun addr gathering. r=drno
Comment on attachment 8899939 [details] Bug 1391857 - fixing ctx flags for e10s stun addr gathering. Approval Request Comment [Feature/Bug causing the regression]: Bug 1345511 [User impact if declined]: Machines with multiple interfaces will have more than the default local stun address exposed with ice candidates are gathered. [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: No [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: n/a [Is the change risky?]: It is low risk. [Why is the change risky/not risky?]: One method was added that sets flags we're already setting later on the ice context. The flags needed to be set earlier in the processing in e10s mode. [String changes made/needed]: n/a
Attachment #8899939 - Flags: approval-mozilla-beta?
Comment on attachment 8899939 [details] Bug 1391857 - fixing ctx flags for e10s stun addr gathering. "Machines with multiple interfaces will have more than the default local stun address exposed with ice candidates are gathered." OK, I get this is something to do with WebRTC networking but no idea what this sentence means as far as user impact. It seems to have regressed in 55 though. Let's uplift this patch for beta 7 on the assumption it is important for some WebRTC users.
Attachment #8899939 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.