Allow TransportController to create a QuicTransportChannel

webrtc/p2p/base/dtlstransport.h

Index: webrtc/p2p/base/dtlstransport.h
diff --git a/webrtc/p2p/base/dtlstransport.h b/webrtc/p2p/base/dtlstransport.h
index 276b05f786f265bf026f941fa32bb75a51fe130d..8960be5e6dfe95d8954f4e0ae34225a82de6e351 100644
--- a/webrtc/p2p/base/dtlstransport.h
+++ b/webrtc/p2p/base/dtlstransport.h
@@ -64,27 +64,11 @@ class DtlsTransport : public Base {
     rtc::SSLFingerprint* local_fp =
         Base::local_description()->identity_fingerprint.get();
 
-    if (local_fp) {
-      // Sanity check local fingerprint.
-      if (certificate_) {
-        rtc::scoped_ptr<rtc::SSLFingerprint> local_fp_tmp(
-            rtc::SSLFingerprint::Create(local_fp->algorithm,
-                                        certificate_->identity()));
-        ASSERT(local_fp_tmp.get() != NULL);
-        if (!(*local_fp_tmp == *local_fp)) {
-          std::ostringstream desc;
-          desc << "Local fingerprint does not match identity. Expected: ";
-          desc << local_fp_tmp->ToString();
-          desc << " Got: " << local_fp->ToString();
-          return BadTransportDescription(desc.str(), error_desc);
-        }
-      } else {
-        return BadTransportDescription(
-            "Local fingerprint provided but no identity available.",
-            error_desc);
-      }
-    } else {
+    if (!local_fp) {
       certificate_ = nullptr;
+    } else if (!Base::VerifyCertificateFingerprint(certificate_.get(), local_fp,
+                                                   error_desc)) {
+      return false;
     }
 
     if (!channel->SetLocalCertificate(certificate_)) {
@@ -98,101 +82,25 @@ class DtlsTransport : public Base {
 
   bool NegotiateTransportDescription(ContentAction local_role,
                                      std::string* error_desc) override {
-    if (!Base::local_description() || !Base::remote_description()) {
-      const std::string msg = "Local and Remote description must be set before "
-                              "transport descriptions are negotiated";
-      return BadTransportDescription(msg, error_desc);
-    }
-
-    rtc::SSLFingerprint* local_fp =
-        Base::local_description()->identity_fingerprint.get();
     rtc::SSLFingerprint* remote_fp =
         Base::remote_description()->identity_fingerprint.get();
-
-    if (remote_fp && local_fp) {
+    if (!remote_fp) {
+      rtc::SSLFingerprint* local_fp =
+          Base::local_description()->identity_fingerprint.get();

[Taylor Brandstetter, 2016/04/12 22:33:11]

What if local_description() is null here? If it's impossible, then the check in
Base::NegotiateRole should be removed, and maybe an RTC_CHECK should be put in
this method.

[pthatcher1, 2016/04/12 23:26:59]

I think we should keep the check that both local and remote description are
non-null, like it was before.  Whether we keep that in NegotiateRole or not I'm
not sure.  Perhaps pass the (guaranteed to be non-null) descriptions into
NegotiateRole instead of having those read from the member variables again.

[mikescarlett, 2016/04/13 00:58:24]

I added the check back. That shouldn't have been deleted.

+      if (local_fp && (local_role == CA_ANSWER)) {
+        return BadTransportDescription(
+            "Local fingerprint supplied when caller didn't offer DTLS.",
+            error_desc);
+      }
+      // Otherwise we are not doing DTLS.
+      remote_fingerprint_.reset(new rtc::SSLFingerprint("", nullptr, 0));
+    } else {

[Taylor Brandstetter, 2016/04/12 22:33:11]

What if "remote_fp && !local_fp"? Before, that wouldn't set the remote
fingerprint. Maybe that doesn't matter, but if it does, it may be easier to just
keep the same if/else if/else structure as before.

[pthatcher1, 2016/04/12 23:26:59]

Yeah, I agree that keeping the old "if (remote_fp && local_fp)" would be better.
 Than we're just factoring out the big case where both are.

[mikescarlett, 2016/04/13 00:58:24]

Changed back to if (remote_fp && local_fp).

       remote_fingerprint_.reset(new rtc::SSLFingerprint(*remote_fp));
 
-      // From RFC 4145, section-4.1, The following are the values that the
-      // 'setup' attribute can take in an offer/answer exchange:
-      //       Offer      Answer
-      //      ________________
-      //      active     passive / holdconn
-      //      passive    active / holdconn
-      //      actpass    active / passive / holdconn
-      //      holdconn   holdconn
-      //
-      // Set the role that is most conformant with RFC 5763, Section 5, bullet 1
-      // The endpoint MUST use the setup attribute defined in [RFC4145].
-      // The endpoint that is the offerer MUST use the setup attribute
-      // value of setup:actpass and be prepared to receive a client_hello
-      // before it receives the answer.  The answerer MUST use either a
-      // setup attribute value of setup:active or setup:passive.  Note that
-      // if the answerer uses setup:passive, then the DTLS handshake will
-      // not begin until the answerer is received, which adds additional
-      // latency. setup:active allows the answer and the DTLS handshake to
-      // occur in parallel.  Thus, setup:active is RECOMMENDED.  Whichever
-      // party is active MUST initiate a DTLS handshake by sending a
-      // ClientHello over each flow (host/port quartet).
-      // IOW - actpass and passive modes should be treated as server and
-      // active as client.
-      ConnectionRole local_connection_role =
-          Base::local_description()->connection_role;
-      ConnectionRole remote_connection_role =
-          Base::remote_description()->connection_role;
-
-      bool is_remote_server = false;
-      if (local_role == CA_OFFER) {
-        if (local_connection_role != CONNECTIONROLE_ACTPASS) {
-          return BadTransportDescription(
-              "Offerer must use actpass value for setup attribute.",
-              error_desc);
-        }
-
-        if (remote_connection_role == CONNECTIONROLE_ACTIVE ||
-            remote_connection_role == CONNECTIONROLE_PASSIVE ||
-            remote_connection_role == CONNECTIONROLE_NONE) {
-          is_remote_server = (remote_connection_role == CONNECTIONROLE_PASSIVE);
-        } else {
-          const std::string msg =
-              "Answerer must use either active or passive value "
-              "for setup attribute.";
-          return BadTransportDescription(msg, error_desc);
-        }
-        // If remote is NONE or ACTIVE it will act as client.
-      } else {
-        if (remote_connection_role != CONNECTIONROLE_ACTPASS &&
-            remote_connection_role != CONNECTIONROLE_NONE) {
-          return BadTransportDescription(
-              "Offerer must use actpass value for setup attribute.",
-              error_desc);
-        }
-
-        if (local_connection_role == CONNECTIONROLE_ACTIVE ||
-            local_connection_role == CONNECTIONROLE_PASSIVE) {
-          is_remote_server = (local_connection_role == CONNECTIONROLE_ACTIVE);
-        } else {
-          const std::string msg =
-              "Answerer must use either active or passive value "
-              "for setup attribute.";
-          return BadTransportDescription(msg, error_desc);
-        }
-
-        // If local is passive, local will act as server.
+      if (!Base::NegotiateRole(local_role, &secure_role_, error_desc)) {
+        return false;
       }
-
-      secure_role_ = is_remote_server ? rtc::SSL_CLIENT :
-                                        rtc::SSL_SERVER;
-
-    } else if (local_fp && (local_role == CA_ANSWER)) {
-      return BadTransportDescription(
-          "Local fingerprint supplied when caller didn't offer DTLS.",
-          error_desc);
-    } else {
-      // We are not doing DTLS
-      remote_fingerprint_.reset(new rtc::SSLFingerprint(
-          "", NULL, 0));
     }
-
     // Now run the negotiation for the base class.
     return Base::NegotiateTransportDescription(local_role, error_desc);
   }