Fix senders and receivers when transceivers are removed.

This also adds a failing test for the outstanding issue with transceiver
updates.

Bug: chromium:980879


Checkpoint

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

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

@@ -3146,6 +3146,17 @@ void RTCPeerConnection::DidModifyTransceivers(
         mute_tracks.push_back(transceiver->receiver()->track());
     }
   }
+  if (sdp_semantics_ == webrtc::SdpSemantics::kUnifiedPlan) {
+    // Update the rtp_senders_ and rtp_receivers_ members to only contain
+    // senders and receivers that are in the current set of transceivers.
+    rtp_senders_.clear();
+    rtp_receivers_.clear();
+    for (auto& transceiver : transceivers_) {
+      rtp_senders_.push_back(transceiver->sender());
+      rtp_receivers_.push_back(transceiver->receiver());
+    }
+  }
+
   MediaStreamVector current_streams = getRemoteStreams();
 
   // Modify and fire "pc.onsignalingchange" synchronously.

third_party/blink/web_tests/external/wpt/webrtc/RTCRtpTransceiver-stop-expected.txt

 This is a testharness.js-based test.
 PASS A transceiver added and stopped before the initial offer generation should not trigger an offer m-section generation
 PASS During renegotiation, adding and stopping a transceiver should not trigger a renegotiated offer m-section generation
-FAIL A stopped sendonly transceiver should generate a sendonly m-section in the offer assert_true: The audio m-section should be sendonly expected true got false
+PASS A stopped sendonly transceiver should generate an inactive m-section in the offer
 PASS A stopped inactive transceiver should generate an inactive m-section in the offer
-PASS If a track is stopped locally, setting a locally generated answer should still work
-PASS If a track is stopped remotely, setting a locally generated answer should still work
+PASS If a transceiver is stopped locally, setting a locally generated answer should still work
+PASS If a transceiver is stopped remotely, setting a locally generated answer should still work
+PASS If a transceiver is stopped, transceivers, senders and receivers should disappear after offer/answer
+FAIL If a transceiver is stopped, transceivers should end up in state stopped assert_equals: expected "recvonly" but got "stopped"
 Harness: the test ran to completion.
 

third_party/blink/web_tests/external/wpt/webrtc/RTCRtpTransceiver-stop.html

@@ -59,8 +59,8 @@ promise_test(async (t)=> {
 
     const offer = await pc1.createOffer();
 
-    assert_true(offer.sdp.includes("a=sendonly"), "The audio m-section should be sendonly");
-}, "A stopped sendonly transceiver should generate a sendonly m-section in the offer");
+    assert_true(offer.sdp.includes("a=inactive"), "The audio m-section should be inactive");
+}, "A stopped sendonly transceiver should generate an inactive m-section in the offer");
 
 promise_test(async (t)=> {
     const pc1 = new RTCPeerConnection();
@@ -90,7 +90,7 @@ promise_test(async (t) => {
   pc1.getTransceivers()[0].stop();
   await exchangeOfferAnswer(pc1, pc2);
   await pc1.setLocalDescription(await pc1.createOffer());
-}, 'If a track is stopped locally, setting a locally generated answer should still work');
+}, 'If a transceiver is stopped locally, setting a locally generated answer should still work');
 
 promise_test(async (t) => {
   const pc1 = new RTCPeerConnection();
@@ -102,6 +102,42 @@ promise_test(async (t) => {
   pc2.getTransceivers()[0].stop();
   await exchangeOfferAnswer(pc2, pc1);
   await pc1.setLocalDescription(await pc1.createOffer());
-}, 'If a track is stopped remotely, setting a locally generated answer should still work');
+}, 'If a transceiver is stopped remotely, setting a locally generated answer should still work');
+
+promise_test(async (t) => {
+  const pc1 = new RTCPeerConnection();
+  const pc2 = new RTCPeerConnection();
+  t.add_cleanup(() => pc1.close());
+  t.add_cleanup(() => pc2.close());
+  pc1.addTransceiver("audio");
+  await exchangeOfferAnswer(pc1, pc2);
+  assert_equals(pc1.getTransceivers().length, 1);
+  assert_equals(pc2.getTransceivers().length, 1);
+  pc1.getTransceivers()[0].stop();
+  await exchangeOfferAnswer(pc1, pc2);
+  assert_equals(pc1.getTransceivers().length, 0);
+  assert_equals(pc2.getTransceivers().length, 0);
+  assert_equals(pc1.getSenders().length, 0, 'caller senders');
+  assert_equals(pc1.getReceivers().length, 0, 'caller receivers');
+  assert_equals(pc2.getSenders().length, 0, 'callee senders');
+  assert_equals(pc2.getReceivers().length, 0, 'callee receivers');
+}, 'If a transceiver is stopped, transceivers, senders and receivers should disappear after offer/answer');
+
+promise_test(async (t) => {
+  const pc1 = new RTCPeerConnection();
+  const pc2 = new RTCPeerConnection();
+  t.add_cleanup(() => pc1.close());
+  t.add_cleanup(() => pc2.close());
+  pc1.addTransceiver("audio");
+  await exchangeOfferAnswer(pc1, pc2);
+  assert_equals(pc1.getTransceivers().length, 1);
+  assert_equals(pc2.getTransceivers().length, 1);
+  pc1Transceiver = pc1.getTransceivers()[0];
+  pc2Transceiver = pc2.getTransceivers()[0];
+  pc1.getTransceivers()[0].stop();
+  await exchangeOfferAnswer(pc1, pc2);
+  assert_equals('stopped', pc1Transceiver.direction);
+  assert_equals('stopped', pc2Transceiver.direction);
+}, 'If a transceiver is stopped, transceivers should end up in state stopped');
 
 </script>

third_party/blink/web_tests/external/wpt/webrtc/RTCRtpTransceiver.https-expected.txt

 This is a testharness.js-based test.
+Harness Error. harness_status.status = 1 , harness_status.message = Unhandled rejection: assert_equals: RTCRtpTransceiver.stop() with closed PC throws InvalidStateError expected "InvalidStateError" but got "Error"
 PASS checkAddTransceiverNoTrack
 PASS checkAddTransceiverWithTrack
 PASS checkAddTransceiverWithAddTrack
@@ -23,7 +24,7 @@ PASS checkRemoveAndReadd
 PASS checkAddTrackExistingTransceiverThenRemove
 FAIL checkRemoveTrackNegotiation assert_equals: pc2.setRemoteDescription(offer) should've added 2 tracks to receive stream expected 2 but got 0
 FAIL checkMute assert_true: expected true got false
-FAIL checkStop assert_equals: getReceivers does not expose a receiver of a stopped transceiver expected 0 but got 1
+PASS checkStop
 PASS checkStopAfterCreateOffer
 PASS checkStopAfterSetLocalOffer
 PASS checkStopAfterSetRemoteOffer
@@ -32,7 +33,7 @@ PASS checkStopAfterSetLocalAnswer
 PASS checkStopAfterClose
 PASS checkLocalRollback
 PASS checkRollbackAndSetRemoteOfferWithDifferentType
-FAIL checkRemoteRollback promise_test: Unhandled rejection with value: object "InvalidAccessError: Failed to execute 'addTrack' on 'RTCPeerConnection': A sender already exists for the track."
+FAIL checkRemoteRollback assert_equals: track event from remote rollback expected 1 but got 0
 FAIL checkMsectionReuse assert_equals: expected "[{currentDirection:null,mid:null}]" but got "[{currentDirection:\"inactive\",mid:\"0\"}]"
 PASS checkStopAfterCreateOfferWithReusedMsection
 PASS checkAddIceCandidateToStoppedTransceiver

third_party/blink/web_tests/external/wpt/webrtc/RTCRtpTransceiver.https.html

@@ -1235,9 +1235,8 @@
       stoppedTransceiver.receiver.track.onended = resolve;
     });
     stoppedTransceiver.stop();
-    assert_equals(pc1.getReceivers().length, 0, 'getReceivers does not expose a receiver of a stopped transceiver');
-    assert_equals(pc1.getSenders().length, 0, 'getSenders does not expose a sender of a stopped transceiver');
-
+    assert_equals(pc1.getReceivers().length, 1, 'getReceivers exposes a receiver of a stopped transceiver before negotiation');
+    assert_equals(pc1.getSenders().length, 1, 'getSenders exposes a sender of a stopped transceiver before negotiation');
     await onended;
     // The transceiver has [[stopping]] = true, [[stopped]] = false
     hasPropsAndUniqueMids(pc1.getTransceivers(),
@@ -1273,30 +1272,38 @@
     offer = await pc1.createOffer();
     await pc1.setLocalDescription(offer);
 
-    stoppedTransceiver = pc2.getTransceivers()[0];
+    const stoppedCalleeTransceiver = pc2.getTransceivers()[0];
     onended = new Promise(resolve => {
-      stoppedTransceiver.receiver.track.onended = resolve;
+      stoppedCalleeTransceiver.receiver.track.onended = resolve;
     });
 
     await pc2.setRemoteDescription(offer);
 
     await onended;
-
-    // pc2's transceiver was stopped remotely, so has
-    // [[stopping]] = true, [[stopped]] = true.
+    // pc2's transceiver was stopped remotely.
+    // The track ends when setRemeoteDescription(offer) is set.
     hasProps(pc2.getTransceivers(),
       [
         {
           sender: {track: {kind: "audio"}},
           receiver: {track: {kind: "audio", readyState: "ended"}},
-          mid: null,
           currentDirection: "stopped",
-          direction: "stopped"
+          direction: "sendrecv"
         }
       ]);
+    // After setLocalDescription(answer), the transceiver has
+    // [[stopping]] = true, [[stopped]] = true, and is removed from pc2.
+    const stoppingAnswer = await pc2.createAnswer();
+    await pc2.setLocalDescription(stoppingAnswer);
+    assert_equals(pc2.getTransceivers().length, 0);
+    assert_equals(pc2.getReceivers().length, 0, 'getReceivers does not expose a receiver of a stopped transceiver after negotiation');
+    assert_equals(pc2.getSenders().length, 0, 'getSenders does not expose a sender of a stopped transceiver after negotiation');
 
     // Shouldn't throw either
     stoppedTransceiver.stop();
+    await pc1.setRemoteDescription(stoppingAnswer);
+    assert_equals(pc1.getReceivers().length, 0, 'getReceivers does not expose a receiver of a stopped transceiver after negotiation');
+    assert_equals(pc1.getSenders().length, 0, 'getSenders does not expose a sender of a stopped transceiver after negotiation');
 
     pc1.close();
     pc2.close();
@@ -1897,6 +1904,8 @@
 
     // After all this SRD/rollback, we should still get the track event
     let trackEvents = await setRemoteDescriptionReturnTrackEvents(pc2, offer);
+
+    assert_equals(trackEvents.length, 1);
     hasProps(trackEvents,
       [
         {
@@ -1910,6 +1919,7 @@
 
     // Make sure all this rollback hasn't messed up the signaling
     trackEvents = await setRemoteDescriptionReturnTrackEvents(pc1, answer);
+    assert_equals(trackEvents.length, 1);
     hasProps(trackEvents,
       [
         {
@@ -1939,6 +1949,8 @@
 
     trackEvents =
       await setRemoteDescriptionReturnTrackEvents(pc2, {type: "rollback"});
+
+    assert_equals(trackEvents.length, 1, 'track event from remote rollback');
     hasProps(trackEvents,
       [
         {

third_party/blink/web_tests/virtual/webrtc-wpt-plan-b/external/wpt/webrtc/RTCRtpTransceiver-stop-expected.txt

 This is a testharness.js-based test.
 FAIL A transceiver added and stopped before the initial offer generation should not trigger an offer m-section generation promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'."
 FAIL During renegotiation, adding and stopping a transceiver should not trigger a renegotiated offer m-section generation promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'."
-FAIL A stopped sendonly transceiver should generate a sendonly m-section in the offer promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'."
+FAIL A stopped sendonly transceiver should generate an inactive m-section in the offer promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'."
 FAIL A stopped inactive transceiver should generate an inactive m-section in the offer promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'."
-FAIL If a track is stopped locally, setting a locally generated answer should still work promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'."
-FAIL If a track is stopped remotely, setting a locally generated answer should still work promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'."
+FAIL If a transceiver is stopped locally, setting a locally generated answer should still work promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'."
+FAIL If a transceiver is stopped remotely, setting a locally generated answer should still work promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'."
+FAIL If a transceiver is stopped, transceivers, senders and receivers should disappear after offer/answer promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'."
+FAIL If a transceiver is stopped, transceivers should end up in state stopped promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'."
 Harness: the test ran to completion.