Bug 235167
| Summary: | MediaStreamTrack endedEvent fired when RTCPeerConnection connectionState is already RTCPeerConnectionState::Closed | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Gabriel Nava Marino <gnavamarino> |
| Component: | WebRTC | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW | ||
| Severity: | Normal | CC: | webkit-bug-importer, youennf |
| Priority: | P2 | Keywords: | InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Gabriel Nava Marino
This bug is to investigate whether the current RTCPeerConnection follows from the W3 recommendation mentioned in https://www.w3.org/TR/webrtc/#garbage-collection.
"An RTCPeerConnection object MUST not be garbage collected as long as any event can cause an event handler to be triggered on the object. When the object's [[IsClosed]] internal slot is true, no such event handler can be triggered and it is therefore safe to garbage collect the object."
Bug 235165 shows there are some scenarios that allow the event handler to be triggered on endedEvent, after the RTCPeerConnection "object's [[IsClosed]] internal slot is true".
More specifically, this can happen when stopping RTCPeerConnection from the destructor. In this case, RTCPeerConnection::doClose() sets the connectionState to RTCPeerConnectionState::Closed, but without preventing the endedEvent from firing while stopping the RTCRtpReceiver (RTCRtpTransceiver).
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
youenn fablet
I think all browsers do fire ended events.
I am guessing we should either update the spec or implementations.
youenn fablet
Ah, nevermind, browsers do fire events when close is called.
I think they do not when their context goes away as the dom object would be stopped.
It would be good to validate that other browsers are not firing such events in case of GCing the peer connection.
Gabriel Nava Marino
Per Youenn's recommendation in the related radar:
We should probably update MediaStreamTrack::trackEnded to queue a task to fire the event instead of firing the event right away.
This matches https://w3c.github.io/mediacapture-main/#track-ended.
This would allow to remove the activeDOMObjectsAreSuspended/activeDOMObjectsAreStopped checks in that method.
It would also be good to write a corresponding test. internals.simulateMediaStreamTrackCaptureSourceFailure maybe be able to help validate that the event is not fired synchronously.
youenn fablet
Will implement "queue a task to fire ended event" in bug 235227.
Radar WebKit Bug Importer
<rdar://problem/87800182>