Change ICE connection state on transceiver changes

This ensures that if the PC iceConnectionState should change
because unused transports are discarded, the state is updated.

Bug: chromium:966798

Change-Id: I09d945f5e70eec813f33c3131fbe889825613652
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632254Reviewed-by: Henrik Boström <hbos@chromium.org>
Commit-Queue: Harald Alvestrand <hta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664206}

third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc

@@ -2899,6 +2899,15 @@ void RTCPeerConnection::DidModifyTransceivers(
     transceiver->receiver()->track()->Component()->Source()->SetReadyState(
         MediaStreamSource::kReadyStateLive);
   }
+
+  // Transceiver modifications can cause changes in the set of ICE
+  // transports, which may affect ICE transport state.
+  // Note - this must be done every time the set of ICE transports happens.
+  // At the moment this only happens in SLD/SRD, and this function is called
+  // whenever these functions complete.
+  if (sdp_semantics_ == webrtc::SdpSemantics::kUnifiedPlan) {
+    UpdateIceConnectionState();
+  }
 }
 
 void RTCPeerConnection::SetAssociatedMediaStreams(

third_party/blink/web_tests/external/wpt/webrtc/RTCPeerConnection-iceConnectionState.https.html

@@ -276,5 +276,42 @@ async_test(t => {
 
       closed
         The RTCIceTransport has shut down and is no longer responding to STUN requests.
-   */
+  */
+
+for (let bundle_policy of ['balanced', 'max-bundle', 'max-compat']) {
+
+
+    promise_test(async t => {
+      const caller = new RTCPeerConnection({bundlePolicy: bundle_policy});
+      t.add_cleanup(() => caller.close());
+      const stream = await navigator.mediaDevices.getUserMedia(
+          {audio: true, video:true});
+      t.add_cleanup(() => stream.getTracks().forEach(track => track.stop()));
+      const [track1, track2] = stream.getTracks();
+      const sender1 = caller.addTrack(track1);
+      const sender2 = caller.addTrack(track2);
+      caller.createDataChannel('datachannel');
+      const callee = new RTCPeerConnection();
+      t.add_cleanup(() => callee.close());
+      coupleIceCandidates(caller, callee);
+      const offer = await caller.createOffer();
+      await caller.setLocalDescription(offer);
+      const [caller_transceiver1, caller_transceiver2] = caller.getTransceivers();
+      assert_equals(sender1.transport, caller_transceiver1.sender.transport);
+      await callee.setRemoteDescription(offer);
+      const [callee_transceiver1, callee_transceiver2] = callee.getTransceivers();
+      const answer = await callee.createAnswer();
+      await callee.setLocalDescription(answer);
+      await caller.setRemoteDescription(answer);
+      // At this point, we should have a single ICE transport, and it
+      // should be in the "connected" state.
+      assert_equals(caller_transceiver1.receiver.transport.iceTransport.state,
+                    'connected', 'ICE transport.state');
+      // The PeerConnection's iceConnectionState should therefore be 'connected'
+      assert_equals(caller.iceConnectionState, 'connected',
+                    'PC.iceConnectionState:');
+    }, 'iceConnectionState changes at the right time, with bundle policy ' +
+                 bundle_policy);
+  }
+
 </script>

third_party/blink/web_tests/external/wpt/webrtc/protocol/candidate-exchange.https.html

@@ -26,15 +26,36 @@ function iceGatheringCompleteWaiter(pc) {
   return waiter;
 }
 
+class StateLogger {
+  constructor(source, eventname, field) {
+    source.addEventListener(eventname, event => {
+      this.events.push(source[field]);
+    });
+    this.events = [source[field]];
+  }
+}
+
+class IceStateLogger extends StateLogger {
+  constructor(source) {
+    super(source, 'iceconnectionstatechange', 'iceConnectionState');
+  }
+}
+
 promise_test(async t => {
   const pc1 = new RTCPeerConnection();
   const pc2 = new RTCPeerConnection();
   t.add_cleanup(() => pc1.close());
   t.add_cleanup(() => pc2.close());
   pc1.createDataChannel('datachannel');
+  pc1IceStates = new IceStateLogger(pc1);
+  pc2IceStates = new IceStateLogger(pc1);
   coupleIceCandidates(pc1, pc2);
   await doSignalingHandshake(pc1, pc2);
-  await waitForIceStateChange(pc1, ['connected', 'completed']);
+  // Note - it's been claimed that this state sometimes jumps straight
+  // to "completed". If so, this test should be flaky.
+  await waitForIceStateChange(pc1, ['connected']);
+  assert_array_equals(pc1IceStates.events, ['new', 'checking', 'connected']);
+  assert_array_equals(pc2IceStates.events, ['new', 'checking', 'connected']);
 }, 'Two way ICE exchange works');
 
 promise_test(async t => {
@@ -42,6 +63,8 @@ promise_test(async t => {
   const pc2 = new RTCPeerConnection();
   t.add_cleanup(() => pc1.close());
   t.add_cleanup(() => pc2.close());
+  pc1IceStates = new IceStateLogger(pc1);
+  pc2IceStates = new IceStateLogger(pc1);
   let candidates = [];
   pc1.createDataChannel('datachannel');
   pc1.onicecandidate = e => {
@@ -62,6 +85,8 @@ promise_test(async t => {
   const candidate_pair = pc1.sctp.transport.iceTransport.getSelectedCandidatePair();
   assert_equals(candidate_pair.local.type, 'host');
   assert_equals(candidate_pair.remote.type, 'prflx');
+  assert_array_equals(pc1IceStates.events, ['new', 'checking', 'connected']);
+  assert_array_equals(pc2IceStates.events, ['new', 'checking', 'connected']);
 }, 'Adding only caller -> callee candidates gives a connection');
 
 promise_test(async t => {
@@ -69,6 +94,8 @@ promise_test(async t => {
   const pc2 = new RTCPeerConnection();
   t.add_cleanup(() => pc1.close());
   t.add_cleanup(() => pc2.close());
+  pc1IceStates = new IceStateLogger(pc1);
+  pc2IceStates = new IceStateLogger(pc1);
   let candidates = [];
   pc1.createDataChannel('datachannel');
   pc2.onicecandidate = e => {
@@ -89,8 +116,92 @@ promise_test(async t => {
   const candidate_pair = pc2.sctp.transport.iceTransport.getSelectedCandidatePair();
   assert_equals(candidate_pair.local.type, 'host');
   assert_equals(candidate_pair.remote.type, 'prflx');
+  assert_array_equals(pc1IceStates.events, ['new', 'checking', 'connected']);
+  assert_array_equals(pc2IceStates.events, ['new', 'checking', 'connected']);
 }, 'Adding only callee -> caller candidates gives a connection');
 
+promise_test(async t => {
+  const pc1 = new RTCPeerConnection();
+  const pc2 = new RTCPeerConnection();
+  t.add_cleanup(() => pc1.close());
+  t.add_cleanup(() => pc2.close());
+  pc1IceStates = new IceStateLogger(pc1);
+  pc2IceStates = new IceStateLogger(pc1);
+  let pc2ToPc1Candidates = [];
+  pc1.createDataChannel('datachannel');
+  pc2.onicecandidate = e => {
+    pc2ToPc1Candidates.push(e.candidate);
+    // This particular test verifies that candidates work
+    // properly if added from the pc2 onicecandidate event.
+    if (!e.candidate) {
+      for (const candidate of pc2ToPc1Candidates) {
+        if (candidate) {
+          pc1.addIceCandidate(candidate);
+        }
+      }
+    }
+  }
+  // Candidates from |pc1| are not delivered to |pc2|. |pc2| will use
+  // peer-reflexive candidates.
+  await doSignalingHandshake(pc1, pc2);
+  await Promise.all([waitForIceStateChange(pc1, ['connected', 'completed']),
+                     waitForIceStateChange(pc2, ['connected', 'completed'])]);
+  const candidate_pair = pc2.sctp.transport.iceTransport.getSelectedCandidatePair();
+  assert_equals(candidate_pair.local.type, 'host');
+  assert_equals(candidate_pair.remote.type, 'prflx');
+  assert_array_equals(pc1IceStates.events, ['new', 'checking', 'connected']);
+  assert_array_equals(pc2IceStates.events, ['new', 'checking', 'connected']);
+}, 'Adding callee -> caller candidates from end-of-candidates gives a connection');
+
+promise_test(async t => {
+  const pc1 = new RTCPeerConnection();
+  const pc2 = new RTCPeerConnection();
+  t.add_cleanup(() => pc1.close());
+  t.add_cleanup(() => pc2.close());
+  pc1IceStates = new IceStateLogger(pc1);
+  pc2IceStates = new IceStateLogger(pc1);
+  let pc1ToPc2Candidates = [];
+  let pc2ToPc1Candidates = [];
+  pc1.createDataChannel('datachannel');
+  pc1.onicecandidate = e => {
+    pc1ToPc2Candidates.push(e.candidate);
+  }
+  pc2.onicecandidate = e => {
+    pc2ToPc1Candidates.push(e.candidate);
+  }
+  const offer = await pc1.createOffer();
+  await Promise.all([pc1.setLocalDescription(offer),
+                     pc2.setRemoteDescription(offer)]);
+  const answer = await pc2.createAnswer();
+  await iceGatheringCompleteWaiter(pc1);
+  await pc2.setLocalDescription(answer).then(() => {
+    for (const candidate of pc1ToPc2Candidates) {
+      if (candidate) {
+        pc2.addIceCandidate(candidate);
+      }
+    }
+  });
+  await iceGatheringCompleteWaiter(pc2);
+  pc1.setRemoteDescription(answer).then(async () => {
+    for (const candidate of pc2ToPc1Candidates) {
+      if (candidate) {
+        await pc1.addIceCandidate(candidate);
+      }
+    }
+  });
+  await Promise.all([waitForIceStateChange(pc1, ['connected', 'completed']),
+                     waitForIceStateChange(pc2, ['connected', 'completed'])]);
+  const candidate_pair =
+        pc1.sctp.transport.iceTransport.getSelectedCandidatePair();
+  assert_equals(candidate_pair.local.type, 'host');
+  // When we supply remote candidates, we expect a jump to the 'host' candidate,
+  // but it might also remain as 'prflx'.
+  assert_true(candidate_pair.remote.type == 'host' ||
+              candidate_pair.remote.type == 'prflx');
+  assert_array_equals(pc1IceStates.events, ['new', 'checking', 'connected']);
+  assert_array_equals(pc2IceStates.events, ['new', 'checking', 'connected']);
+}, 'Explicit offer/answer exchange gives a connection');
+
 </script>
 </body>
 </html>

third_party/blink/web_tests/virtual/webrtc-wpt-plan-b/external/wpt/webrtc/RTCPeerConnection-iceConnectionState.https-expected.txt

@@ -6,5 +6,8 @@ FAIL connection with one data channel should eventually have connected connectio
 PASS connection with audio track should eventually have connected connection state
 PASS connection with audio and video tracks should eventually have connected connection state
 FAIL ICE can connect in a recvonly usecase promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'."
+FAIL iceConnectionState changes at the right time, with bundle policy balanced promise_test: Unhandled rejection with value: object "TypeError: Cannot read property 'sender' of undefined"
+FAIL iceConnectionState changes at the right time, with bundle policy max-bundle promise_test: Unhandled rejection with value: object "TypeError: Cannot read property 'sender' of undefined"
+FAIL iceConnectionState changes at the right time, with bundle policy max-compat promise_test: Unhandled rejection with value: object "TypeError: Cannot read property 'sender' of undefined"
 Harness: the test ran to completion.
 

third_party/blink/web_tests/virtual/webrtc-wpt-plan-b/external/wpt/webrtc/protocol/candidate-exchange.https-expected.txt

@@ -2,5 +2,7 @@ This is a testharness.js-based test.
 PASS Two way ICE exchange works
 FAIL Adding only caller -> callee candidates gives a connection promise_test: Unhandled rejection with value: object "TypeError: Cannot read property 'transport' of null"
 FAIL Adding only callee -> caller candidates gives a connection promise_test: Unhandled rejection with value: object "TypeError: Cannot read property 'transport' of null"
+FAIL Adding callee -> caller candidates from end-of-candidates gives a connection promise_test: Unhandled rejection with value: object "TypeError: Cannot read property 'transport' of null"
+FAIL Explicit offer/answer exchange gives a connection promise_test: Unhandled rejection with value: object "TypeError: Cannot read property 'transport' of null"
 Harness: the test ran to completion.