RESOLVED FIXED 174314
RTCPeerConnection's close method should update signalingState
https://bugs.webkit.org/show_bug.cgi?id=174314
Summary RTCPeerConnection's close method should update signalingState
Mark Roberts
Reported 2017-07-10 10:37:50 PDT
Run the following code: (async () => { console.log('1. Constructing an RTCPeerConnection') const pc = new RTCPeerConnection() const signalingStateChangeEvent = new Promise(resolve => pc.onsignalingstatechange = resolve) console.log('2. Closing the RTCPeerConnection') pc.close() if (pc.signalingState !== 'closed') { console.error(`Expected the RTCPeerConnection's signaling state to be "closed", not "${pc.signalingState}"`) } console.log('3. Awaiting a "signalingstatechange" event') await signalingStateChangeEvent console.log('Success!') })() Expected output (Chrome, Firefox): 1. Constructing an RTCPeerConnection 2. Closing the RTCPeerConnection 3. Awaiting a "signalingstatechange" event Success! Actual output (Safari): 1. Constructing an RTCPeerConnection 2. Closing the RTCPeerConnection Expected the RTCPeerConnection's signaling state to be "closed", not "stable" 3. Awaiting a "signalingstatechange" event In the actual output, "Success!" never prints because `signalingStateChangeEvent` never resolves. I guess the behavior around this was only recently specified here? https://github.com/w3c/webrtc-pc/pull/1371
Attachments
Patch (4.51 KB, patch)
2018-03-12 10:52 PDT, youenn fablet
no flags
Mark Roberts
Comment 1 2017-07-10 11:01:35 PDT
RTCPeerConnection's `close` method should also update `iceConnectionState`. Perhaps this issue could be tracked here, too, instead of a separate ticket. Run the following code: (async () => { console.log('1. Constructing an RTCPeerConnection') const pc = new RTCPeerConnection() const iceConnectionStateChangeEvent = new Promise(resolve => pc.oniceconnectionstatechange = resolve) console.log('2. Closing the RTCPeerConnection') pc.close() if (pc.iceConnectionState !== 'closed') { console.error(`Expected the RTCPeerConnection's ICE connection state to be "closed", not "${pc.iceConnectionState}"`) } console.log('3. Awaiting a "iceconnectionstatechange" event') await iceConnectionStateChangeEvent console.log('Success!') })() Expected output (Firefox): 1. Constructing an RTCPeerConnection 2. Closing the RTCPeerConnection Expected the RTCPeerConnection's ICE connection state to be "closed", not "new" 3. Awaiting a "iceconnectionstatechange" event Success! Actual output (Safari): 1. Constructing an RTCPeerConnection 2. Closing the RTCPeerConnection Expected the RTCPeerConnection's ICE connection state to be "closed", not "new" 3. Awaiting a "iceconnectionstatechange" event In the actual output, "Success!" never prints because `iceConnectionStateChangeEvent` never resolves. This behavior seems specified in the Step 4 of `close` and the description of "closed" in RTCIceConnectionState: * http://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-close * http://w3c.github.io/webrtc-pc/#rtciceconnectionstate-enum (Note: In Chrome, the error prints, too, I guess because `iceConnectionState` isn't updated synchronously?)
Radar WebKit Bug Importer
Comment 2 2017-07-12 10:40:53 PDT
youenn fablet
Comment 3 2018-03-12 10:49:58 PDT
Thanks Mark, it is true we are missing the closed state. I will add it. Looking at the spec, it does not seem to mandate firing an event though. Maybe this is my reading of the spec.
youenn fablet
Comment 4 2018-03-12 10:50:16 PDT
Testing further, you are right that Chrome and Firefox are firing this event.
youenn fablet
Comment 5 2018-03-12 10:52:07 PDT
youenn fablet
Comment 6 2018-03-12 10:57:51 PDT
Filed https://github.com/w3c/webrtc-pc/issues/1799 for clarification about the event part of the bug.
WebKit Commit Bot
Comment 7 2018-03-12 14:16:39 PDT
Comment on attachment 335605 [details] Patch Clearing flags on attachment: 335605 Committed r229548: <https://trac.webkit.org/changeset/229548>
WebKit Commit Bot
Comment 8 2018-03-12 14:16:40 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.