Track pending ICE restarts independently for different media sections.

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 5957cae8aeb2bd8211d80f6e74c4a06e45339689..b37cc660b766e84d4ea149eab38e93ea3614308e 100644
--- a/talk/app/webrtc/webrtcsession.cc
+++ b/talk/app/webrtc/webrtcsession.cc
@@ -474,24 +474,21 @@ uint32_t ConvertIceTransportTypeToCandidateFilter(
   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 remote peer has requested an 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
+  // description where the 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; }
 
   // This method has two purposes: 1. Return whether |new_desc| requests
   // an ICE restart (i.e., new ufrag/pwd). 2. If it requests an ICE restart
@@ -504,38 +501,35 @@ 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.";
-        if (new_desc->type() == SessionDescriptionInterface::kOffer) {
-          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 request ice restart.";
+      if (new_desc->type() == SessionDescriptionInterface::kOffer) {
+        ice_restart_ = true;
       }
+      return true;
     }
     return false;
   }
 
  private:
   bool ice_restart_;
+  cricket::MediaType media_type_;
 };
 
 WebRtcSession::WebRtcSession(webrtc::MediaControllerInterface* media_controller,
@@ -560,7 +554,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(
@@ -812,6 +811,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);
   }
@@ -857,18 +860,31 @@ bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc,
 
   // 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) {
+  // TODO(deadbeef): When we start storing both the current and pending remote
+  // description, this should reset the latch and compare against the current
+  // description.
+  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

[honghaiz3, 2016/02/10 01:01:48]

This and the next line can be merged.

[Taylor Brandstetter, 2016/02/10 21:18:32]

Done.

+    // 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(), desc);
+        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) {
@@ -1465,12 +1481,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(