Remove BaseChannel's dependency on TransportController.

webrtc/api/webrtcsession.cc

 /*
  *  Copyright 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 1021 matching lines @@
   return transport_controller_->GetRemoteSSLCertificate(transport_name);
 }
 
 bool WebRtcSession::EnableBundle(const cricket::ContentGroup& bundle) {
   const std::string* first_content_name = bundle.FirstContentName();
   if (!first_content_name) {
     LOG(LS_WARNING) << "Tried to BUNDLE with no contents.";
     return false;
   }
   const std::string& transport_name = *first_content_name;
-  cricket::BaseChannel* first_channel = GetChannel(transport_name);
 
 #ifdef HAVE_QUIC
   if (quic_data_transport_ &&
       bundle.HasContentName(quic_data_transport_->content_name()) &&
       quic_data_transport_->transport_name() != transport_name) {
     LOG(LS_ERROR) << "Unable to BUNDLE " << quic_data_transport_->content_name()
                   << " on " << transport_name << "with QUIC.";
   }
 #endif
 
-  auto maybe_set_transport = [this, bundle, transport_name,
-                              first_channel](cricket::BaseChannel* ch) {
+  auto maybe_set_transport = [this, bundle,
+                              transport_name](cricket::BaseChannel* ch) {
     if (!ch || !bundle.HasContentName(ch->content_name())) {
       return true;
     }
 
     if (ch->transport_name() == transport_name) {
       LOG(LS_INFO) << "BUNDLE already enabled for " << ch->content_name()
                    << " on " << transport_name << ".";
       return true;
     }
 
-    if (!ch->SetTransport(transport_name)) {
+    cricket::TransportChannel* rtp_transport =
+        transport_controller_->CreateTransportChannel(
+            transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP);
+    cricket::TransportChannel* rtcp_transport = nullptr;
+    if (ch->ShouldCreateRtcpTransport()) {
+      rtcp_transport = transport_controller_->CreateTransportChannel(
+          transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP);
+    }
+    if (!ch->SetTransport(rtp_transport, rtcp_transport)) {

[Taylor Brandstetter, 2017/01/09 22:50:42]

I think there's an opportunity here to reduce some thread hops. Currently each
"SetTransport" will hop to the network thread, but instead, we could invoke
"EnableBundle" on the network thread.

       LOG(LS_WARNING) << "Failed to enable BUNDLE for " << ch->content_name();
       return false;
     }
     LOG(LS_INFO) << "Enabled BUNDLE for " << ch->content_name() << " on "
                  << transport_name << ".";
     return true;
   };
 
   if (!maybe_set_transport(voice_channel()) ||
       !maybe_set_transport(video_channel()) ||
@@ skipping 587 matching lines @@
   if (require_rtcp_mux) {
     voice_channel_->ActivateRtcpMux();
   }
 
   voice_channel_->SignalDtlsSetupFailure.connect(
       this, &WebRtcSession::OnDtlsSetupFailure);
 
   SignalVoiceChannelCreated();
   voice_channel_->SignalSentPacket.connect(this,
                                            &WebRtcSession::OnSentPacket_w);
+  voice_channel_->SignalDestroyTransport.connect(
+      this, &WebRtcSession::OnDestroyTransport);

[Taylor Brandstetter, 2017/01/09 22:50:42]

It would be nice if the creation *and* destruction were both handled in
WebRtcSession. Would that be feasible? The cases where DestroyTransportChannel
should be called are:

* When the BaseChannel is destroyed
* When SetTransport is called
* When ActivateRtcpMux is called

This sounds reasonable to handle here, and that would avoid needing to have
"SignalDestroyTransport".

[Zhi Huang, 2017/01/12 03:47:46]

Done.

   return true;
 }
 
 bool WebRtcSession::CreateVideoChannel(const cricket::ContentInfo* content,
                                        const std::string* bundle_transport) {
   bool require_rtcp_mux =
       rtcp_mux_policy_ == PeerConnectionInterface::kRtcpMuxPolicyRequire;
   bool create_rtcp_transport_channel = !require_rtcp_mux;
   video_channel_.reset(channel_manager_->CreateVideoChannel(
       media_controller_, transport_controller_.get(), content->name,
       bundle_transport, create_rtcp_transport_channel, SrtpRequired(),
       video_options_));
   if (!video_channel_) {
     return false;
   }
   if (require_rtcp_mux) {
     video_channel_->ActivateRtcpMux();
   }
   video_channel_->SignalDtlsSetupFailure.connect(
       this, &WebRtcSession::OnDtlsSetupFailure);
 
   SignalVideoChannelCreated();
   video_channel_->SignalSentPacket.connect(this,
                                            &WebRtcSession::OnSentPacket_w);
+  video_channel_->SignalDestroyTransport.connect(
+      this, &WebRtcSession::OnDestroyTransport);
   return true;
 }
 
 bool WebRtcSession::CreateDataChannel(const cricket::ContentInfo* content,
                                       const std::string* bundle_transport) {
 #ifdef HAVE_QUIC
   if (data_channel_type_ == cricket::DCT_QUIC) {
     RTC_DCHECK(transport_controller_->quic());
     const std::string transport_name =
         bundle_transport ? *bundle_transport : content->name;
@@ skipping 19 matching lines @@
   if (sctp) {
     data_channel_->SignalDataReceived.connect(
         this, &WebRtcSession::OnDataChannelMessageReceived);
   }
 
   data_channel_->SignalDtlsSetupFailure.connect(
       this, &WebRtcSession::OnDtlsSetupFailure);
 
   SignalDataChannelCreated();
   data_channel_->SignalSentPacket.connect(this, &WebRtcSession::OnSentPacket_w);
+  data_channel_->SignalDestroyTransport.connect(
+      this, &WebRtcSession::OnDestroyTransport);
   return true;
 }
 
 std::unique_ptr<SessionStats> WebRtcSession::GetStats_n(
     const ChannelNamePairs& channel_name_pairs) {
   ASSERT(network_thread()->IsCurrent());
   std::unique_ptr<SessionStats> session_stats(new SessionStats());
   for (const auto channel_name_pair : { &channel_name_pairs.voice,
                                         &channel_name_pairs.video,
                                         &channel_name_pairs.data }) {
@@ skipping 354 matching lines @@
   return channel->transport_name();
 }
 
 void WebRtcSession::OnDtlsHandshakeError(rtc::SSLHandshakeError error) {
   if (metrics_observer_) {
     metrics_observer_->IncrementEnumCounter(
         webrtc::kEnumCounterDtlsHandshakeError, static_cast<int>(error),
         static_cast<int>(rtc::SSLHandshakeError::MAX_VALUE));
   }
 }
+
+void WebRtcSession::OnDestroyTransport(const std::string& transport_name,
+                                       int component) {
+  transport_controller_->DestroyTransportChannel(transport_name, component);
+}
 }  // namespace webrtc