Perform safe addIceCandidate together with signaling handshake

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();

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;

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');
 
   /*

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}`);

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);
   });
 }
 

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');
 
   /*

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');
 
   /*

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');

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];
 

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 => {

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 => {