Fixing some issues with ICE restart signaling.

talk/app/webrtc/webrtcsession.cc

 /*
  * libjingle
  * Copyright 2012 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 456 matching lines @@
         return cricket::CF_RELAY;
     case PeerConnectionInterface::kNoHost:
         return (cricket::CF_ALL & ~cricket::CF_HOST);
     case PeerConnectionInterface::kAll:
         return cricket::CF_ALL;
     default: ASSERT(false);
   }
   return cricket::CF_NONE;
 }
 
-// Help class used to remember if a a remote peer has requested ice restart by
-// by sending a description with new ice ufrag and password.
+// Helper class used to remember if a a remote peer has requested ICE restart
+// by sending a description with a new ice ufrag and password.
 class IceRestartAnswerLatch {
  public:
-  IceRestartAnswerLatch() : ice_restart_(false) { }
+  IceRestartAnswerLatch(cricket::MediaType media_type)
+      : ice_restart_(false), media_type_(media_type) {}
 
   // Returns true if CheckForRemoteIceRestart has been called with a new session
   // description where ice password and ufrag has changed since last time
   // Reset() was called.
   bool Get() const {
     return ice_restart_;
   }
 
-  void Reset() {
-    if (ice_restart_) {
-      ice_restart_ = false;
-    }
-  }
+  void Reset() { ice_restart_ = false; }
 
   bool CheckForRemoteIceRestart(const SessionDescriptionInterface* old_desc,
                                 const SessionDescriptionInterface* new_desc) {
     if (!old_desc || new_desc->type() != SessionDescriptionInterface::kOffer) {
       return false;
     }
     const SessionDescription* new_sd = new_desc->description();
     const SessionDescription* old_sd = old_desc->description();
-    const ContentInfos& contents = new_sd->contents();
-    for (size_t index = 0; index < contents.size(); ++index) {
-      const ContentInfo* cinfo = &contents[index];
-      if (cinfo->rejected) {
-        continue;
-      }
-      // If the content isn't rejected, check if ufrag and password has
-      // changed.
-      const cricket::TransportDescription* new_transport_desc =
-          new_sd->GetTransportDescriptionByName(cinfo->name);
-      const cricket::TransportDescription* old_transport_desc =
-          old_sd->GetTransportDescriptionByName(cinfo->name);
-      if (!new_transport_desc || !old_transport_desc) {
-        // No transport description exist. This is not an ice restart.
-        continue;
-      }
-      if (cricket::IceCredentialsChanged(old_transport_desc->ice_ufrag,
-                                         old_transport_desc->ice_pwd,
-                                         new_transport_desc->ice_ufrag,
-                                         new_transport_desc->ice_pwd)) {
-        LOG(LS_INFO) << "Remote peer request ice restart.";
-        ice_restart_ = true;
-        return true;
-      }
+    const ContentInfo* cinfo =
+        GetFirstMediaContent(new_sd->contents(), media_type_);
+    if (!cinfo || cinfo->rejected) {
+      return false;
+    }
+    // If the content isn't rejected, check if ufrag and password has changed.
+    const cricket::TransportDescription* new_transport_desc =
+        new_sd->GetTransportDescriptionByName(cinfo->name);
+    const cricket::TransportDescription* old_transport_desc =
+        old_sd->GetTransportDescriptionByName(cinfo->name);
+    if (!new_transport_desc || !old_transport_desc) {
+      // No transport description exists. This is not an ICE restart.
+      return false;
+    }
+    if (cricket::IceCredentialsChanged(
+            old_transport_desc->ice_ufrag, old_transport_desc->ice_pwd,
+            new_transport_desc->ice_ufrag, new_transport_desc->ice_pwd)) {
+      LOG(LS_INFO) << "Remote peer requested ICE restart for media type: "
+                   << media_type_;
+      ice_restart_ = true;
+      return true;
     }
     return false;
   }
 
  private:
   bool ice_restart_;
+  cricket::MediaType media_type_;
 };
 
 WebRtcSession::WebRtcSession(webrtc::MediaControllerInterface* media_controller,
                              rtc::Thread* signaling_thread,
                              rtc::Thread* worker_thread,
                              cricket::PortAllocator* port_allocator)
     : signaling_thread_(signaling_thread),
       worker_thread_(worker_thread),
       port_allocator_(port_allocator),
       // RFC 3264: The numeric value of the session id and version in the
       // o line MUST be representable with a "64 bit signed integer".
       // Due to this constraint session id |sid_| is max limited to LLONG_MAX.
       sid_(rtc::ToString(rtc::CreateRandomId64() & LLONG_MAX)),
       transport_controller_(new cricket::TransportController(signaling_thread,
                                                              worker_thread,
                                                              port_allocator)),
       media_controller_(media_controller),
       channel_manager_(media_controller_->channel_manager()),
       ice_observer_(NULL),
       ice_connection_state_(PeerConnectionInterface::kIceConnectionNew),
       ice_connection_receiving_(true),
       older_version_remote_peer_(false),
       dtls_enabled_(false),
       data_channel_type_(cricket::DCT_NONE),
-      ice_restart_latch_(new IceRestartAnswerLatch),
+      audio_ice_restart_latch_(
+          new IceRestartAnswerLatch(cricket::MEDIA_TYPE_AUDIO)),
+      video_ice_restart_latch_(
+          new IceRestartAnswerLatch(cricket::MEDIA_TYPE_VIDEO)),
+      data_ice_restart_latch_(
+          new IceRestartAnswerLatch(cricket::MEDIA_TYPE_DATA)),
       metrics_observer_(NULL) {
   transport_controller_->SetIceRole(cricket::ICEROLE_CONTROLLED);
   transport_controller_->SignalConnectionState.connect(
       this, &WebRtcSession::OnTransportControllerConnectionState);
   transport_controller_->SignalReceiving.connect(
       this, &WebRtcSession::OnTransportControllerReceiving);
   transport_controller_->SignalGatheringState.connect(
       this, &WebRtcSession::OnTransportControllerGatheringState);
   transport_controller_->SignalCandidatesGathered.connect(
       this, &WebRtcSession::OnTransportControllerCandidatesGathered);
@@ skipping 260 matching lines @@
 
   if (!UpdateSessionState(action, cricket::CS_LOCAL, err_desc)) {
     return false;
   }
 
   if (remote_desc_) {
     // Now that we have a local description, we can push down remote candidates.
     UseCandidatesInSessionDescription(remote_desc_.get());
   }
 
+  audio_ice_restart_latch_->Reset();
+  video_ice_restart_latch_->Reset();
+  data_ice_restart_latch_->Reset();
+
   if (error() != ERROR_NONE) {
     return BadLocalSdp(desc->type(), GetSessionErrorMsg(), err_desc);
   }
   return true;
 }
 
 bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc,
                                          std::string* err_desc) {
   ASSERT(signaling_thread()->IsCurrent());
 
@@ skipping 23 matching lines @@
   // NOTE: Candidates allocation will be initiated only when SetLocalDescription
   // is called.
   if (!UpdateSessionState(action, cricket::CS_REMOTE, err_desc)) {
     return false;
   }
 
   if (local_desc_ && !UseCandidatesInSessionDescription(desc)) {
     return BadRemoteSdp(desc->type(), kInvalidCandidates, err_desc);
   }
 
+  audio_ice_restart_latch_->Reset();
+  video_ice_restart_latch_->Reset();
+  data_ice_restart_latch_->Reset();
+
   // Check if this new SessionDescription contains new ice ufrag and password
   // that indicates the remote peer requests ice restart.
-  bool ice_restart =
-      ice_restart_latch_->CheckForRemoteIceRestart(old_remote_desc.get(), desc);
-  // We retain all received candidates only if ICE is not restarted.
-  // When ICE is restarted, all previous candidates belong to an old generation
-  // and should not be kept.
-  // TODO(deadbeef): This goes against the W3C spec which says the remote
-  // description should only contain candidates from the last set remote
-  // description plus any candidates added since then. We should remove this
-  // once we're sure it won't break anything.
-  if (!ice_restart) {
+  if (!audio_ice_restart_latch_->CheckForRemoteIceRestart(old_remote_desc.get(),
+                                                          desc)) {
+    // We retain all received candidates only if ICE is not restarted.
+    // When ICE is restarted, all previous candidates belong to an old
+    // generation
+    // and should not be kept.
+    // TODO(deadbeef): This goes against the W3C spec which says the remote
+    // description should only contain candidates from the last set remote
+    // description plus any candidates added since then. We should remove this
+    // once we're sure it won't break anything.
     WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription(
-        old_remote_desc.get(), desc);
+        old_remote_desc.get(), cricket::MEDIA_TYPE_AUDIO, desc);
+  }
+  if (!video_ice_restart_latch_->CheckForRemoteIceRestart(old_remote_desc.get(),
+                                                          desc)) {
+    WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription(
+        old_remote_desc.get(), cricket::MEDIA_TYPE_VIDEO, desc);
+  }
+  if (!data_ice_restart_latch_->CheckForRemoteIceRestart(old_remote_desc.get(),
+                                                         desc)) {
+    WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription(
+        old_remote_desc.get(), cricket::MEDIA_TYPE_DATA, desc);
   }
 
   if (error() != ERROR_NONE) {
     return BadRemoteSdp(desc->type(), GetSessionErrorMsg(), err_desc);
   }
 
   // Set the the ICE connection state to connecting since the connection may
   // become writable with peer reflexive candidates before any remote candidate
   // is signaled.
   // TODO(pthatcher): This is a short-term solution for crbug/446908. A real fix
@@ skipping 563 matching lines @@
 }
 
 bool WebRtcSession::ReadyToSendData() const {
   return data_channel_ && data_channel_->ready_to_send_data();
 }
 
 cricket::DataChannelType WebRtcSession::data_channel_type() const {
   return data_channel_type_;
 }
 
-bool WebRtcSession::IceRestartPending() const {
-  return ice_restart_latch_->Get();
+bool WebRtcSession::AudioIceRestartPending() const {
+  return audio_ice_restart_latch_->Get();
 }
 
-void WebRtcSession::ResetIceRestartLatch() {
-  ice_restart_latch_->Reset();
+bool WebRtcSession::VideoIceRestartPending() const {
+  return video_ice_restart_latch_->Get();
+}
+
+bool WebRtcSession::DataIceRestartPending() const {
+  return data_ice_restart_latch_->Get();
 }
 
 void WebRtcSession::OnCertificateReady(
     const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) {
   transport_controller_->SetLocalCertificate(certificate);
 }
 
 bool WebRtcSession::waiting_for_certificate_for_testing() const {
   return webrtc_session_desc_factory_->waiting_for_certificate_for_testing();
 }
@@ skipping 694 matching lines @@
   }
 }
 
 void WebRtcSession::OnSentPacket_w(cricket::TransportChannel* channel,
                                    const rtc::SentPacket& sent_packet) {
   RTC_DCHECK(worker_thread()->IsCurrent());
   media_controller_->call_w()->OnSentPacket(sent_packet);
 }
 
 }  // namespace webrtc