1265827 - Add support for RTCPeerConnectionState in RTCPeerConnection

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P2)

Description

[Rajmohan Banavi]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.75 Safari/537.36

Steps to reproduce:

The current WebRTC spec mentions about RTCPeerConnectionState read only attribute in the peer connection object.


Actual results:

Did not locate this attribute in the pc object


Expected results:

Need to add support for the same.

Comment 1

[Rajmohan Banavi]

https://www.w3.org/TR/webrtc/#rtcpeerconnectionstate-enum

Comment 2

[Eric Shepherd [:sheppy]]

The RTCPeerConnectionState enum has been documented: https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection#RTCPeerConnectionState_enum

RTCPeerConnection.connectionState has also been documented: https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/connectionState

The compatibility table on connectionState indicates the lack of implementation.

Leaving this in doc-needed state, however, because additions will have to be made to Firefox X for developers once it's implemented.

Comment 3

[Diwakar]

There has been no update to this bug in a few months.
Can this be prioritized please?
Thanks!

Comment 4

[Danilo B]

Any update on this?

Comment 5

[Bulk Bug Changes for mreavy's org]

Mass change P1->P2 to align with new Mozilla triage process

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 8

[Philipp Hancke [:fippo]]

Chrome just landed connectionState in Chrome 72.

Comment 9

[Nils Ohlmeier [:drno]]

Since the PeerConnectionState appears to be an aggregate of ICE connection state and DTLS connection state I'm not sure if it makes sense to try to implement this without implementing the DTLS connection object as well.

Comment 10

[Byron Campen [:bwc]]

This is already causing timeouts in at least some web-platform-tests from upstream.

Comment 11

[mfatihmar]

Hello!
Coming back to this in 2021, I wonder if there is a particular reason why this API is sitting at P2 (for years!) and not getting implemented?

Related:

• WebRTC 1.0 is moved from Candidate Recommentation to Recommentation
• Standard RTCPeerConnection Interface Definition from the W3C Draft

I was also iterating through some links over MDN docs & GitHub side, someone suggested using RTCPeerConnection.iceConnectionState as a workaround but according to the standard (and also MDN covers this very well), that wouldn't cover the exact same connection state information & behavior.

So, I wanted to bump this thread up to get more information.
Hopefully, this ticket would be triaged to higher priority so that an implementation might get into the next FF release cycle :)

Comment 12

[Rudi Floren [:valkum]]

Attachment: Bug 1265827 - Add connectionState to RTCPeerConnection. r=jib

Subscribes to dtls changes and aggragates them together with the iceConnectionState.
The aggregates state is exposed as RTCPeerConnection.connectionState.
The respective events are dispatched for connectionstatechange

Comment 13

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Sorry for taking so long with this. Will try to get to review tomorrow.

Comment 14

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

The bug assignee didn't login in Bugzilla in the last months and this bug has priority 'P2'.
:mjf, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 15

[Michael Froman [:mjf]]

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #13)

Sorry for taking so long with this. Will try to get to review tomorrow.

:jib - have you had a chance to look at the patch?

Comment 16

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Byron reviewed this on March 30th and unfortunately we haven't heard back from the contributor. Unfortunately, a lot of refactoring has happened in the code this patch touches since it was written. I'm thankful for the contributor providing this patch, and it's on us that we didn't review it in time.

At this point our best path forward may be for someone to commandeer the revision (keeping original attribution) and salvage necessary parts to where they need to go in the new structure. Byron would probably be able to do this quickest, since he did the refactor, with me coming in second.

Byron, does that sound right?

Comment 17

[Byron Campen [:bwc]]

I think that's the best way forward at this point.

Comment 18

[Alan Mantoux]

Any chances to see this land in a release?
Some apps would like to support Firefox for WebRTC

Comment 19

[Byron Campen [:bwc]]

Ok, I think we ought to be able to just implement this in terms of RTCDtlsTransport.state, which we already have implemented. I think this ought to be relatively easy.

Comment 20

[Byron Campen [:bwc]]

Attachment: Bug 1265827: Re-enable wpt for RTCPeerConnectionState. r?jib

Also, update meta files for tests that still fail for another reason.

Comment 21

[Byron Campen [:bwc]]

Attachment: Bug 1265827: Implement RTCPeerConnectionState. r?jib

Depends on D168142

Comment 22

[Byron Campen [:bwc]]

Try looks fine:

https://treeherder.mozilla.org/jobs?repo=try&revision=8d41c47d0b2dbf650f6fb94f90f760570cd6a30c
https://treeherder.mozilla.org/jobs?repo=try&revision=41a57e631d0973aae20d2ec5b096753ec2bb4bcc

Comment 23

[Byron Campen [:bwc]]

https://bugzilla.mozilla.org/show_bug.cgi?id=1643050#c2 indicates that split.https.html might work once this is implemented, but I have not noticed that. Just adding a note here.

Comment 24

[Byron Campen [:bwc]]

Looks like split.https.html works with this change now.

https://treeherder.mozilla.org/jobs?repo=try&revision=51ff7f521dde5e8f560bcfc6271ad7f1887204a5

Comment 26

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

The severity field for this bug is set to S3. However, the following bug duplicate has higher severity:

• Bug 1635922: S2

:bwc, could you consider increasing the severity of this bug to S2?

For more information, please visit auto_nag documentation.

Comment 27

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/jobs?repo=try&revision=da54a14eba1fcedaf7cf16ab59f585b23f128f0f
https://treeherder.mozilla.org/jobs?repo=try&revision=35dc6dbdd70b6834e84bc10b30695eff0dd2ce0c

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0dba91984a39
Re-enable wpt for RTCPeerConnectionState. r=jib
https://hg.mozilla.org/integration/autoland/rev/56f00875890b
Implement RTCPeerConnectionState. r=jib,webidl,smaug

Comment 29

[Cristian Tuns]

Backed out for causing build bustages on PeerConnectionImpl.cpp

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp:2024:38: error: unused variable 'id' [-Werror=unused-variable]

mochitest log: https://treeherder.mozilla.org/logviewer?job_id=405685822&repo=autoland

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/20130ff69694
Re-enable wpt for RTCPeerConnectionState. r=jib
https://hg.mozilla.org/integration/autoland/rev/3bbf94f50b4e
Implement RTCPeerConnectionState. r=jib,webidl,smaug

Comment 31

[Sandor Molnar]

Backed out for causing wpt failures in

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

Push with failures

Failure log #1 | TEST-UNEXPECTED-TIMEOUT | /webrtc/RTCPeerConnection-remote-track-mute.https.html | pc.close() on one side causes mute events on the other - Test timed out

Failure log #2 | TEST-FAIL | /webrtc/RTCPeerConnection-getStats.https.html | getStats() audio and video validate all mandatory stats - assert_equals: Expect dictionary.transportId to be string expected "string" but got "undefined"

Failure log #3 | TEST-UNEXPECTED-FAIL | /webrtc/RTCPeerConnection-getStats.https.html | getStats() on track associated with RTCRtpReceiver should return stats report containing inbound-rtp stats - assert_unreached: required stats <...>

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3d9a0fe10930
Re-enable wpt for RTCPeerConnectionState. r=jib
https://hg.mozilla.org/integration/autoland/rev/b145b1e8ced6
Implement RTCPeerConnectionState. r=jib,webidl,smaug

Comment 33

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/3d9a0fe10930
https://hg.mozilla.org/mozilla-central/rev/b145b1e8ced6

Comment 34

[Ryan VanderMeulen [:RyanVM]]

Is this something we should call out in the Fx113 relnotes? Please nominate if so.

Comment 35

[Hamish Willee]

FYI MDN docs for this, including the MDN release note can be tracked in https://github.com/mdn/content/issues/26146. Note this is still waiting on browser compatibility data to be reviewed.