Closed Bug 1265827 Opened 8 years ago Closed 1 year ago

Add support for RTCPeerConnectionState in RTCPeerConnection

Categories

(Core :: WebRTC, defect, P2)

45 Branch
defect

Tracking

()

RESOLVED FIXED
113 Branch
Tracking Status
firefox113 --- fixed

People

(Reporter: rajmohanbanavi, Assigned: bwc)

References

(Blocks 3 open bugs)

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 1 obsolete file)

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.
Rank: 29
Priority: -- → P2
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.
There has been no update to this bug in a few months.
Can this be prioritized please?
Thanks!
Any update on this?
Blocks: webrtc_spec
Rank: 29 → 19
Priority: P2 → P1
Mass change P1->P2 to align with new Mozilla triage process
Priority: P1 → P2
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
Priority: P2 → P3
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
Chrome just landed connectionState in Chrome 72.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.

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

Blocks: 1533020
Priority: P3 → P2

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:

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 :)

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

Assignee: nobody → rudi.floren
Status: NEW → ASSIGNED
Attachment #9256529 - Attachment description: WIP: Bug 1265827 - Add connectionState to RTCPeerConnection. r?ng → Bug 1265827 - Add connectionState to RTCPeerConnection. r?ng
Attachment #9256529 - Attachment description: Bug 1265827 - Add connectionState to RTCPeerConnection. r?ng → Bug 1265827 - Add connectionState to RTCPeerConnection. r=jib

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

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.

Assignee: rudi.floren → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(mfroman)

(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?

Flags: needinfo?(mfroman) → needinfo?(jib)

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?

Flags: needinfo?(jib) → needinfo?(docfaraday)

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

Assignee: nobody → docfaraday
Flags: needinfo?(docfaraday)
Severity: normal → S3

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

Depends on: 1307994
Depends on: 1811912
No longer depends on: 1307994
No longer depends on: 1811912

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.

Attachment #9256529 - Attachment is obsolete: true

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

Depends on D168142

Blocks: 1813147

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.

See Also: → 1643050
Duplicate of this bug: 1635922

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

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

For more information, please visit auto_nag documentation.

Flags: needinfo?(docfaraday)
Flags: needinfo?(docfaraday)
Pushed by bcampen@mozilla.com:
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

Backed out for causing build bustages on PeerConnectionImpl.cpp

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

Flags: needinfo?(docfaraday)
Pushed by bcampen@mozilla.com:
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
Pushed by bcampen@mozilla.com:
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
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 113 Branch
Flags: needinfo?(docfaraday)

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

Flags: needinfo?(docfaraday)
Flags: needinfo?(docfaraday)

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.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: