Replaces SSLIdentity* with scoped_refptr<RTCCertificate> in cricket::Transport layer.

webrtc/p2p/base/dtlstransport.h

 /*
  *  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 12 matching lines @@
 class PortAllocator;
 
 // Base should be a descendant of cricket::Transport
 template<class Base>
 class DtlsTransport : public Base {
  public:
   DtlsTransport(rtc::Thread* signaling_thread,
                 rtc::Thread* worker_thread,
                 const std::string& content_name,
                 PortAllocator* allocator,
-                rtc::SSLIdentity* identity)
+                const rtc::scoped_refptr<rtc::RTCCertificate>& certificate)
       : Base(signaling_thread, worker_thread, content_name, allocator),
-        identity_(identity),
+        certificate_(certificate),
         secure_role_(rtc::SSL_CLIENT),
         ssl_max_version_(rtc::SSL_PROTOCOL_DTLS_10) {
   }
 
   ~DtlsTransport() {
     Base::DestroyAllChannels();
   }
-  virtual void SetIdentity_w(rtc::SSLIdentity* identity) {
-    identity_ = identity;
+  void SetCertificate_w(
+      const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) override {

[tommi, 2015/08/25 10:28:08]

since this is a _w method, can we DCHECK that we're on the worker thread?  (I
know the code hasn't' been doing this, but since you're modifying it, maybe it's
not that much extra work)

[hbos, 2015/08/25 15:45:44]

Done. (And for other _w methods)

+    certificate_ = certificate;
   }
-  virtual bool GetIdentity_w(rtc::SSLIdentity** identity) {
-    if (!identity_)
+  bool GetCertificate_w(
+      rtc::scoped_refptr<rtc::RTCCertificate>* certificate) override {
+    if (!certificate_)
       return false;
 
-    *identity = identity_->GetReference();
+    *certificate = certificate_;
     return true;
   }
 
   virtual bool SetSslMaxProtocolVersion_w(rtc::SSLProtocolVersion version) {
     ssl_max_version_ = version;
     return true;
   }
 
   virtual bool ApplyLocalTransportDescription_w(TransportChannelImpl* channel,
                                                 std::string* error_desc) {
     rtc::SSLFingerprint* local_fp =

[tommi, 2015/08/25 10:28:08]

In general the implementation of this class should be moved to the .cc file and
there should be thread checks since there aren't any locks before accessing
member variables.  Can you add a TODO() at class level for doing that?

[hbos, 2015/08/25 15:45:44]

It's a template class so having everything in header should be OK?
Added TODO about DCHECKs.

         Base::local_description()->identity_fingerprint.get();
 
     if (local_fp) {
       // Sanity check local fingerprint.
-      if (identity_) {
+      if (certificate_) {
         rtc::scoped_ptr<rtc::SSLFingerprint> local_fp_tmp(
             rtc::SSLFingerprint::Create(local_fp->algorithm,
-                                              identity_));
+                                        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 {
-      identity_ = NULL;
+      certificate_ = nullptr;
     }
 
-    if (!channel->SetLocalIdentity(identity_)) {
+    // TODO(hbos): SetLocalCertificate

[tommi, 2015/08/25 10:28:08]

will this be addressed before checkin? It's not clear what the todo means.

[hbos, 2015/08/25 15:45:44]

Oh, sorry, that's sort of a "note to self"... I split this identity->cert
replace work up into a series of 3 CLs (they're all done but I'm gonna upload
them one at a time so I can merge in-between). The TODOs I'm adding in one of
these CLs is immediately addressed in the next one.
Should I make the TODOs more elaborate before landing or is it OK because
they're super temporary?

+    if (!channel->SetLocalIdentity(
+        certificate_ ? certificate_->identity() : nullptr)) {
       return BadTransportDescription("Failed to set local identity.",
                                      error_desc);
     }
 
     // Apply the description in the base class.
     return Base::ApplyLocalTransportDescription_w(channel, error_desc);
   }
 
   virtual bool NegotiateTransportDescription_w(ContentAction local_role,
                                                std::string* error_desc) {
@@ skipping 132 matching lines @@
     if (!channel->SetRemoteFingerprint(
             remote_fingerprint_->algorithm,
             reinterpret_cast<const uint8*>(remote_fingerprint_->digest.data()),
             remote_fingerprint_->digest.size())) {
       return BadTransportDescription("Failed to apply remote fingerprint.",
                                      error_desc);
     }
     return Base::ApplyNegotiatedTransportDescription_w(channel, error_desc);
   }
 
-  rtc::SSLIdentity* identity_;
+  rtc::scoped_refptr<rtc::RTCCertificate> certificate_;
   rtc::SSLRole secure_role_;
   rtc::SSLProtocolVersion ssl_max_version_;
   rtc::scoped_ptr<rtc::SSLFingerprint> remote_fingerprint_;
 };
 
 }  // namespace cricket
 
 #endif  // WEBRTC_P2P_BASE_DTLSTRANSPORT_H_