Fixing some issues with ICE restart signaling.

talk/app/webrtc/webrtcsession.cc

Index: talk/app/webrtc/webrtcsession.cc
diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc
index 1fc2a37902740aeda83a8d3c3e58d90aa9d9117c..e8749bbd86483e23c00943b9b9c231851b1bc8df 100644
--- a/talk/app/webrtc/webrtcsession.cc
+++ b/talk/app/webrtc/webrtcsession.cc
@@ -474,11 +474,12 @@ uint32_t ConvertIceTransportTypeToCandidateFilter(
   return cricket::CF_NONE;
 }
 
-// Help class used to remember if a a remote peer has requested ice restart by
+// Helper class used to remember if a a remote peer has requested ICE restart by
 // by sending a description with 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
@@ -487,11 +488,7 @@ class IceRestartAnswerLatch {
     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) {
@@ -500,36 +497,34 @@ class IceRestartAnswerLatch {
     }
     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,
@@ -554,7 +549,12 @@ WebRtcSession::WebRtcSession(webrtc::MediaControllerInterface* media_controller,
       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(
@@ -835,6 +835,10 @@ bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc,
     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);
   }
@@ -878,20 +882,34 @@ bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* 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(), 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(), desc);
+        old_remote_desc.get(), cricket::MEDIA_TYPE_DATA, desc);
   }
 
   if (error() != ERROR_NONE) {
@@ -1475,12 +1493,16 @@ 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();
+}
+
+bool WebRtcSession::VideoIceRestartPending() const {
+  return video_ice_restart_latch_->Get();
 }
 
-void WebRtcSession::ResetIceRestartLatch() {
-  ice_restart_latch_->Reset();
+bool WebRtcSession::DataIceRestartPending() const {
+  return data_ice_restart_latch_->Get();
 }
 
 void WebRtcSession::OnCertificateReady(