1157766 - Firefox 37 only supports 16 datachannels

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P1)

Description

[Ben Schwartz]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2369.0 Safari/537.36

Steps to reproduce:

Go to http://mozilla.github.io/webrtc-landing/data_test.html
Click Start!
Open the console
pc2.ondatachannel = function(e) { console.log("ondatachannel with label", e.channel.label); };
pc1.createDataChannel("1");
> "ondatachannel with label" "1"
pc1.createDataChannel("2")
> "ondatachannel with label" "2"
pc1.createDataChannel("3")
> "ondatachannel with label" "3"
pc1.createDataChannel("4")
> "ondatachannel with label" "4"
pc1.createDataChannel("5")
> "ondatachannel with label" "5"
pc1.createDataChannel("6")
> "ondatachannel with label" "6"
pc1.createDataChannel("7")
> "ondatachannel with label" "7"
pc1.createDataChannel("8")
pc1.createDataChannel("9")

The receiver never gets channels 8 or 9.


Actual results:

Only the first 7 channels are received


Expected results:

All channels received.

Comment 1

[[:Aleksej]]

firefox-37.0.2.en-US.linux64
2015-04-23-03-02-04-mozilla-central-firefox-40.0a1.ru.linux-x86_64

Reproduced as described, according to the output on the page.

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Attachment: mismatched DataChannel initial channel size in JSEP database breaks adding channels

Comment 3

[Byron Campen [:bwc]]

Comment on attachment 8596775 [details] [diff] [review]
mismatched DataChannel initial channel size in JSEP database breaks adding channels

Review of attachment 8596775 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Regressed in 37; 36 is good.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/89f436c87a2f

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

[Tracking Requested - why for this release]:
Recent regression; affects some applications are badly affected

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8596775 [details] [diff] [review]
mismatched DataChannel initial channel size in JSEP database breaks adding channels

Approval Request Comment
[Feature/regressing bug #]: bug 1079729 (probably, or perhaps that plus the JSEP signaling rewrite in 37)

[User impact if declined]: Inability to create more than 8 datachannels from each end (16 total)

[Describe test coverage new/current, TreeHerder]: Manual testing via the data_test.html page.

[Risks and why]: Very low risk; risk is almost solely that expansion isn't tested often (filed bug for that; should have coverage soon).  Alternatively we could effectively land the 36 patch on 38 (to up the limit to 256 and not allow increases), and then take this on 40 and 39.  Or more to the point, land this (to default to 256 always) and add a simple error return in place of requests to increase the number (i.e. the 36 patch).

[String/UUID change made/needed]: none

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

> Approval Request Comment
> [Feature/regressing bug #]: bug 1079729 (probably, or perhaps that plus the
> JSEP signaling rewrite in 37)

I checked, the bug crept in during the rewrite of the signaling for 37 (constant change landed after code was copied to form the basis of the JSEP implementation I imagine).

> [Risks and why]: Very low risk; risk is almost solely that expansion isn't
> tested often (filed bug for that; should have coverage soon).  Alternatively
> we could effectively land the 36 patch on 38 (to up the limit to 256 and not
> allow increases), and then take this on 40 and 39.  Or more to the point,
> land this (to default to 256 always) and add a simple error return in place
> of requests to increase the number (i.e. the 36 patch).

Actually the 36 patch did leave expansion working, but upped the default to 256 from 16.  This means 36/35/34 had this fixed and with working expansion, and some developers were hitting it (which is how bug 1079729 was filed).  This patch simply restores that behavior.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/89f436c87a2f

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8596775 [details] [diff] [review]
mismatched DataChannel initial channel size in JSEP database breaks adding channels

[Triage Comment]
Should be in 38 beta 8

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8c45feceeee4

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/a8fb9422ff13

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a8fb9422ff13

Comment 14

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Reproduced the original issue (limited number of channels received) using Firefox 38 Beta 8. The issue no longer reproduces as the number of channels is no longer limited, when using Firefox 38 Beta 9, on Windows 7 x64, Mac OS X 10.10, Ubuntu 14.04 x64.