1800136 - RTCRtpSender.getStats() does not get any ICE stats

Status: RESOLVED FIXED

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

Description

[Byron Campen [:bwc]]

Just noticed this while looking at code. RTCRtpReceiver has this code block:

https://searchfox.org/mozilla-central/rev/dd216c1307a2bf1b0d2465b9749fa86dac44303a/dom/media/webrtc/jsapi/RTCRtpReceiver.cpp#517-521

But RTCRtpSender does not have anything comparable! This is pretty broken. I'm kinda shocked we have not seen a bug report for this.

P3/S3 since there's an easy workaround (RTCPeerConnection::GetStats), but still.

Comment 1

[Byron Campen [:bwc]]

Attachment: Bug 1800136: Test-cases for ICE candidate stats. r?jib

Comment 2

[Byron Campen [:bwc]]

Attachment: Bug 1800136: Make RTCRtpSender::GetStats get ICE stats. r?ng

Depends on D162321

Comment 3

[Byron Campen [:bwc]]

Try looks good:

https://treeherder.mozilla.org/jobs?repo=try&revision=9f8198368f33834cfa9edf35cf8c36c98c30dd5b
https://treeherder.mozilla.org/jobs?repo=try&revision=b1a7948df4cd5cd85fc2987129e24a1207a69133

Comment 4

[Byron Campen [:bwc]]

Needed a little work:

https://treeherder.mozilla.org/jobs?repo=try&revision=d60d78f20f1ebc771719277b3cfaf7c9f1a3a522
https://treeherder.mozilla.org/jobs?repo=try&revision=ea4a7eef46708dc9bc5906eb7ee29bbab6426875

Comment 5

[Byron Campen [:bwc]]

Attachment: Bug 1800136: Add tests for duplicate stats ids in WebrtcGlobalInformation. r?ng

Depends on D162321

Comment 6

[Byron Campen [:bwc]]

Attachment: Bug 1800136: Fix gap between PC.close() and the storage of its final stats in PeerConnectionCtx. r?ng

Depends on D162322

Comment 7

[Byron Campen [:bwc]]

Found another race in a test-case:

https://treeherder.mozilla.org/jobs?repo=try&revision=213b48b708106d44a5646ad4c7e78a84fb2cd4c9
https://treeherder.mozilla.org/jobs?repo=try&revision=d352d158a84fbd4c97f9eac53e9260feb6901f88

Comment 8

[Byron Campen [:bwc]]

Try looks fine now:

https://treeherder.mozilla.org/jobs?repo=try&revision=26b922ee33bb0c68c58fc5d965e146f49d196006
https://treeherder.mozilla.org/jobs?repo=try&revision=dd71bfac539e80469d62fba90bcbe944cd088842

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e09b3e06b10
Test-cases for ICE candidate stats. r=jib
https://hg.mozilla.org/integration/autoland/rev/d2ace48b830c
Add tests for duplicate stats ids in WebrtcGlobalInformation. r=ng
https://hg.mozilla.org/integration/autoland/rev/3621f5bf1cb0
Make RTCRtpSender::GetStats get ICE stats. r=ng
https://hg.mozilla.org/integration/autoland/rev/fb1a765ec213
Fix gap between PC.close() and the storage of its final stats in PeerConnectionCtx. r=ng

Comment 10

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

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

Comment 11

[Atila Butkovits]

Backed out for causing build bustages at PeerConnectionCtx.cpp.

Backout link: https://hg.mozilla.org/integration/autoland/rev/4606f9c0720cba01d9fa3561c8b23c896ee62e89

Push where failures started: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=HddZvbjhR7G5vwyAROfy1w.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=1931d3d21dc938214dc0d14ecb7ab82981f209b7

Failure log: https://treeherder.mozilla.org/logviewer?job_id=400334988&repo=autoland&lineNumber=10712

Comment 12

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

Upstream PR was closed without merging

Comment 13

[Byron Campen [:bwc]]

Backed out because a subsequent tooling push was unhappy with the new code? Of all the bad luck...

Comment 14

[Byron Campen [:bwc]]

Try looks fine:

https://treeherder.mozilla.org/jobs?repo=try&revision=e7a2c9b3cff515b561b75a87b1e7bc99220db280

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a80074bc3200
Test-cases for ICE candidate stats. r=jib
https://hg.mozilla.org/integration/autoland/rev/5801060dd3d0
Add tests for duplicate stats ids in WebrtcGlobalInformation. r=ng
https://hg.mozilla.org/integration/autoland/rev/99ea4f221834
Make RTCRtpSender::GetStats get ICE stats. r=ng
https://hg.mozilla.org/integration/autoland/rev/fe152aff2a22
Fix gap between PC.close() and the storage of its final stats in PeerConnectionCtx. r=ng

Comment 16

[Cristian Tuns]

Backed out for causing build bustages

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/dom/media/webrtc/jsapi/PeerConnectionCtx.cpp:502:21: error: unused variable 'id' [-Werror=unused-variable]

Comment 17

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

Upstream PR was closed without merging

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a10a84084cc3
Test-cases for ICE candidate stats. r=jib
https://hg.mozilla.org/integration/autoland/rev/ab6eb1cfa9a0
Add tests for duplicate stats ids in WebrtcGlobalInformation. r=ng
https://hg.mozilla.org/integration/autoland/rev/fef019ffca5f
Make RTCRtpSender::GetStats get ICE stats. r=ng
https://hg.mozilla.org/integration/autoland/rev/3cbde68f2f7d
Fix gap between PC.close() and the storage of its final stats in PeerConnectionCtx. r=ng

Comment 19

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/a10a84084cc3
https://hg.mozilla.org/mozilla-central/rev/ab6eb1cfa9a0
https://hg.mozilla.org/mozilla-central/rev/fef019ffca5f
https://hg.mozilla.org/mozilla-central/rev/3cbde68f2f7d

Comment 20

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

Upstream PR was closed without merging