Modified PeerConnection and WebRtcSession for end-to-end QuicDataChannel usage.

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.
  */
 
 #include "webrtc/api/webrtcsession.h"
 
 #include <limits.h>
 
 #include <algorithm>
 #include <set>
 #include <utility>
 #include <vector>
 
 #include "webrtc/api/jsepicecandidate.h"
 #include "webrtc/api/jsepsessiondescription.h"
 #include "webrtc/api/peerconnectioninterface.h"
 #include "webrtc/api/sctputils.h"
 #include "webrtc/api/webrtcsessiondescriptionfactory.h"
 #include "webrtc/audio_sink.h"
 #include "webrtc/base/basictypes.h"
+#include "webrtc/base/bind.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/helpers.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/stringencode.h"
 #include "webrtc/base/stringutils.h"
 #include "webrtc/call.h"
 #include "webrtc/media/base/mediaconstants.h"
 #include "webrtc/media/base/videocapturer.h"
 #include "webrtc/p2p/base/portallocator.h"
 #include "webrtc/p2p/base/transportchannel.h"
 #include "webrtc/pc/channel.h"
 #include "webrtc/pc/channelmanager.h"
 #include "webrtc/pc/mediasession.h"
 
+#ifdef HAVE_QUIC
+#include "webrtc/p2p/quic/quictransportchannel.h"
+#endif  // HAVE_QUIC
+
 using cricket::ContentInfo;
 using cricket::ContentInfos;
 using cricket::MediaContentDescription;
 using cricket::SessionDescription;
 using cricket::TransportInfo;
 
 using cricket::LOCAL_PORT_TYPE;
 using cricket::STUN_PORT_TYPE;
 using cricket::RELAY_PORT_TYPE;
 using cricket::PRFLX_PORT_TYPE;
@@ skipping 400 matching lines @@
     return true;
   }
   return false;
 }
 
 WebRtcSession::WebRtcSession(webrtc::MediaControllerInterface* media_controller,
                              rtc::Thread* network_thread,
                              rtc::Thread* worker_thread,
                              rtc::Thread* signaling_thread,
                              cricket::PortAllocator* port_allocator)
-    : worker_thread_(worker_thread),
+    : network_thread_(network_thread),
+      worker_thread_(worker_thread),
       signaling_thread_(signaling_thread),
       // RFC 3264: The numeric value of the session id and version in the
       // o line MUST be representable with a "64 bit signed integer".
       // Due to this constraint session id |sid_| is max limited to LLONG_MAX.
       sid_(rtc::ToString(rtc::CreateRandomId64() & LLONG_MAX)),
       transport_controller_(new cricket::TransportController(signaling_thread,
                                                              network_thread,
                                                              port_allocator)),
       media_controller_(media_controller),
       channel_manager_(media_controller_->channel_manager()),
@@ skipping 66 matching lines @@
     if (rtc_configuration.enable_dtls_srtp) {
       dtls_enabled_ = *(rtc_configuration.enable_dtls_srtp);
     }
   }
 
   // Enable creation of RTP data channels if the kEnableRtpDataChannels is set.
   // It takes precendence over the disable_sctp_data_channels
   // PeerConnectionFactoryInterface::Options.
   if (rtc_configuration.enable_rtp_data_channel) {
     data_channel_type_ = cricket::DCT_RTP;
-  } else {
+  }
+#ifdef HAVE_QUIC
+  else if (rtc_configuration.enable_quic) {
+    // Use QUIC instead of DTLS when |enable_quic| is true.
+    data_channel_type_ = cricket::DCT_QUIC;
+    transport_controller_->use_quic();
+    if (dtls_enabled_) {
+      LOG(LS_INFO) << "Using QUIC instead of DTLS";
+    }
+    quic_data_transport_.reset(
+        new QuicDataTransport(signaling_thread(), worker_thread(),
+                              network_thread(), transport_controller_.get()));
+  }
+#endif  // HAVE_QUIC
+  else {
     // DTLS has to be enabled to use SCTP.
     if (!options.disable_sctp_data_channels && dtls_enabled_) {
       data_channel_type_ = cricket::DCT_SCTP;
     }
   }
 
   video_options_.screencast_min_bitrate_kbps =
       rtc_configuration.screencast_min_bitrate;
   audio_options_.combined_audio_video_bwe =
       rtc_configuration.combined_audio_video_bwe;
@@ skipping 1093 matching lines @@
       this, &WebRtcSession::OnDtlsSetupFailure);
 
   SignalVideoChannelCreated();
   video_channel_->SignalSentPacket.connect(this,
                                            &WebRtcSession::OnSentPacket_w);
   return true;
 }
 
 bool WebRtcSession::CreateDataChannel(const cricket::ContentInfo* content,
                                       const std::string* bundle_transport) {
+#ifdef HAVE_QUIC
+  if (data_channel_type_ == cricket::DCT_QUIC) {
+    // The TransportController may not have a QuicTransportChannel, so force
+    // creation of it.
+    LOG(LS_INFO) << "Getting or creating QuicTransportChannel";
+    RTC_DCHECK(transport_controller_->quic());
+    cricket::TransportChannel* transport_channel =
+        network_thread_->Invoke<cricket::TransportChannel*>(
+            RTC_FROM_HERE,
+            rtc::Bind(&cricket::TransportController::CreateTransportChannel_n,
+                      transport_controller_.get(), content->name,
+                      cricket::ICE_CANDIDATE_COMPONENT_RTP));
+    if (!transport_channel) {
+      return false;
+    }
+    quic_transport_channel_ =
+        static_cast<cricket::QuicTransportChannel*>(transport_channel);
+    quic_data_transport_->SetTransportChannel(quic_transport_channel_);
+    quic_data_transport_->set_transport_name(content->name);
+    quic_data_transport_->set_content_name(content->name);
+    return true;

[pthatcher1, 2016/07/22 17:57:57]

I don't understand why we create the quic_transport_channel_ here, and why we
store it in WebrtcSession at all.  Just calling
quic_data_transport_->SetTransport(content->name) here instead of
SetTransportChnnel and set_transport_name would accomplish the same in  more
simple way, wouldn't it?

[Zhi Huang, 2016/07/25 23:40:36]

Yes, we don't need quic_transport_channel_ here anymore.

+  }
+#endif  // HAVE_QUIC

[Taylor Brandstetter, 2016/07/21 23:39:57]

Ok, so when using QUIC, the QuicTransportChannel always exists, but its content
name isn't populated until SetXDescription. This seems fine. But why don't you
use the SetTransport method here, which will handle creating the transport
channel? If you do that, set_transport_name and SetTransportChannel can also be
private.

   bool sctp = (data_channel_type_ == cricket::DCT_SCTP);
   bool require_rtcp_mux =
       rtcp_mux_policy_ == PeerConnectionInterface::kRtcpMuxPolicyRequire;
   bool create_rtcp_transport_channel = !sctp && !require_rtcp_mux;
   data_channel_.reset(channel_manager_->CreateDataChannel(
       transport_controller_.get(), content->name, bundle_transport,
       create_rtcp_transport_channel, data_channel_type_));
   if (!data_channel_) {
     return false;
   }
@@ skipping 163 matching lines @@
 
 // We need to check the local/remote description for the Transport instead of
 // the session, because a new Transport added during renegotiation may have
 // them unset while the session has them set from the previous negotiation.
 // Not doing so may trigger the auto generation of transport description and
 // mess up DTLS identity information, ICE credential, etc.
 bool WebRtcSession::ReadyToUseRemoteCandidate(
     const IceCandidateInterface* candidate,
     const SessionDescriptionInterface* remote_desc,
     bool* valid) {
-  *valid = true;;
+  *valid = true;
 
   const SessionDescriptionInterface* current_remote_desc =
       remote_desc ? remote_desc : remote_desc_.get();
 
   if (!current_remote_desc) {
     return false;
   }
 
   size_t mediacontent_index =
       static_cast<size_t>(candidate->sdp_mline_index());
   size_t remote_content_size =
       current_remote_desc->description()->contents().size();
   if (mediacontent_index >= remote_content_size) {
     LOG(LS_ERROR) << "ReadyToUseRemoteCandidate: Invalid candidate media index "
                   << mediacontent_index;
 
     *valid = false;
     return false;
   }
 
   cricket::ContentInfo content =
       current_remote_desc->description()->contents()[mediacontent_index];
   cricket::BaseChannel* channel = GetChannel(content.name);

[pthatcher1, 2016/07/22 17:57:57]

I think we should add a method called GetTransportName(content.name) and use
that.  Then we could put in the #ifdef HAVE_QUIC version to return that
transport name and only call transport_controller_->ReadyForRemoteCandidates
once. 

[Zhi Huang, 2016/07/25 23:40:36]

Done.

   if (!channel) {
+#ifdef HAVE_QUIC
+    if (data_channel_type_ == cricket::DCT_QUIC && quic_transport_channel_ &&
+        content.name == quic_transport_channel_->transport_name()) {
+      // QUIC data channels do not have a BaseChannel unless bundle is enabled.
+      return transport_controller_->ReadyForRemoteCandidates(
+          quic_transport_channel_->transport_name());

[pthatcher1, 2016/07/22 17:57:57]

Why get it from the quic_transport_channel_ instead of getting it from the
quic_data_transport_?

[Zhi Huang, 2016/07/25 23:40:36]

Done.

+    }
+#endif  // HAVE_QUIC
     return false;
   }
 
   return transport_controller_->ReadyForRemoteCandidates(
       channel->transport_name());
 }
 
 void WebRtcSession::OnTransportControllerGatheringState(
     cricket::IceGatheringState state) {
   ASSERT(signaling_thread()->IsCurrent());
@@ skipping 121 matching lines @@
     metrics_observer_->IncrementSparseEnumCounter(ssl_counter_type,
                                                   ssl_cipher_suite);
   }
 }
 
 void WebRtcSession::OnSentPacket_w(const rtc::SentPacket& sent_packet) {
   RTC_DCHECK(worker_thread()->IsCurrent());
   media_controller_->call_w()->OnSentPacket(sent_packet);
 }
 
 }  // namespace webrtc

[Taylor Brandstetter, 2016/07/21 23:39:57]

We still need to clean up the QuicDataTransport at the correct times. Wherever
the cricket::DataChannel would be destroyed, we should call SetTransport("") or
something which will make the QuicDataTransport tear down its transport channel.

There's also the fact that bundling isn't handled. If you try to bundle data on
the audio transport, nothing will happen. I guess this is ok for now as long as
we document it, but we had talked about returning an error.

[pthatcher1, 2016/07/22 17:57:57]

Agreed, but I would say we should just destroy the QuicDataTransport, not
SetTransport("").
I think we should go all the way and call SetTransportName at the right time (in
the code that enables bundle).  That was the whole point of making
QuicDataTransport look like a BaseChannel, wasn't it?