From 1937a04bf6b3c183d0fcd53e552fe9ecb969119b Mon Sep 17 00:00:00 2001 From: Soares Chen Date: Sun, 4 Nov 2018 15:14:08 +0700 Subject: [PATCH] Perform safe addIceCandidate together with signaling handshake --- webrtc/RTCDTMFSender-helper.js | 5 +-- ...TCDtlsTransport-getRemoteCertificates.html | 6 +-- webrtc/RTCPeerConnection-connectionState.html | 6 +-- webrtc/RTCPeerConnection-getStats.https.html | 7 +-- webrtc/RTCPeerConnection-helper.js | 45 ++++++++++--------- .../RTCPeerConnection-iceConnectionState.html | 9 ++-- .../RTCPeerConnection-iceGatheringState.html | 7 ++- webrtc/RTCPeerConnection-ondatachannel.html | 18 ++++---- .../RTCPeerConnection-track-stats.https.html | 12 ++--- ...Receiver-getContributingSources.https.html | 6 +-- ...eiver-getSynchronizationSources.https.html | 3 +- 11 files changed, 57 insertions(+), 67 deletions(-) diff --git a/webrtc/RTCDTMFSender-helper.js b/webrtc/RTCDTMFSender-helper.js index 84cc771dda283da..fa8f423f44f54db 100644 --- a/webrtc/RTCDTMFSender-helper.js +++ b/webrtc/RTCDTMFSender-helper.js @@ -8,7 +8,7 @@ // The following helper functions are called from RTCPeerConnection-helper.js: // getTrackFromUserMedia -// doSignalingHandshake +// doSignalingAndCandidateHandshake // Create a RTCDTMFSender using getUserMedia() // Connect the PeerConnection to another PC and wait until it is @@ -25,8 +25,7 @@ function createDtmfSender(pc = new RTCPeerConnection()) { // on whether sending should be possible before negotiation. const pc2 = new RTCPeerConnection(); Object.defineProperty(pc, 'otherPc', { value: pc2 }); - exchangeIceCandidates(pc, pc2); - return doSignalingHandshake(pc, pc2); + return doSignalingAndCandidateHandshake(pc, pc2); }).then(() => { if (!('canInsertDTMF' in dtmfSender)) { return Promise.resolve(); diff --git a/webrtc/RTCDtlsTransport-getRemoteCertificates.html b/webrtc/RTCDtlsTransport-getRemoteCertificates.html index 0614364e9756ec9..34a0170ffb0eedd 100644 --- a/webrtc/RTCDtlsTransport-getRemoteCertificates.html +++ b/webrtc/RTCDtlsTransport-getRemoteCertificates.html @@ -8,8 +8,7 @@ 'use strict'; // The following helper functions are called from RTCPeerConnection-helper.js: - // exchangeIceCandidates - // doSignalingHandshake + // doSignalingAndCandidateHandshake /* 5.5. RTCDtlsTransport Interface @@ -43,9 +42,8 @@ t.add_cleanup(() => pc2.close()); pc1.createDataChannel('test'); - exchangeIceCandidates(pc1, pc2); - doSignalingHandshake(pc1, pc2) + doSignalingAndCandidateHandshake(pc1, pc2) .then(t.step_func(() => { // pc.sctp is set when set*Description(answer) is called const sctpTransport1 = pc1.sctp; diff --git a/webrtc/RTCPeerConnection-connectionState.html b/webrtc/RTCPeerConnection-connectionState.html index d8e941127181557..eae9932a322fbe7 100644 --- a/webrtc/RTCPeerConnection-connectionState.html +++ b/webrtc/RTCPeerConnection-connectionState.html @@ -11,8 +11,7 @@ // https://w3c.github.io/webrtc-pc/archives/20170605/webrtc.htm // The following helper functions are called from RTCPeerConnection-helper.js: - // exchangeIceCandidates - // doSignalingHandshake + // doSignalingAndCandidateHandshake /* 4.3.2. Interface Definition @@ -137,8 +136,7 @@ pc1.addEventListener('connectionstatechange', onConnectionStateChange); - exchangeIceCandidates(pc1, pc2); - doSignalingHandshake(pc1, pc2); + doSignalingAndCandidateHandshake(pc1, pc2); }, 'connection with one data channel should eventually have connected connection state'); /* diff --git a/webrtc/RTCPeerConnection-getStats.https.html b/webrtc/RTCPeerConnection-getStats.https.html index 247402b83be0a26..c798d77ab802d4d 100644 --- a/webrtc/RTCPeerConnection-getStats.https.html +++ b/webrtc/RTCPeerConnection-getStats.https.html @@ -21,8 +21,7 @@ // assert_stats_report_has_stats // The following helper function is called from RTCPeerConnection-helper.js - // exchangeIceCandidates - // doSignalingHandshake + // doSignalingAndCandidateHandshake /* 8.2. getStats @@ -331,9 +330,7 @@ } })); - - exchangeIceCandidates(pc1, pc2); - doSignalingHandshake(pc1, pc2); + return doSignalingAndCandidateHandshake(pc1, pc2); })) .catch(t.step_func(err => { assert_unreached(`test failed with error: ${err}`); diff --git a/webrtc/RTCPeerConnection-helper.js b/webrtc/RTCPeerConnection-helper.js index 0153247146b164c..1bf4930a0bb023b 100644 --- a/webrtc/RTCPeerConnection-helper.js +++ b/webrtc/RTCPeerConnection-helper.js @@ -201,19 +201,17 @@ function test_never_resolve(testFunc, testName) { }, testName); } -// Helper function to exchange ice candidates between -// two local peer connections -function exchangeIceCandidates(pc1, pc2) { +// Helper function to exchange ice candidates between two local peer connections. +// Accepts an optional handshakePromise to wait for setRemoteDescription() to be +// done before calling addIceCandidate(). +function exchangeIceCandidates(pc1, pc2, handshakePromise) { // private function function doExchange(localPc, remotePc) { - localPc.addEventListener('icecandidate', event => { + localPc.addEventListener('icecandidate', async event => { const { candidate } = event; - // candidate may be null to indicate end of candidate gathering. - // There is ongoing discussion on w3c/webrtc-pc#1213 - // that there should be an empty candidate string event - // for end of candidate for each m= section. if(candidate && remotePc.signalingState !== 'closed') { + await handshakePromise remotePc.addIceCandidate(candidate); } }); @@ -225,15 +223,23 @@ function exchangeIceCandidates(pc1, pc2) { // Helper function for doing one round of offer/answer exchange // betweeen two local peer connections -function doSignalingHandshake(localPc, remotePc) { - return localPc.createOffer() - .then(offer => Promise.all([ - localPc.setLocalDescription(offer), - remotePc.setRemoteDescription(offer)])) - .then(() => remotePc.createAnswer()) - .then(answer => Promise.all([ - remotePc.setLocalDescription(answer), - localPc.setRemoteDescription(answer)])) +async function doSignalingHandshake(localPc, remotePc) { + const offer = await localPc.createOffer(); + await localPc.setLocalDescription(offer); + await remotePc.setRemoteDescription(offer); + + const answer = await remotePc.createAnswer(); + await remotePc.setLocalDescription(answer); + await localPc.setRemoteDescription(answer); +} + +// Helper function to do both signaling handshake and ICE candidates +// exchange. The two tasks are done in parallel in a safe way, with +// care to make sure addIceCandidate() called after setRemoteDescription(). +async function doSignalingAndCandidateHandshake(pc1, pc2) { + const handshakePromise = doSignalingHandshake(pc1, pc2) + exchangeIceCandidates(pc1, pc2, handshakePromise) + await handshakePromise } // Helper function to create a pair of connected data channel. @@ -247,8 +253,6 @@ function createDataChannelPair( { const channel1 = pc1.createDataChannel(''); - exchangeIceCandidates(pc1, pc2); - return new Promise((resolve, reject) => { let channel2; let opened1 = false; @@ -287,7 +291,8 @@ function createDataChannelPair( pc2.addEventListener('datachannel', onDataChannel); - doSignalingHandshake(pc1, pc2); + doSignalingAndCandidateHandshake(pc1, pc2) + .catch(reject); }); } diff --git a/webrtc/RTCPeerConnection-iceConnectionState.html b/webrtc/RTCPeerConnection-iceConnectionState.html index 4071033a3c9eff6..d5cd476b9fad6dc 100644 --- a/webrtc/RTCPeerConnection-iceConnectionState.html +++ b/webrtc/RTCPeerConnection-iceConnectionState.html @@ -11,8 +11,7 @@ // https://w3c.github.io/webrtc-pc/archives/20170605/webrtc.html // The following helper functions are called from RTCPeerConnection-helper.js: - // exchangeIceCandidates - // doSignalingHandshake + // doSignalingAndCandidateHandshake /* 4.3.2. Interface Definition @@ -105,8 +104,8 @@ async_test(t => { const pc1 = new RTCPeerConnection(); t.add_cleanup(() => pc1.close()); - const pc2 = new RTCPeerConnection(); + const pc2 = new RTCPeerConnection(); t.add_cleanup(() => pc2.close()); const onIceConnectionStateChange = t.step_func(() => { @@ -139,8 +138,8 @@ pc1.addEventListener('iceconnectionstatechange', onIceConnectionStateChange); - exchangeIceCandidates(pc1, pc2); - doSignalingHandshake(pc1, pc2); + doSignalingAndCandidateHandshake(pc1, pc2) + .catch(t.unreached_func('handshake error')); }, 'connection with one data channel should eventually have connected connection state'); /* diff --git a/webrtc/RTCPeerConnection-iceGatheringState.html b/webrtc/RTCPeerConnection-iceGatheringState.html index fb9e514194b9806..f579c4e6de64bc0 100644 --- a/webrtc/RTCPeerConnection-iceGatheringState.html +++ b/webrtc/RTCPeerConnection-iceGatheringState.html @@ -11,8 +11,7 @@ // https://w3c.github.io/webrtc-pc/archives/20170605/webrtc.html // The following helper functions are called from RTCPeerConnection-helper.js: - // exchangeIceCandidates - // doSignalingHandshake + // doSignalingAndCandidateHandshake /* 4.3.2. Interface Definition @@ -133,8 +132,8 @@ // when icegatheringstatechange event is fired. pc2.addEventListener('icegatheringstatechange', onIceGatheringStateChange); - exchangeIceCandidates(pc1, pc2); - doSignalingHandshake(pc1, pc2); + doSignalingAndCandidateHandshake(pc1, pc2) + .catch(t.unreached_func('handshake error')); }, 'connection with one data channel should eventually have connected connection state'); /* diff --git a/webrtc/RTCPeerConnection-ondatachannel.html b/webrtc/RTCPeerConnection-ondatachannel.html index 1070ee701d2336c..e6991ccfacd9ec5 100644 --- a/webrtc/RTCPeerConnection-ondatachannel.html +++ b/webrtc/RTCPeerConnection-ondatachannel.html @@ -11,8 +11,7 @@ // https://w3c.github.io/webrtc-pc/archives/20170605/webrtc.html // The following helper functions are called from RTCPeerConnection-helper.js: - // exchangeIceCandidates - // doSignalingHandshake + // doSignalingAndCandidateHandshake /* 6.2. RTCDataChannel @@ -70,8 +69,9 @@ localPc.createDataChannel('test'); remotePc.addEventListener('datachannel', onDataChannel); - exchangeIceCandidates(localPc, remotePc); - doSignalingHandshake(localPc, remotePc); + + doSignalingAndCandidateHandshake(localPc, remotePc) + .catch(t.unreached_func('handshake error')); }, 'datachannel event should fire when new data channel is announced to the remote peer'); /* @@ -134,8 +134,9 @@ assert_equals(localChannel.priority, 'high'); remotePc.addEventListener('datachannel', onDataChannel); - exchangeIceCandidates(localPc, remotePc); - doSignalingHandshake(localPc, remotePc); + + doSignalingAndCandidateHandshake(localPc, remotePc) + .catch(t.unreached_func('handshake error')); }, 'Data channel created on remote peer should match the same configuration as local peer'); /* @@ -162,8 +163,9 @@ }); remotePc.addEventListener('datachannel', onDataChannel); - exchangeIceCandidates(localPc, remotePc); - doSignalingHandshake(localPc, remotePc); + + doSignalingAndCandidateHandshake(localPc, remotePc) + .catch(t.unreached_func('handshake error')); t.step_timeout(t.step_func_done(), 200); }, 'Data channel created with negotiated set to true should not fire datachannel event on remote peer'); diff --git a/webrtc/RTCPeerConnection-track-stats.https.html b/webrtc/RTCPeerConnection-track-stats.https.html index 682e7e57e465ccd..21a106792db7d06 100644 --- a/webrtc/RTCPeerConnection-track-stats.https.html +++ b/webrtc/RTCPeerConnection-track-stats.https.html @@ -404,8 +404,7 @@ let [tracks, streams] = await getUserMediaTracksAndStreams(2); let sender = caller.addTrack(tracks[0], streams[0]); callee.addTrack(tracks[1], streams[1]); - exchangeIceCandidates(caller, callee); - await doSignalingHandshake(caller, callee); + await doSignalingAndCandidateHandshake(caller, callee); await onIceConnectionStateCompleted(caller); let receiver = caller.getReceivers()[0]; @@ -452,8 +451,7 @@ let [tracks, streams] = await getUserMediaTracksAndStreams(2); let sender = caller.addTrack(tracks[0], streams[0]); callee.addTrack(tracks[1], streams[1]); - exchangeIceCandidates(caller, callee); - await doSignalingHandshake(caller, callee); + await doSignalingAndCandidateHandshake(caller, callee); await onIceConnectionStateCompleted(caller); let receiver = caller.getReceivers()[0]; @@ -500,8 +498,7 @@ let [tracks, streams] = await getUserMediaTracksAndStreams(2); let sender = caller.addTrack(tracks[0], streams[0]); callee.addTrack(tracks[1], streams[1]); - exchangeIceCandidates(caller, callee); - await doSignalingHandshake(caller, callee); + await doSignalingAndCandidateHandshake(caller, callee); await onIceConnectionStateCompleted(caller); let senderReport = await sender.getStats(); @@ -527,8 +524,7 @@ let [tracks, streams] = await getUserMediaTracksAndStreams(2); let sender = caller.addTrack(tracks[0], streams[0]); callee.addTrack(tracks[1], streams[1]); - exchangeIceCandidates(caller, callee); - await doSignalingHandshake(caller, callee); + await doSignalingAndCandidateHandshake(caller, callee); await onIceConnectionStateCompleted(caller); let receiver = caller.getReceivers()[0]; diff --git a/webrtc/RTCRtpReceiver-getContributingSources.https.html b/webrtc/RTCRtpReceiver-getContributingSources.https.html index 8ddff5be410d904..e1881f8af8e33a3 100644 --- a/webrtc/RTCRtpReceiver-getContributingSources.https.html +++ b/webrtc/RTCRtpReceiver-getContributingSources.https.html @@ -12,8 +12,7 @@ // The following helper function is called from RTCPeerConnection-helper.js // getTrackFromUserMedia - // exchangeIceCandidates - // doSignalingHandshake + // doSignalingAndCandidateHandshake /* 5.3. RTCRtpReceiver Interface @@ -58,8 +57,7 @@ return getTrackFromUserMedia('audio') .then(([track, mediaStream]) => { pc1.addTrack(track, mediaStream); - exchangeIceCandidates(pc1, pc2); - return doSignalingHandshake(pc1, pc2); + return doSignalingAndCandidateHandshake(pc1, pc2); }) .then(() => ontrackPromise) .then(receiver => { diff --git a/webrtc/RTCRtpReceiver-getSynchronizationSources.https.html b/webrtc/RTCRtpReceiver-getSynchronizationSources.https.html index 80685580e227ec7..252fa0abdac0dcc 100644 --- a/webrtc/RTCRtpReceiver-getSynchronizationSources.https.html +++ b/webrtc/RTCRtpReceiver-getSynchronizationSources.https.html @@ -45,8 +45,7 @@ return getTrackFromUserMedia('audio') .then(([track, mediaStream]) => { pc1.addTrack(track, mediaStream); - exchangeIceCandidates(pc1, pc2); - return doSignalingHandshake(pc1, pc2); + return doSignalingAndCandidateHandshake(pc1, pc2); }) .then(() => ontrackPromise) .then(receiver => {