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

webrtc/p2p/base/session.cc

 /*
  *  Copyright 2004 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 259 matching lines @@
     if (!HasChannel(cand->component())) {
       *error = "Candidate has unknown component: " + cand->ToString() +
                " for content: " + content_name_;
       return false;
     }
   }
   transport_->get()->OnRemoteCandidates(candidates);
   return true;
 }
 
-void TransportProxy::SetIdentity(
-    rtc::SSLIdentity* identity) {
-  transport_->get()->SetIdentity(identity);
+void TransportProxy::SetCertificate(
+    const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) {
+  transport_->get()->SetCertificate(certificate);

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

thread safe? (ok to touch transport_ on any thread?)

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

The |transport_| object's SetCertificate posts to the worker thread and should
be safe. The |transport_| variable itself is set and read without locks so
TransportProxy should either be used single threadedly or with care,
|transport_| is only set in SetupMux...

 }
 
 std::string BaseSession::StateToString(State state) {
   switch (state) {
     case STATE_INIT:
       return "STATE_INIT";
     case STATE_SENTINITIATE:
       return "STATE_SENTINITIATE";
     case STATE_RECEIVEDINITIATE:
       return "STATE_RECEIVEDINITIATE";
@@ skipping 36 matching lines @@
                          const std::string& content_type,
                          bool initiator)
     : state_(STATE_INIT),
       error_(ERROR_NONE),
       signaling_thread_(signaling_thread),
       worker_thread_(worker_thread),
       port_allocator_(port_allocator),
       sid_(sid),
       content_type_(content_type),
       initiator_(initiator),
-      identity_(NULL),
       ssl_max_version_(rtc::SSL_PROTOCOL_DTLS_10),
       ice_tiebreaker_(rtc::CreateRandomId64()),
       role_switch_(false),
       ice_receiving_timeout_(-1) {
   ASSERT(signaling_thread->IsCurrent());
 }
 
 BaseSession::~BaseSession() {
   ASSERT(signaling_thread()->IsCurrent());
 
@@ skipping 33 matching lines @@
   // TODO(tommi): Assert on thread correctness.
   if (sdesc != remote_description_)
     remote_description_.reset(sdesc);
 }
 
 const SessionDescription* BaseSession::initiator_description() const {
   // TODO(tommi): Assert on thread correctness.
   return initiator_ ? local_description_.get() : remote_description_.get();
 }
 
-bool BaseSession::SetIdentity(rtc::SSLIdentity* identity) {
-  if (identity_)
+bool BaseSession::SetCertificate(
+    const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) {
+  if (certificate_ || !certificate)

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

I'm not sure I understand this check... is it not allowed to set a null
certificate or set one if a certificate has been set?
if so, then I think it would be slightly more readable as two separate if()
statements.

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

You could only set it to null if it was already null but if it already is then
why bother, pointless. The addition of this if statement (!certificate) could
arguably also be called pointless and I added it at a time when I had to do
certificate->identity() (and wanted to avoid a conditional operator) but that is
no longer the case. Also, I think this function is never called with a null
certificate. Pointless all around.

Anyway, I pointlessly made it two if statements for clarity.

[tommi, 2015/08/25 16:34:53]

point(less) taken

     return false;
-  identity_ = identity;
+  certificate_ = certificate;
   for (TransportMap::iterator iter = transports_.begin();
        iter != transports_.end(); ++iter) {
-    iter->second->SetIdentity(identity_);
+    iter->second->SetCertificate(certificate_);
   }
   return true;
 }
 
 bool BaseSession::SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version) {
   if (state_ != STATE_INIT) {
     return false;
   }
 
   ssl_max_version_ = version;
@@ skipping 126 matching lines @@
       this, &BaseSession::OnRoleConflict);
   transport->SignalCompleted.connect(
       this, &BaseSession::OnTransportCompleted);
   transport->SignalFailed.connect(
       this, &BaseSession::OnTransportFailed);
 
   transproxy = new TransportProxy(worker_thread_, sid_, content_name,
                                   new TransportWrapper(transport));
   transproxy->SignalCandidatesReady.connect(
       this, &BaseSession::OnTransportProxyCandidatesReady);
-  if (identity_)
-    transproxy->SetIdentity(identity_);
+  if (certificate_)
+    transproxy->SetCertificate(certificate_);
   transports_[content_name] = transproxy;
 
   return transproxy;
 }
 
 Transport* BaseSession::GetTransport(const std::string& content_name) {
   TransportProxy* transproxy = GetTransportProxy(content_name);
   if (transproxy == NULL)
     return NULL;
   return transproxy->impl();
@@ skipping 10 matching lines @@
   TransportMap::iterator iter = transports_.find(content_name);
   if (iter != transports_.end()) {
     delete iter->second;
     transports_.erase(content_name);
   }
 }
 
 Transport* BaseSession::CreateTransport(const std::string& content_name) {
   Transport* transport = new DtlsTransport<P2PTransport>(
       signaling_thread(), worker_thread(), content_name, port_allocator(),
-      identity_);
+      certificate_);
   transport->SetChannelReceivingTimeout(ice_receiving_timeout_);
   return transport;
 }
 
 void BaseSession::SetState(State state) {
   ASSERT(signaling_thread_->IsCurrent());
   if (state != state_) {
     LogState(state_, state);
     state_ = state;
     SignalState(this, state_);
@@ skipping 218 matching lines @@
 
     default:
       // Explicitly ignoring some states here.
       break;
     }
     break;
   }
 }
 
 }  // namespace cricket