1056433 - getStats should be allowed on a closed PeerConnection

Status: ASSIGNED

(Core :: WebRTC, defect, P3)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Change getStats so that calling it on a closed PeerConnection does not throw, but instead makes available the stats at the time the PeerConnection closed.

Use-case: A webpage wants stats about the call that just terminated from the
other side - e.g. how many frames dropped?

Basically we already cache a final call to getStats internally for about:webrtc, so this bug is about returning this to content when getStats is called after close.

Comment 1

[Bulk Bug Changes for mreavy's org]

Mass change P3->P4 to align with new Mozilla triage process.

Comment 2

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

We already take a snapshot of stats at time of close internally, for about:webrtc so this could be low-hanging fruit.

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Steps to reproduce:
1. Open https://jsfiddle.net/jib1/wmae5jtn/ and allow access
2. Click "Close!" button

Expected results:
 - Stats numbers should stop increasing, and fps which should go to 0.

Actual result:
 - TypeError: stats is null

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Here's some newer STRs for this bug that don't require interaction:

1. Open https://jsfiddle.net/jib1/6rzL3asv/18/

Expected result (Chrome):

checking
connected
[object Object]
[object Object]
disconnected
failed

Actual result (Firefox):

checking
connected
[object Object]
undefined
disconnected
failed

Comment 6

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Spec support on preserving stats after transceiver.stop() and pc.close():

"With a few exceptions, monitored objects, once created, exist for the duration of their associated RTCPeerConnection. This ensures statistics from them are available in the result from getStats() even past the associated peer connection being closed.

Only a few monitored objects have shorter lifetimes. Statistics from these objects are no longer available in subsequent getStats() results. The object descriptions in [WEBRTC-STATS] describe when these monitored objects are deleted. "

And

"RTP monitored objects are not deleted."

Comment 7

[Philipp Hancke [:fippo]]

Is there an ETA? I got a request to polyfill this in adapter.js but adding polyfills without an exit criterion is more harm than good:
https://github.com/webrtcHacks/adapter/issues/1179

Comment 8

[Philipp Hancke [:fippo]]

Attachment: Bug 1056433: do not throw when calling getStats on a closed peerconnection

Note: this retains local and remote candidate stats which Chromium does not
and which might violate the specification.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4c0a122007df
https://hg.mozilla.org/integration/autoland/rev/e05d4e4c4ceb
do not throw when calling getStats on a closed peerconnection r=webrtc-reviewers,bwc

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e9328fdda48d
https://hg.mozilla.org/integration/autoland/rev/5386cca723f6
Revert "Bug 1056433: do not throw when calling getStats on a closed peerconnection r=webrtc-reviewers,bwc" for causing mda failures at getStats peer connection

Comment 11

[Silaghi Andreea]

Backed out for causing mda failures at getStats peer connection
Backout Link
Push with failures
Failure Log
Failure line TEST-UNEXPECTED-FAIL | dom/media/webrtc/tests/mochitests/test_peerConnection_bug1773067.html | getStats on closed peer connection fails instead of hanging - got undefined, expected "InvalidStateError"