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

webrtc/api/webrtcsession.cc

Index: webrtc/api/webrtcsession.cc
diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc
index 8c83dbbc62a414cd304c44409c9630d4e08887f4..bd8ca9bdee1a8927ecf7879178123a1bc097b27b 100644
--- a/webrtc/api/webrtcsession.cc
+++ b/webrtc/api/webrtcsession.cc
@@ -24,6 +24,7 @@
 #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"
@@ -38,6 +39,10 @@
 #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;
@@ -458,7 +463,8 @@ WebRtcSession::WebRtcSession(webrtc::MediaControllerInterface* media_controller,
                              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".
@@ -545,7 +551,21 @@ bool WebRtcSession::Initialize(
   // 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;
@@ -1659,6 +1679,29 @@ bool WebRtcSession::CreateVideoChannel(const cricket::ContentInfo* content,
 
 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;
@@ -1842,7 +1885,7 @@ 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();
@@ -1867,6 +1910,14 @@ bool WebRtcSession::ReadyToUseRemoteCandidate(
       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;
   }