1535442 - Pay attention to ufrag when incorporating ICE candidates into SDP

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, enhancement, P2)

Description

[Byron Campen [:bwc]]

The ufrag needs to match, if the candidate has one.

Comment 2

[Byron Campen [:bwc]]

Folded bug 1532822 into this one. This means this bug covers:

• Picking the right description (current or pending) to incorporate the candidate into
• Picking the right media-section to incorporate the candidate into
• "candidate" here includes a=end-of-candidates

Comment 3

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=77f8e1f30934eba49ed7c42fc733ac0e30d6d66a

Comment 4

[Byron Campen [:bwc]]

Attachment: Bug 1535442 - Part 0: Re-enable some test-cases. r=jib

Comment 5

[Byron Campen [:bwc]]

Attachment: Bug 1535442 - Part 1: Pay attention to ufrag when incorporating candidates into remote SDP. r?mjf

Depends on D24274

Comment 6

[Byron Campen [:bwc]]

Attachment: Bug 1535442 - Part 2: Fix some test-cases for addIceCandidate. r?jib

Depends on D24275

Comment 7

[Byron Campen [:bwc]]

Attachment: Bug 1535442 - Part 3: Fire per-transport end-of-candidates signals, with ufrag. r?mjf

Depends on D24276

Comment 8

[Byron Campen [:bwc]]

Attachment: Bug 1535442 - Part 4: Put the ufrag in the right attribute, and handle end-of-candidates. r?jib

Depends on D24277

Comment 9

[Byron Campen [:bwc]]

Attachment: Bug 1535442 - Part 5: Use ufrag when incorporating local candidates into SDP. r?mjf

Depends on D24278

Comment 10

[Byron Campen [:bwc]]

Attachment: Bug 1535442 - Part 6: Don't assume SDP has end-of-candidates and other transport cruft on msections that don't have their own transport. r?jib

Depends on D24279

Comment 11

[Byron Campen [:bwc]]

Attachment: Bug 1535442 - Part 7: Allow empty |candidate| and check that |usernameFragment| is present. r?jib

Depends on D24280

Comment 12

[Byron Campen [:bwc]]

Attachment: Bug 1535442 - Part 8: Don't trickle candidates in the restart/rollback tests. r?jib

Depends on D24281

Comment 13

[Byron Campen [:bwc]]

Attachment: Bug 1535442 - Part 9: Do not re-open RTCP-mux transports on renegotiation. r?mjf

Depends on D24282

Comment 14

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=376f8a11edbc607f069b0378dbd916e77736fef3

Comment 15

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3088b0e9236cbebfe751031b05ecee2c6020614b

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2326906f1874
Part 0: Re-enable some test-cases. r=jib
https://hg.mozilla.org/integration/autoland/rev/124cf9c814eb
Part 1: Pay attention to ufrag when incorporating candidates into remote SDP. r=mjf
https://hg.mozilla.org/integration/autoland/rev/32f8c041f72d
Part 2: Fix some test-cases for addIceCandidate. r=jib
https://hg.mozilla.org/integration/autoland/rev/490575f0e834
Part 3: Fire per-transport end-of-candidates signals, with ufrag. r=mjf
https://hg.mozilla.org/integration/autoland/rev/d31a37ebf804
Part 4: Put the ufrag in the right attribute, and handle end-of-candidates. r=jib
https://hg.mozilla.org/integration/autoland/rev/94781a70cd20
Part 5: Use ufrag when incorporating local candidates into SDP. r=mjf
https://hg.mozilla.org/integration/autoland/rev/53556fae6a9b
Part 6: Don't assume SDP has end-of-candidates and other transport cruft on msections that don't have their own transport. r=jib
https://hg.mozilla.org/integration/autoland/rev/8342491a4e91
Part 7: Allow empty |candidate| and check that |usernameFragment| is present. r=jib
https://hg.mozilla.org/integration/autoland/rev/96249192254a
Part 8: Don't trickle candidates in the restart/rollback tests. r=jib
https://hg.mozilla.org/integration/autoland/rev/fe0c1f8b519b
Part 9: Do not re-open RTCP-mux transports on renegotiation. r=mjf

Comment 17

[Cristina Coroiu [:ccoroiu]]

Backed out 10 changesets (bug 1535442) for mochitest failure at dom/presentation/tests/mochitest/test_presentation_1ua_sender_and_receiver_inproc.htm

Backout: https://hg.mozilla.org/integration/autoland/rev/cebb5fa563f96382ce4036ddd0f53168d9211006

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=236234325&searchStr=linux%2Cdebug%2Cmochitests%2Ctest-linux32%2Fdebug-mochitest-6%2Cm%286%29&revision=fe0c1f8b519b06f6f2ccb3605086c790b28e5a0f

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=236234325&repo=autoland&lineNumber=3282

task 2019-03-26T22:46:21.768Z] 22:46:21 INFO - TEST-PASS | dom/presentation/tests/mochitest/test_presentation_1ua_sender_and_receiver_inproc.html | Receiver: navigator.presentation.defaultRequest should be null.
[task 2019-03-26T22:46:21.769Z] 22:46:21 INFO - Buffered messages finished
[task 2019-03-26T22:46:21.769Z] 22:46:21 INFO - TEST-UNEXPECTED-FAIL | dom/presentation/tests/mochitest/test_presentation_1ua_sender_and_receiver_inproc.html | Receiver: Error occurred when getting the connection: [Exception... "Illegal value" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "<unknown>" data: no]
[task 2019-03-26T22:46:21.770Z] 22:46:21 INFO - SimpleTest.ok@SimpleTest/SimpleTest.js:275:18
[task 2019-03-26T22:46:21.770Z] 22:46:21 INFO - receiverListener@dom/presentation/tests/mochitest/test_presentation_1ua_sender_and_receiver.js:59:9
[task 2019-03-26T22:46:21.771Z] 22:46:21 INFO - GECKO(1629) | JavaScript error: , line 0: uncaught exception: undefined
[task 2019-03-26T22:46:21.772Z] 22:46:21 INFO - GECKO(1629) | [Parent 1629: Main Thread]: E/signaling [main|PeerConnectionImpl] PeerConnectionImpl.cpp:2718: GetStats: Found no pipelines matching selector.
[task 2019-03-26T22:46:22.339Z] 22:46:22 INFO - GECKO(1629) | --DOMWINDOW == 38 (0xdbbe2780) [pid = 1629] [serial = 15] [outer = (nil)] [url = moz-extension://b521006c-b65a-4149-be31-ff346c7e1401/_generated_background_page.html]
[task 2019-03-26T22:46:22.340Z] 22:46:22 INFO - GECKO(1629) | --DOCSHELL 0xda087800 == 13 [pid = 1629] [id = {e323300a-a2f1-41db-90fa-75515963a7c2}] [url = moz-extension://b521006c-b65a-4149-be31-ff346c7e1401/_generated_background_page.html]
[task 2019-03-26T22:46:22.691Z] 22:46:22 INFO - GECKO(1629) | [Parent 1629: Main Thread]: E/signaling [main|PeerConnectionImpl] PeerConnectionImpl.cpp:2718: GetStats: Found no pipelines matching selector.
[task 2019-03-26T22:46:23.696Z] 22:46:23 INFO - GECKO(1629) | [Parent 1629: Main Thread]: E/signaling [main|PeerConnectionImpl] PeerConnectionImpl.cpp:2718: GetStats: Found no pipelines matching selector.
[task 2019-03-26T22:46:24.689Z] 22:46:24 INFO - GECKO(1629) | [Parent 1629: Main Thread]: E/signaling [main|PeerConnectionImpl] PeerConnectionImpl.cpp:2718: GetStats: Found no pipelines matching selector.
[task 2019-03-26T22:46:25.597Z] 22:46:25 INFO - GECKO(1629) | [Parent 1629, StreamTrans #5] WARNING: 'NS_FAILED(rv)', file /builds/worker/workspace/build/src/modules/libjar/nsJARChannel.cpp, line 371
[task 2019-03-26T22:46:25.599Z] 22:46:25 INFO - GECKO(1629) | [Parent 1629, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file /builds/worker/workspace/build/src/modules/libjar/nsJARChannel.cpp, line 994
[task 2019-03-26T22:46:25.696Z] 22:46:25 INFO - GECKO(1629) | [Parent 1629: Main Thread]: E/signaling [main|PeerConnectionImpl] PeerConnectionImpl.cpp:2718: GetStats: Found no pipelines matching selector.
[task 2019-03-26T22:46:26.158Z] 22:46:26 INFO - GECKO(1629) | (ice/INFO) ICE(PC:1553640380685084 (id=44 url=http://mochi.test:8888/tests/dom/presentation/tests/mochitest/test_presentation_1ua_sender_and_rece): peer (PC:1553640380685084 (id=44 url=http://mochi.test:8888/tests/dom/presentation/tests/mochitest/test_presentation_1ua_sender_and_rece:default) Trickle grace period is over; marking every component with only failed pairs as failed.
[task 2019-03-26T22:46:26.402Z] 22:46:26 INFO - GECKO(1629) | Timecard created 1553640380.983709
[task 2019-03-26T22:46:26.403Z] 22:46:26 INFO - GECKO(1629) | Timestamp | Delta | Event | File | Function
[task 2019-03-26T22:46:26.406Z] 22:46:26 INFO - GECKO(1629) | ======================================================================================================================
[task 2019-03-26T22:46:26.407Z] 22:46:26 INFO - GECKO(1629) | 0.000036 | 0.000036 | Constructor Completed | PeerConnectionImpl.cpp:338 | PeerConnectionImpl
[task 2019-03-26T22:46:26.408Z] 22:46:26 INFO - GECKO(1629) | 0.002294 | 0.002258 | Initializing PC Ctx | PeerConnectionImpl.cpp:465 | Initialize
[task 2019-03-26T22:46:26.408Z] 22:46:26 INFO - GECKO(1629) | 0.014631 | 0.012337 | Set Remote Description | PeerConnectionImpl.cpp:1445 | SetRemoteDescription
[task 2019-03-26T22:46:26.411Z] 22:46:26 INFO - GECKO(1629) | 0.084403 | 0.069772 | Create Answer | PeerConnectionImpl.cpp:1296 | CreateAnswer
[task 2019-03-26T22:46:26.412Z] 22:46:26 INFO - GECKO(1629) | 0.085511 | 0.001108 | Add Ice Candidate | PeerConnectionImpl.cpp:1620 | AddIceCandidate
[task 2019-03-26T22:46:26.413Z] 22:46:26 INFO - GECKO(1629) | 0.088339 | 0.002828 | Add Ice Candidate | PeerConnectionImpl.cpp:1620 | AddIceCandidate
[task 2019-03-26T22:46:26.414Z] 22:46:26 INFO - GECKO(1629) | 0.089655 | 0.001316 | Add Ice Candidate | PeerConnectionImpl.cpp:1620 | AddIceCandidate
[task 2019-03-26T22:46:26.416Z] 22:46:26 INFO - GECKO(1629) | 0.090317 | 0.000662 | Add Ice Candidate | PeerConnectionImpl.cpp:1620 | AddIceCandidate
[task 2019-03-26T22:46:26.417Z] 22:46:26 INFO - GECKO(1629) | 0.090951 | 0.000634 | Add Ice Candidate | PeerConnectionImpl.cpp:1620 | AddIceCandidate
[task 2019-03-26T22:46:26.419Z] 22:46:26 INFO - GECKO(1629) | 0.091606 | 0.000655 | Add Ice Candidate | PeerConnectionImpl.cpp:1620 | AddIceCandidate
[task 2019-03-26T22:46:26.419Z] 22:46:26 INFO - GECKO(1629) | 0.092291 | 0.000685 | Add Ice Candidate | PeerConnectionImpl.cpp:1620 | AddIceCandidate
[task 2019-03-26T22:46:26.420Z] 22:46:26 INFO - GECKO(1629) | 0.093681 | 0.001390 | Set Local Description | PeerConnectionImpl.cpp:1342 | SetLocalDescription
[task 2019-03-26T22:46:26.420Z] 22:46:26 INFO - GECKO(1629) | 0.135821 | 0.042140 | Ice gathering state: gathering | PeerConnectionImpl.cpp:2671 | IceGatheringStateChange
[task 2019-03-26T22:46:26.420Z] 22:46:26 INFO - GECKO(1629) | 0.137960 | 0.002139 | Ice gathering state: complete | PeerConnectionImpl.cpp:2674 | IceGatheringStateChange
[task 2019-03-26T22:46:26.420Z] 22:46:26 INFO - GECKO(1629) | 0.147719 | 0.009759 | Ice state: checking | PeerConnectionImpl.cpp:2626 | IceConnectionStateChange
[task 2019-03-26T22:46:26.420Z] 22:46:26 INFO - GECKO(1629) | 0.263659 | 0.115940 | Add Ice Candidate | PeerConnectionImpl.cpp:1620 | AddIceCandidate
[task 2019-03-26T22:46:26.420Z] 22:46:26 INFO - GECKO(1629) | 5.415261 | 5.151602 | Destructor Invoked | PeerConnectionImpl.cpp:347 | ~PeerConnectionImpl
[task 2019-03-26T22:46:26.420Z] 22:46:26 INFO - GECKO(1629) | [Parent 1629: Main Thread]: I/signaling [main|PeerConnectionImpl] PeerConnectionImpl.cpp:372: ~PeerConnectionImpl: PeerConnectionImpl destructor invoked for 48d3ec072fa8607e
[task 2019-03-26T22:46:26.697Z] 22:46:26 INFO - GECKO(1629) | [Parent 1629: Main Thread]: E/signaling [main|PeerConnectionImpl] PeerConnectionImpl.cpp:2718: GetStats: Found no pipelines matching selector.
[task 2019-03-26T22:46:27.687Z] 22:46:27 INFO - GECKO(1629) | [Parent 1629: Main Thread]: E/signaling [main|PeerConnectionImpl] PeerConnectionImpl.cpp:2718: GetStats: Found no pipelines matching selector.

Comment 18

[Byron Campen [:bwc]]

That's a new one. Apparently there's a Presentation API that uses RTCPeerConnection?

Comment 19

[Byron Campen [:bwc]]

And this Presentation API is calling addIceCandidate on the wrong PeerConnection. Lovely.

Comment 20

[Byron Campen [:bwc]]

Attachment: Bug 1535442 - Part 10: Fix bug in presentation API mochitest where ICE candidates were being sent to the wrong PC.

Depends on D24523

Comment 21

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8faa89f3b7a93f76cc42a534402f376b1bed367c

Comment 22

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16143 for changes under testing/web-platform/tests

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f9b7a9f2a8d0
Part 0: Re-enable some test-cases. r=jib
https://hg.mozilla.org/integration/autoland/rev/fe3d1118979c
Part 1: Pay attention to ufrag when incorporating candidates into remote SDP. r=mjf
https://hg.mozilla.org/integration/autoland/rev/2ac346fc1029
Part 2: Fix some test-cases for addIceCandidate. r=jib
https://hg.mozilla.org/integration/autoland/rev/37a419113a39
Part 3: Fire per-transport end-of-candidates signals, with ufrag. r=mjf
https://hg.mozilla.org/integration/autoland/rev/3322742f14d5
Part 4: Put the ufrag in the right attribute, and handle end-of-candidates. r=jib
https://hg.mozilla.org/integration/autoland/rev/dcba91554b51
Part 5: Use ufrag when incorporating local candidates into SDP. r=mjf
https://hg.mozilla.org/integration/autoland/rev/8acda96bbd54
Part 6: Don't assume SDP has end-of-candidates and other transport cruft on msections that don't have their own transport. r=jib
https://hg.mozilla.org/integration/autoland/rev/f005ceaf83bd
Part 7: Allow empty |candidate| and check that |usernameFragment| is present. r=jib
https://hg.mozilla.org/integration/autoland/rev/57e67666b450
Part 8: Don't trickle candidates in the restart/rollback tests. r=jib
https://hg.mozilla.org/integration/autoland/rev/1b43b2ac0ad0
Part 9: Do not re-open RTCP-mux transports on renegotiation. r=mjf
https://hg.mozilla.org/integration/autoland/rev/4cfdc4abb3f0
Part 10: Fix bug in presentation API mochitest where ICE candidates were being sent to the wrong PC. r=smaug

Comment 24

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/f9b7a9f2a8d0
https://hg.mozilla.org/mozilla-central/rev/fe3d1118979c
https://hg.mozilla.org/mozilla-central/rev/2ac346fc1029
https://hg.mozilla.org/mozilla-central/rev/37a419113a39
https://hg.mozilla.org/mozilla-central/rev/3322742f14d5
https://hg.mozilla.org/mozilla-central/rev/dcba91554b51
https://hg.mozilla.org/mozilla-central/rev/8acda96bbd54
https://hg.mozilla.org/mozilla-central/rev/f005ceaf83bd
https://hg.mozilla.org/mozilla-central/rev/57e67666b450
https://hg.mozilla.org/mozilla-central/rev/1b43b2ac0ad0
https://hg.mozilla.org/mozilla-central/rev/4cfdc4abb3f0

Comment 25

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging