Reland "Modify Bundle logic to not add & destroy extra transport at add-track"

This relands commit I41cae74605fecf454900a958776b95607ccf3745, after
reverting it in order to merge the revert to M93 (the deadline for
which is now exceeded).

Original change description:

> If a bundle is established, then per JSEP, the offerer is required to
> include the new track in the bundle, and per BUNDLE, the answerer has
> to either accept the track as part of the bundle or reject the track;
> it cannot move it out of the group. So we will never need the transport.
>
> Bug: webrtc:12837
> Change-Id: I41cae74605fecf454900a958776b95607ccf3745
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221601
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#34290}

Bug: webrtc:12837
Change-Id: I30a8f03165ab797ed766b51c4eb15c2a9cecb5ed
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228500
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34727}

pc/jsep_transport_controller.cc

@@ -551,6 +551,17 @@ RTCError JsepTransportController::ApplyDescription_n(
         MergeEncryptedHeaderExtensionIdsForBundles(description);
   }
 
+  // Because the creation of transports depends on whether
+  // certain mids are present, we have to process rejection
+  // before we try to create transports.
+  for (size_t i = 0; i < description->contents().size(); ++i) {
+    const cricket::ContentInfo& content_info = description->contents()[i];
+    if (content_info.rejected) {
+      // This may cause groups to be removed from `bundles_.bundle_groups()`.
+      HandleRejectedContent(content_info);
+    }
+  }
+
   for (const cricket::ContentInfo& content_info : description->contents()) {
     // Don't create transports for rejected m-lines and bundled m-lines.
     if (content_info.rejected ||
@@ -569,9 +580,8 @@ RTCError JsepTransportController::ApplyDescription_n(
     const cricket::ContentInfo& content_info = description->contents()[i];
     const cricket::TransportInfo& transport_info =
         description->transport_infos()[i];
+
     if (content_info.rejected) {
-      // This may cause groups to be removed from `bundles_.bundle_groups()`.
-      HandleRejectedContent(content_info);
       continue;
     }
 
@@ -977,7 +987,21 @@ RTCError JsepTransportController::MaybeCreateJsepTransport(
   if (transport) {
     return RTCError::OK();
   }
-
+  // If we have agreed to a bundle, the new mid will be added to the bundle
+  // according to JSEP, and the responder can't move it out of the group
+  // according to BUNDLE. So don't create a transport.
+  // The MID will be added to the bundle elsewhere in the code.
+  if (bundles_.bundle_groups().size() > 0) {
+    const auto& default_bundle_group = bundles_.bundle_groups()[0];
+    if (default_bundle_group->content_names().size() > 0) {
+      auto bundle_transport =
+          GetJsepTransportByName(default_bundle_group->content_names()[0]);
+      if (bundle_transport) {
+        transports_.SetTransportForMid(content_info.name, bundle_transport);
+        return RTCError::OK();
+      }
+    }
+  }
   const cricket::MediaContentDescription* content_desc =
       content_info.media_description();
   if (certificate_ && !content_desc->cryptos().empty()) {

pc/jsep_transport_controller_unittest.cc

@@ -2051,7 +2051,6 @@ TEST_F(JsepTransportControllerTest, ChangeTaggedMediaSectionMaxBundle) {
   EXPECT_TRUE(transport_controller_
                   ->SetLocalDescription(SdpType::kOffer, local_reoffer.get())
                   .ok());
-
   std::unique_ptr<cricket::SessionDescription> remote_reanswer(
       local_reoffer->Clone());
   EXPECT_TRUE(

pc/peer_connection_integrationtest.cc

@@ -3639,6 +3639,20 @@ TEST_P(PeerConnectionIntegrationInteropTest,
   ASSERT_TRUE(ExpectNewFrames(media_expectations));
 }
 
+TEST_P(PeerConnectionIntegrationTest, NewTracksDoNotCauseNewCandidates) {
+  ASSERT_TRUE(CreatePeerConnectionWrappers());
+  ConnectFakeSignaling();
+  caller()->AddAudioVideoTracks();
+  caller()->CreateAndSetAndSignalOffer();
+  ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
+  ASSERT_TRUE_WAIT(DtlsConnected(), kDefaultTimeout);
+  caller()->ExpectCandidates(0);
+  callee()->ExpectCandidates(0);
+  caller()->AddAudioTrack();
+  caller()->CreateAndSetAndSignalOffer();
+  ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
+}
+
 INSTANTIATE_TEST_SUITE_P(
     PeerConnectionIntegrationTest,
     PeerConnectionIntegrationInteropTest,

pc/rtcp_mux_filter.cc

@@ -91,7 +91,8 @@ bool RtcpMuxFilter::SetAnswer(bool answer_enable, ContentSource src) {
   }
 
   if (!ExpectAnswer(src)) {
-    RTC_LOG(LS_ERROR) << "Invalid state for RTCP mux answer";
+    RTC_LOG(LS_ERROR) << "Invalid state for RTCP mux answer, state is "
+                      << state_ << ", source is " << src;
     return false;
   }
 

pc/test/integration_test_helpers.h

@@ -17,6 +17,7 @@
 
 #include <algorithm>
 #include <functional>
+#include <limits>
 #include <list>
 #include <map>
 #include <memory>
@@ -704,6 +705,11 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
     audio_concealed_stat_ = *track_stats->concealed_samples;
   }
 
+  // Sets number of candidates expected
+  void ExpectCandidates(int candidate_count) {
+    candidates_expected_ = candidate_count;
+  }
+
  private:
   explicit PeerConnectionIntegrationWrapper(const std::string& debug_name)
       : debug_name_(debug_name) {}
@@ -1089,6 +1095,9 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
       }
     }
 
+    // Check if we expected to have a candidate.
+    EXPECT_GT(candidates_expected_, 1);
+    candidates_expected_--;
     std::string ice_sdp;
     EXPECT_TRUE(candidate->ToString(&ice_sdp));
     if (signaling_message_receiver_ == nullptr || !signal_ice_candidates_) {
@@ -1172,6 +1181,9 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
       peer_connection_signaling_state_history_;
   webrtc::FakeRtcEventLogFactory* event_log_factory_;
 
+  // Number of ICE candidates expected. The default is no limit.
+  int candidates_expected_ = std::numeric_limits<int>::max();
+
   // Variables for tracking delay stats on an audio track
   int audio_packets_stat_ = 0;
   double audio_delay_stat_ = 0.0;