Allow transceivers to be not found in more cases.

This fixes the case where a media section is rejected in an answer,
something that is done by SFUs, but not possible using transceiver.stop().

Bug: chromium:1134686
Change-Id: Ia33579070093ab70c4191710fd1dcb3ca377befd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/187349
Reviewed-by: Henrik Boström <[email protected]>
Commit-Queue: Harald Alvestrand <[email protected]>
Cr-Commit-Position: refs/heads/master@{#32363}

Author: Harald Alvestrand

pc/peer_connection_rtp_unittest.cc

@@ -164,6 +164,28 @@ class PeerConnectionRtpTestUnifiedPlan : public PeerConnectionRtpBaseTest {
  protected:
   PeerConnectionRtpTestUnifiedPlan()
       : PeerConnectionRtpBaseTest(SdpSemantics::kUnifiedPlan) {}
+
+  // Helper to emulate an SFU that rejects an offered media section
+  // in answer.
+  bool ExchangeOfferAnswerWhereRemoteStopsTransceiver(
+      PeerConnectionWrapper* caller,
+      PeerConnectionWrapper* callee,
+      size_t mid_to_stop) {
+    auto offer = caller->CreateOffer();
+    caller->SetLocalDescription(CloneSessionDescription(offer.get()));
+    callee->SetRemoteDescription(std::move(offer));
+    EXPECT_LT(mid_to_stop, callee->pc()->GetTransceivers().size());
+    // Must use StopInternal in order to do instant reject.
+    callee->pc()->GetTransceivers()[mid_to_stop]->StopInternal();
+    auto answer = callee->CreateAnswer();
+    EXPECT_TRUE(answer);
+    bool set_local_answer =
+        callee->SetLocalDescription(CloneSessionDescription(answer.get()));
+    EXPECT_TRUE(set_local_answer);
+    bool set_remote_answer = caller->SetRemoteDescription(std::move(answer));
+    EXPECT_TRUE(set_remote_answer);
+    return set_remote_answer;
+  }
 };
 
 // These tests cover |webrtc::PeerConnectionObserver| callbacks firing upon
@@ -1573,6 +1595,42 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan,
   EXPECT_EQ(0U, callee->pc()->GetReceivers().size());
 }
 
+TEST_F(PeerConnectionRtpTestUnifiedPlan,
+       SetLocalDescriptionWorksAfterRepeatedAddRemove) {
+  auto caller = CreatePeerConnection();
+  auto callee = CreatePeerConnection();
+  auto video_track = caller->CreateVideoTrack("v");
+  auto track = caller->CreateAudioTrack("a");
+  caller->AddTransceiver(video_track);
+  auto transceiver = caller->AddTransceiver(track);
+  ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
+  caller->pc()->RemoveTrack(transceiver->sender());
+  ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
+  caller->AddTrack(track);
+  ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
+  caller->pc()->RemoveTrack(transceiver->sender());
+  ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
+}
+
+// This is a repro of Chromium bug https://crbug.com/1134686
+TEST_F(PeerConnectionRtpTestUnifiedPlan,
+       SetLocalDescriptionWorksAfterRepeatedAddRemoveWithRemoteReject) {
+  auto caller = CreatePeerConnection();
+  auto callee = CreatePeerConnection();
+  auto video_track = caller->CreateVideoTrack("v");
+  auto track = caller->CreateAudioTrack("a");
+  caller->AddTransceiver(video_track);
+  auto transceiver = caller->AddTransceiver(track);
+  ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
+  caller->pc()->RemoveTrack(transceiver->sender());
+  ExchangeOfferAnswerWhereRemoteStopsTransceiver(caller.get(), callee.get(), 1);
+  ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
+  caller->AddTrack(track);
+  ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
+  caller->pc()->RemoveTrack(transceiver->sender());
+  ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
+}
+
 // Test that AddTransceiver fails if trying to use unimplemented RTP encoding
 // parameters with the send_encodings parameters.
 TEST_F(PeerConnectionRtpTestUnifiedPlan,

pc/sdp_offer_answer.cc

@@ -2871,17 +2871,16 @@ RTCError SdpOfferAnswerHandler::UpdateTransceiversAndDataChannels(
         old_remote_content =
             &old_remote_description->description()->contents()[i];
       }
-      // In the case where an m-section has completed its rejection,
-      // and is not being reused, we do not expect a transceiver.
-      if (old_local_content && old_local_content->rejected &&
-          old_remote_content && old_remote_content->rejected &&
-          new_content.rejected) {
-        continue;
-      }
       auto transceiver_or_error =
           AssociateTransceiver(source, new_session.GetType(), i, new_content,
                                old_local_content, old_remote_content);
       if (!transceiver_or_error.ok()) {
+        // In the case where a transceiver is rejected locally, we don't
+        // expect to find a transceiver, but might find it in the case
+        // where state is still "stopping", not "stopped".
+        if (new_content.rejected) {
+          continue;
+        }
         return transceiver_or_error.MoveError();
       }
       auto transceiver = transceiver_or_error.MoveValue();
@@ -2946,8 +2945,9 @@ SdpOfferAnswerHandler::AssociateTransceiver(
       transceiver = transceivers().FindByMLineIndex(mline_index);
     }
     if (!transceiver) {
+      // This may happen normally when media sections are rejected.
       LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
-                           "Unknown transceiver");
+                           "Transceiver not found based on m-line index");
     }
   } else {
     RTC_DCHECK_EQ(source, cricket::CS_REMOTE);