1811912 - Implement RTCIceTransport.state

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, enhancement)

Description

[Byron Campen [:bwc]]

This is the bare minimum we need to get bug 1265827 working.

This will involve touching multiple places, including MediaTransportHandlerSTS, MediaTransportHandlerIPC, PMediaTransport, MediaTransportHandler, RTCRtpTransceiver, and RTCDtlsTransport. Most of this is a pretty straightforward copy/paste of what we're doing for DTLS state, so should not be too hard.

For this, we will need to add some ICE state-change signals to MediaTransportHandler that will live alongside the existing DTLS state-change signals here:

https://searchfox.org/mozilla-central/rev/c5eeb9f4fcfe52b028e7774054444f2982feccc3/dom/media/webrtc/jsapi/MediaTransportHandler.h#129-131

These will be connected to TransportLayerIce, similar to the way that the existing signals are connected to the TransportLayerDtls:

https://searchfox.org/mozilla-central/rev/c5eeb9f4fcfe52b028e7774054444f2982feccc3/dom/media/webrtc/jsapi/MediaTransportHandler.cpp#853-860

Once we have the new signals, we can then use them to update RTCIceTransport.state similar to how we update RTCDtlsTransport.state here:

https://searchfox.org/mozilla-central/rev/c5eeb9f4fcfe52b028e7774054444f2982feccc3/dom/media/webrtc/jsapi/RTCRtpTransceiver.cpp#233-236

and

https://searchfox.org/mozilla-central/rev/c5eeb9f4fcfe52b028e7774054444f2982feccc3/dom/media/webrtc/jsapi/RTCRtpTransceiver.cpp#255-270

Comment 1

[Byron Campen [:bwc]]

Actually, this has nothing to do with RTCPeerConnection.connectionState; that is based on RTCDtlsTransport.state, which we already have. Once we have this, we will want to use it to reimplement our existing implementation of RTCPeerConnection.iceConnectionState.

Comment 2

[Byron Campen [:bwc]]

Attachment: Bug 1811912: Test cases for RTCIceTransport. r?jib

Comment 3

[Byron Campen [:bwc]]

Attachment: Bug 1811912: webidl for RTCIceTransport r?jib,#webidl

Depends on D193124

Comment 4

[Byron Campen [:bwc]]

Attachment: Bug 1811912: Basic webidl impl r?jib

Depends on D193125

Comment 5

[Byron Campen [:bwc]]

Attachment: Bug 1811912: Handle ICE connection and gathering states on a per-transport basis. r?mjf

Depends on D193126

Comment 6

[Byron Campen [:bwc]]

Attachment: Bug 1811912: Handling for state updates. r?jib

Also, set up the cycle collection for the transport objects held by PCImpl

Depends on D193127

Comment 7

[Byron Campen [:bwc]]

Attachment: Bug 1811912: Remove redundant empty candidate signal. r?mjf

Depends on D193128

Comment 8

[Byron Campen [:bwc]]

Attachment: Bug 1811912: Fix bug where the old stream wasn't being closed after an ICE restart. r?mjf

Depends on D193129

Comment 9

[Phabricator Automation]

Comment on attachment 9362714 [details]
Bug 1811912: Fix bug where the old stream wasn't being closed after an ICE restart. r?mjf

Revision D193130 was moved to bug 1792042. Setting attachment 9362714 [details] to obsolete.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:bwc, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 11

[Byron Campen [:bwc]]

Attachment: Bug 1811912: Better logging of timeouts here. r?jib

Depends on D193129

Comment 12

[Byron Campen [:bwc]]

Attachment: Bug 1811912: Update expectations in the ICE unit-test, and fix some handling of the "disconnected" state. r?mjf

Depends on D193129

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/65eb9f0691f0
Test cases for RTCIceTransport. r=jib
https://hg.mozilla.org/integration/autoland/rev/0692ec0775c8
webidl for RTCIceTransport r=jib,webidl,saschanaz
https://hg.mozilla.org/integration/autoland/rev/8c0e34780310
Basic webidl impl r=jib
https://hg.mozilla.org/integration/autoland/rev/5eb89e1610b7
Handle ICE connection and gathering states on a per-transport basis. r=mjf
https://hg.mozilla.org/integration/autoland/rev/e17aa225e500
Handling for state updates. r=jib
https://hg.mozilla.org/integration/autoland/rev/da08fd49b4ec
Remove redundant empty candidate signal. r=mjf
https://hg.mozilla.org/integration/autoland/rev/502af1ea1f4c
Update expectations in the ICE unit-test, and fix some handling of the "disconnected" state. r=mjf

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/44687 for changes under testing/web-platform/tests

Comment 15

[Sandor Molnar[:smolnar]]

Backed out for causing mochitest failures @ dom/tests/mochitest/general/test_interfaces.html

Backout link: https://hg.mozilla.org/integration/autoland/rev/d569825df70b0d17ba27168cdee1552145d14bd2

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | dom/tests/mochitest/general/test_interfaces.html | If this is failing: DANGER, are you sure you want to expose the new interface RTCIceTransport to all webpages as a property on 'window'?

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f743d6b74f60
Test cases for RTCIceTransport. r=jib
https://hg.mozilla.org/integration/autoland/rev/c0dd798d5c5d
webidl for RTCIceTransport r=jib,webidl,saschanaz
https://hg.mozilla.org/integration/autoland/rev/fd5b4b44fba4
Basic webidl impl r=jib
https://hg.mozilla.org/integration/autoland/rev/a2abe7d0112f
Handle ICE connection and gathering states on a per-transport basis. r=mjf
https://hg.mozilla.org/integration/autoland/rev/e410081135ed
Handling for state updates. r=jib
https://hg.mozilla.org/integration/autoland/rev/476e830b9b9f
Remove redundant empty candidate signal. r=mjf
https://hg.mozilla.org/integration/autoland/rev/ef36a8aca9a2
Update expectations in the ICE unit-test, and fix some handling of the "disconnected" state. r=mjf

Comment 18

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/f743d6b74f60
https://hg.mozilla.org/mozilla-central/rev/c0dd798d5c5d
https://hg.mozilla.org/mozilla-central/rev/fd5b4b44fba4
https://hg.mozilla.org/mozilla-central/rev/a2abe7d0112f
https://hg.mozilla.org/mozilla-central/rev/e410081135ed
https://hg.mozilla.org/mozilla-central/rev/476e830b9b9f
https://hg.mozilla.org/mozilla-central/rev/ef36a8aca9a2

Comment 19

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 20

[Hamish Willee]

FF125 MDN Docs work for this can be tracked in https://github.com/mdn/content/issues/32775. This primarily includes some tidying of the existing docs, compatibility updates, and an MDN release note.

FYI we didn't spot this initially because the keyword dev-docs-needed had not been added. Would appreciate that being added when something ships. We picked this up from BCD collector tests.