1365081 - Data channel fails to open if maxPacketLifeTime is set

Status: RESOLVED FIXED

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

Description

[Nils Ohlmeier [:drno]]

If maxPacketLifeTime is set to a non zero value in the DataChannel dict the data channel fails to open as reported here: https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/discuss-webrtc/PPY8164HLus/NIpU0xPSAQAJ

A run with logs shows this:

2017-05-15 22:46:33.986490 UTC - [Socket Thread]: D/DataChannel Association change: SCTP_COMM_UP
2017-05-15 22:46:33.986492 UTC - [Socket Thread]: D/DataChannel DTLS connect() succeeded!  Entering connected mode
2017-05-15 22:46:33.986494 UTC - [Socket Thread]: D/DataChannel Processing queued open for 0x11e765f00 (65535)
2017-05-15 22:46:33.986496 UTC - [Socket Thread]: D/DataChannel SendOpenRequest failed, errno = 0
2017-05-15 22:46:33.986497 UTC - [Socket Thread]: D/DataChannel Association change: streams (in/out) = (256/256)
2017-05-15 22:46:33.986498 UTC - [Socket Thread]: D/DataChannel Supports: PR
2017-05-15 22:46:33.986499 UTC - [Socket Thread]: D/DataChannel Supports: AUTH
2017-05-15 22:46:33.986500 UTC - [Socket Thread]: D/DataChannel Supports: ASCONF
2017-05-15 22:46:33.986501 UTC - [Socket Thread]: D/DataChannel Supports: MULTIBUF
2017-05-15 22:46:33.986502 UTC - [Socket Thread]: D/DataChannel Supports: RE-CONFIG

Which points to here: http://searchfox.org/mozilla-central/rev/ae24a3c83d22e0e35aecfd9049c2b463ca7e045b/netwerk/sctp/datachannel/DataChannel.cpp#2113

Comment 1

[Nils Ohlmeier [:drno]]

So we hit this: http://searchfox.org/mozilla-central/rev/ae24a3c83d22e0e35aecfd9049c2b463ca7e045b/netwerk/sctp/datachannel/DataChannel.cpp#981
with a value of 2.

Comment 2

[Nils Ohlmeier [:drno]]

Attachment: Bug 1365081: really use prPolicy as the pocily input value.

Review commit: https://reviewboard.mozilla.org/r/139510/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139510/

Comment 3

[Nils Ohlmeier [:drno]]

Attachment: Bug 1365081: added data channel test function for maxPacketLifeTime.

Review commit: https://reviewboard.mozilla.org/r/139512/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139512/

Comment 4

[Nils Ohlmeier [:drno]]

Looks like the change in the webidl in bug 1319542 changed the numbers in |type|, which then revealed this long standing bug.

Comment 5

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

Comment on attachment 8867974 [details]
Bug 1365081: really use prPolicy as the pocily input value.

https://reviewboard.mozilla.org/r/139510/#review142894

Comment 6

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

Comment on attachment 8867975 [details]
Bug 1365081: added data channel test function for maxPacketLifeTime.

https://reviewboard.mozilla.org/r/139512/#review142896

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0bc24db05046
really use prPolicy as the pocily input value. r=jesup
https://hg.mozilla.org/integration/autoland/rev/d559a766a802
added data channel test function for maxPacketLifeTime. r=jesup

Comment 8

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/0bc24db05046
https://hg.mozilla.org/mozilla-central/rev/d559a766a802

Comment 9

[Ionuț Goldan [:igoldan]]

Hello, :drno. Did this change affected any kind of browser startup functionality? If you look over this graph, there is an extra regression after the previous ones:

https://treeherder.mozilla.org/perf.html#/graphs?series=%5Bmozilla-inbound,c22155824871783bb5d8f514b3a01b4c885034c0,0,1%5D&series=%5Bautoland,c22155824871783bb5d8f514b3a01b4c885034c0,1,1%5D&highlightedRevisions=d559a766a802&highlightAlerts=0&zoom=1495031165361.8843,1495097946000,800,1347.8260869565217

An answer would help my investigation.

Comment 10

[Nils Ohlmeier [:drno]]

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #9)
> Hello, :drno. Did this change affected any kind of browser startup
> functionality? If you look over this graph, there is an extra regression
> after the previous ones:
> 
> https://treeherder.mozilla.org/perf.html#/graphs?series=%5Bmozilla-inbound,
> c22155824871783bb5d8f514b3a01b4c885034c0,0,1%5D&series=%5Bautoland,
> c22155824871783bb5d8f514b3a01b4c885034c0,1,
> 1%5D&highlightedRevisions=d559a766a802&highlightAlerts=0&zoom=1495031165361.
> 8843,1495097946000,800,1347.8260869565217
> 
> An answer would help my investigation.

No the change should only affect any tests in dom/media/tests/mochitest/test_dataChannel*. These tests should be running in the mda2 block. It might also affect some web platform tests. But nothing regarding startup or session restore.