RTCDataChannel.id logic needs an overhaul

RESOLVED FIXED in Firefox 69

Status

()

defect
P2
normal
RESOLVED FIXED
2 months ago
9 days ago

People

(Reporter: bwc, Assigned: bwc)

Tracking

unspecified
mozilla69
Points:
---

Firefox Tracking Flags

(firefox69 fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

Our implementation of DataChannel only assigns ids after the stream id limit has been successfully negotiated in SCTP. The webrtc-pc spec says to set these immediately, and make a best effort to negotiate the stream limit so they can be used. We need to update our code to implement this best-effort approach.

Given that this difference leads to lots of buggy behavior, I think I'm going to call this a defect.

Type: task → defect
Attachment #9069963 - Attachment is obsolete: true
Pushed by bcampen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/112937d690e0
Re-enable some web-platform-tests related to DataChannel ids. r=jib
https://hg.mozilla.org/integration/autoland/rev/f8ce8930bcab
Allocate stream ids as soon as client/server is negotiated, and try to negotiate id limit increases after. r=ng
https://hg.mozilla.org/integration/autoland/rev/a87072937e2c
Simplifications related to DataChannel::mFlags. r=ng
https://hg.mozilla.org/integration/autoland/rev/05898e5d5434
Add some thread assertions, and remove an unused member. r=ng

Looks like this collided with 1558851.

Yeah, it looks like the merge didn't go quite right here.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla69 → ---

Oh sorry I was just preparing a patch to fix the metadata, but understandably the sheriffs decided to back out. I think the right patch is something like

diff --git a/testing/web-platform/meta/webrtc/RTCPeerConnection-createDataChannel.html.ini b/testing/web-platform/meta/webrtc/RTCPeerConnection-createDataChannel.html.ini
index b4dd12fbec66..c40c1ddaa60c 100644
--- a/testing/web-platform/meta/webrtc/RTCPeerConnection-createDataChannel.html.ini
+++ b/testing/web-platform/meta/webrtc/RTCPeerConnection-createDataChannel.html.ini
@@ -43,6 +43,3 @@
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1550497
 
-  [Channels created (after setRemoteDescription) should have id assigned]
-    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=797135
-    expected: FAIL

Yeah, I'm rebasing and rebuilding now. Should be able to re-land later today.

Pushed by bcampen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/947f43a8a712
Re-enable some web-platform-tests related to DataChannel ids. r=jib
https://hg.mozilla.org/integration/autoland/rev/64c2df149745
Allocate stream ids as soon as client/server is negotiated, and try to negotiate id limit increases after. r=ng
https://hg.mozilla.org/integration/autoland/rev/ea730cd703a2
Simplifications related to DataChannel::mFlags. r=ng
https://hg.mozilla.org/integration/autoland/rev/fb1e149d91bf
Add some thread assertions, and remove an unused member. r=ng
Regressed by: 1562341
No longer regressed by: 1562341
Regressions: 1562341

I would suggest to wait until https://github.com/w3c/webrtc-pc/pull/2222 has been merged. This should drastically simplify things.

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