Setting parity keywords.
We got support info for this wrong in our docs and a user reported: "Because of this bug, Firefox does not fit in our new security concept where the WebRTC user gets an generated username and password for a TURN server,"

See https://github.com/mdn/browser-compat-data/pull/4075

In my case it is also needed with mediasoup-client with TURN server that has short-lived credentials.
Essentially, RTCPeerConnection is created upfront and only when it is about to be used credentials are generated and set using peer.setConfiguration() before calling peer.setRemoteDescription().
Not having this feature causes significant inconveniences for me.

Also it is supported in all major browsers except Firefox, please consider implementing it.

We have word from (more) partners this is costing a lot of connections that aren't going to succeed. During setup, sites are unable to adjust the configuration to adapt to changing network conditions, e.g. from RFC8828.

Up-prioritizing.

So, looking at this, right now nICEr keeps the set of ICE servers in the main ice context. We probably need to copy this set of ICE servers when creating each nr_ice_media_stream, so each stream has its own copy. This will allow the use of different sets of ICE servers for each stream so we don't run into dangling pointer problems when changing the set. This will also permit gathering to continue (and ICE to connect) on old streams, if for some reason JS waits after calling setConfiguration, which will mesh better with the way we handle ICE restart and rollback. I am not sure what the spec says to do if the ICE servers are changed with setConfiguration, gathering is started with sLD, and then that sLD is rolled back, but it seems reasonable that the code would continue trying to use the old servers until an ICE restart is actually carried out.

We will also need to break out the ICE server validation code into a separate function on MediaTransportHandler; right now that is built into CreateIceCtx, but that is only ever called once. Additionally, we need to do this validation synchronously, because setConfiguration is synchronous. We probably would not want to actually set anything on the NrIceCtx until we get to the StartIceGathering step, for simplicity's sake.

On the subject of ICE transport policy, we probably need to handle this identically to the ICE servers; each nr_ice_media_stream would need to have its own flags field. It would be fine if we copied from the flags field in nr_ice_ctx, probably. This would have the same desirable properties wrt ICE restart and rollback as above.

Lastly, while we have not yet implemented ICE candidate pool logic, the size of this pool can be tweaked with setConfiguration, so we need to think a little about what implementing that would look like. Presumably, with ICE candidate pooling we would either need to pre-create nr_ice_media_streams for later use, or have some other object that serves as a candidate set inside nICEr, which would then have its own copy of the necessary configuration. Also, there is a wrinkle here; ICE pool gathering starts immediately on setConfiguration. This implies that changing either the ICE servers or policy should result in the old pool being discarded immediately as well, even if the pool size has not changed. The spec explicitly mentions this for configuration.iceServers, but not for configuration.iceTransportPolicy (although it does say that "no action will be taken until the next gathering phase", which could be interpreted to apply to pooled ICE candidates). This may require some spec clarification.

On the subject of testing, there is a little bit of coverage in wpt, but it is extremely limited because wpt does not (and can not) test the actual use of ICE servers, or ICE transport policy, because wpt does not have the ability to use STUN/TURN servers. We will need to write some mochitests for this, in addition to enabling some of the wpt suite.

It looks like we can write at least a little bit of ICE testing for this in wpt by using iceTransportPolicy to set up situations where ICE gathering will yield no candidates (using the 'relay' policy). We can also switch from 'all' to 'relay' (and vice-versa) and get observable effects. We will not be able to test the same sort of behavior for iceServers in wpt, but it is at least something.

Re-enable several tests, and add some ICE testing using iceTransportPolicy.
Cannot test ICE servers, since wpt does not have any harness support for that.

Mainly covers ICE servers, since we don't have support for that in wpt.

Depends on D135361

Depends on D135362

Depends on D135363

Used to be built into CreateIceCtx, but needed to be stand-alone so it
could be called subsequently. Necessitated adding some members so pref-based
config state could be saved for later use.

Depends on D135364

Having a single codepath for this should make things easier to maintain.

Depends on D135365

Used to be part of NrIceCtx::Init, but needed to be broken out.

Depends on D135366

Depends on D135367

Once we allow the ICE servers to be set, each stream can have a different array
of ICE servers, and those ICE servers can have different lifetimes. Let's avoid
the potential dangling pointer issue here.

Also, this fixes a minor spec violation where we were doing foundation comparison
incorrectly (we're supposed to only compare the address).

Depends on D135368

This was a pre-existing bug detected by new tests.

Depends on D135369

This was a pre-existing bug detected by new tests. Also add a little logging.

Depends on D135370

Because a given ICE ctx can have multiple configurations active at once (since
setConfiguration is tentative until the ICE restart it requires is finalized), we
give each stream its own config, since that is simple.

I also renamed the stun/turn server fields in nr_ice_ctx, to make it easier to
check that I did not miss any places that needed to be switched over to using the
fields in nr_ice_media_stream.

Depends on D135371

Depends on D135372

Depends on D135373

This can happen in cases where we have an old ICE ctx that is infeasible, but have
kicked off an ICE restart with a new configuration that is feasible. The old streams
are not obsolete yet, because we have not committed the ICE restart.

Depends on D135374

Depends on D135375

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e980b8f3ceadd897872d43ec78c7256b732f5355

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f53ecc3b0cff1b3554441b79d8527346aabd3108
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8245cad9a0d860267f801391db19c4f59463413

After rebase:

Baseline:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dac0fa9d16c18d80f7dd8ed355550430b93eae09
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3391794e7c71b894c5739a6de77bd7e2f5e59ef5

With patches:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=17c1a8fec9cbcd601e5888db86a0566c5213e427
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff791da951682d002e6e4b4d7c9385eea201216f

It looks like we might need to relax some of the timeouts for windows aarch64. Otherwise, mochitest looks fine. Android 7.0 x86-64 WebRender debug-isolated-process is timing out on the new wpt, similar to how it times out on numerous other wpt already. This is all tier 2 stuff, so maybe not so big of a deal, but I'll at least try to get it working on windows aarch64.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7b8086dfd4c7c272609d1dade39e568c8f1b944e

https://treeherder.mozilla.org/#/jobs?repo=try&revision=53aa8751c90a6a4a173533841d236575893525d7

Ok, figured out what was causing the problems on windows aarch64, and filed bug 1751509 for it. Working around by tweaking the test code.

The patch from Bug 1748852, combined with bug 1752896, caused lots of new failures after these patches were rebased. Right now, investigating whether there's an easy fix for bug 1752896. If not, I'll have to find a workaround in the tests.

https://treeherder.mozilla.org/jobs?repo=try&revision=0768835aefb20a3da296eea1a2b6fa571ca9bd75

Also add some logging that was useful, remove some cruft, and add a little
exception-safety.

Depends on D135376

fuzzy --rebuild 10 runs:
https://treeherder.mozilla.org/jobs?repo=try&revision=c1089388e26a6e54702584bb436b4d06cfd7f15f
https://treeherder.mozilla.org/jobs?repo=try&revision=cfa9ff031b498b9c27000f9dec6f15db681caee4

fuzzy --rebuild 10 runs for baseline:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff96ec0093ff30908f435185a85ff614b2f76d12
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c28366fdc503f9f97f06251a2bfb80bcc129c6e

Backed out for causing Gtest failures.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | WebRtcIcePacketFilterTest.TestRecvStunPacketWithoutAPendingId | Expected equality of these values:

Ugh. Need to update that test case, since there was a fix from bug 1752896.

Hmm, maybe I'll just remove that fix; disallowing STUN responses from unknown transaction ids (but from a known address) is probably beyond the scope of the STUN filter.

https://treeherder.mozilla.org/jobs?repo=try&revision=51443d8d70081d4724b615467f72f0e3d2ba4828
https://treeherder.mozilla.org/jobs?repo=try&revision=23d0ecf58d2cceaf1b51e5b6496c2f9ec3b91fc7
https://treeherder.mozilla.org/jobs?repo=try&revision=ea2c536d47f8e5fcd09839ce66736a89be1f6829

Try looks good.

https://hg.mozilla.org/mozilla-central/rev/13f2b39a4e0b
https://hg.mozilla.org/mozilla-central/rev/56935008c600
https://hg.mozilla.org/mozilla-central/rev/82b9f5abf881
https://hg.mozilla.org/mozilla-central/rev/f040f134be49
https://hg.mozilla.org/mozilla-central/rev/f466f23b4f08
https://hg.mozilla.org/mozilla-central/rev/0dfff3bd0b1e
https://hg.mozilla.org/mozilla-central/rev/3bbb2706fe05
https://hg.mozilla.org/mozilla-central/rev/b6e4501a6ec8
https://hg.mozilla.org/mozilla-central/rev/4ceb2e958df6
https://hg.mozilla.org/mozilla-central/rev/faa316e8ed12
https://hg.mozilla.org/mozilla-central/rev/33a99b2ca936
https://hg.mozilla.org/mozilla-central/rev/458238d76dc2
https://hg.mozilla.org/mozilla-central/rev/9c25fc4e6a83
https://hg.mozilla.org/mozilla-central/rev/aaa4dfe8b392
https://hg.mozilla.org/mozilla-central/rev/b946cbf9f5d8
https://hg.mozilla.org/mozilla-central/rev/9fc47916fba0

FF99 docs work for this can be tracked in https://github.com/mdn/content/issues/13371#issuecomment-1064737103
It is pretty much complete, and was mostly just browser compatibility update, new release note, and update of docs to include a few missing exceptions from the spec.