Allow remote fingerprint update during a call

webrtc/p2p/base/dtlstransportchannel.cc

Index: webrtc/p2p/base/dtlstransportchannel.cc
diff --git a/webrtc/p2p/base/dtlstransportchannel.cc b/webrtc/p2p/base/dtlstransportchannel.cc
index 0c063e03230cf9cc85034a3484af9baeab2a3d89..51012a02f60d6262c21412310cd893501faf5e54 100644
--- a/webrtc/p2p/base/dtlstransportchannel.cc
+++ b/webrtc/p2p/base/dtlstransportchannel.cc
@@ -122,9 +122,12 @@ DtlsTransportChannelWrapper::~DtlsTransportChannelWrapper() {
 }
 
 void DtlsTransportChannelWrapper::Connect() {
-  // We should only get a single call to Connect.
   ASSERT(dtls_state() == DTLS_TRANSPORT_NEW);
-  channel_->Connect();
+  if (channel_->writable()) {
+    OnWritableState(channel_);
+  } else {
+    channel_->Connect();
+  }

[pthatcher1, 2015/11/18 20:42:43]

I think instead of changing the Connect method, we should make an explicit
Reconnect() method.  That way, it's clear that Connect is called once and
Reconnect() can be called many times.  Or just call channel_->Connect() below
instead of using Connect().

[guoweis_webrtc, 2015/11/25 21:03:13]

Done.

 }
 
 bool DtlsTransportChannelWrapper::SetLocalCertificate(
@@ -199,6 +202,8 @@ bool DtlsTransportChannelWrapper::SetRemoteFingerprint(
     size_t digest_len) {
   rtc::Buffer remote_fingerprint_value(digest, digest_len);
 
+  bool reconnect = false;
+
   if (dtls_active_ && remote_fingerprint_value_ == remote_fingerprint_value &&
       !digest_alg.empty()) {
     // This may happen during renegotiation.
@@ -208,7 +213,7 @@ bool DtlsTransportChannelWrapper::SetRemoteFingerprint(
 
   // Allow SetRemoteFingerprint with a NULL digest even if SetLocalCertificate
   // hasn't been called.
-  if (dtls_ || (!dtls_active_ && !digest_alg.empty())) {
+  if (!dtls_active_ && !digest_alg.empty()) {
     LOG_J(LS_ERROR, this) << "Can't set DTLS remote settings in this state.";
     return false;
   }
@@ -219,6 +224,11 @@ bool DtlsTransportChannelWrapper::SetRemoteFingerprint(
     return true;
   }
 
+  if (dtls_) {
+    RTC_DCHECK(remote_fingerprint_value_ != remote_fingerprint_value);
+    reconnect = true;
+  }

[pthatcher1, 2015/11/18 20:42:43]

I think this would be more readable as something like this:

if (remote_fingerprint_value_ == remote_fingerprint_value &&
    remote_fingerprint_algorithm_ == digest_alg) {
  return; 
}

remote_fingerprint_value_ = remote_fingerprint_value.Pass();
remote_fingerprint_algorithm_ = digest_alg;

if (dtls_) {
  SignalDtlsReset(this);
  set_dtls_state(DTLS_TRANSPORT_NEW);
  set_writable(false);
  Reconnect();
  return true;
}

if (!SetupDtls()) {
  set_dtls_state(DTLS_TRANSPORT_FAILED);
  return false;
}

return true;

[guoweis_webrtc, 2015/11/25 21:03:13]

Reconnect has to be called after SetupDtls

+
   // At this point we know we are doing DTLS
   remote_fingerprint_value_ = remote_fingerprint_value.Pass();
   remote_fingerprint_algorithm_ = digest_alg;
@@ -226,6 +236,16 @@ bool DtlsTransportChannelWrapper::SetRemoteFingerprint(
   if (!SetupDtls()) {
     set_dtls_state(DTLS_TRANSPORT_FAILED);
     return false;
+  } else {
+    // We get the Dtls newly set up here. Signal the media side to re-setup SRTP
+    // context.
+    SignalDtlsSrtpSetup(this);
+  }
+
+  if (reconnect) {
+    set_dtls_state(DTLS_TRANSPORT_NEW);
+    set_writable(false);
+    Connect();
   }
 
   return true;