Add signaling to support ICE renomination.

webrtc/api/peerconnection.cc

Index: webrtc/api/peerconnection.cc
diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc
index c675015ac9bbee705038fa997b716571eb342357..b8a8772b795459222a13b53eadd0e58b550b3424 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -513,16 +513,6 @@ bool ParseConstraintsForAnswer(const MediaConstraintsInterface* constraints,
     session_options->bundle_enabled = true;
   }
 
-  bool ice_restart = false;
-  if (FindConstraint(constraints, MediaConstraintsInterface::kIceRestart,
-                     &value, &mandatory_constraints_satisfied)) {
-    // kIceRestart defaults to false according to spec.
-    ice_restart = true;
-  }
-  for (auto& kv : session_options->transport_options) {
-    kv.second.ice_restart = ice_restart;
-  }
-

[pthatcher1, 2016/08/22 23:02:29]

There is so much refactoring in this CL now that it's hard to tell the what's a
behavior change and what's not.  Can we make refactorings like this a separate
CL from the behavior change?  

[honghaiz3, 2016/08/23 00:52:28]

Reverted this change. This part is basically a no-op anyway. 

   if (!constraints) {
     return true;
   }
@@ -631,6 +621,8 @@ bool PeerConnection::Initialize(
 
   stats_.reset(new StatsCollector(this));
 
+  ice_renomination_ = configuration.ice_renomination;
+
   // Initialize the WebRtcSession. It creates transport channels etc.
   if (!session_->Initialize(factory_->options(), std::move(cert_generator),
                             configuration)) {
@@ -1260,6 +1252,8 @@ bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration) {
 
   // TODO(deadbeef): Shouldn't have to hop to the worker thread twice...
   session_->SetIceConfig(session_->ParseIceConfig(configuration));
+
+  ice_renomination_ = configuration.ice_renomination;
   return true;
 }
 
@@ -1606,13 +1600,8 @@ bool PeerConnection::GetOptionsForOffer(
     cricket::MediaSessionOptions* session_options) {
   // TODO(deadbeef): Once we have transceivers, enumerate them here instead of
   // ContentInfos.
-  if (session_->local_description()) {
-    for (const cricket::ContentInfo& content :
-         session_->local_description()->description()->contents()) {
-      session_options->transport_options[content.name] =
-          cricket::TransportOptions();
-    }
-  }
+  InitializeMediaSessionOptions(true, session_options);

[pthatcher1, 2016/08/22 23:02:29]

Can you put a "bool is_offer = true" and then
InitializeMediaSessionOptions(is_offer, session_options)?  That would make it
easier to read.   Here and the other 2 places.

[honghaiz3, 2016/08/23 00:52:28]

This method is removed now. 

+
   if (!ExtractMediaSessionOptions(rtc_options, true, session_options)) {
     return false;
   }
@@ -1649,18 +1638,44 @@ bool PeerConnection::GetOptionsForOffer(
   return true;
 }
 
+void PeerConnection::InitializeMediaSessionOptions(
+    bool is_offer,
+    cricket::MediaSessionOptions* session_options) {
+  if (is_offer) {
+    if (session_->local_description()) {
+      for (const cricket::ContentInfo& content :
+           session_->local_description()->description()->contents()) {
+        session_options->transport_options[content.name] =
+            MakeTransportOptions();
+      }
+    }
+  } else {
+    session_options->recv_audio = false;
+    session_options->recv_video = false;
+    if (session_->remote_description()) {
+      // Initialize the transport_options map.
+      for (const cricket::ContentInfo& content :
+           session_->remote_description()->description()->contents()) {
+        session_options->transport_options[content.name] =
+            MakeTransportOptions();
+      }
+    }
+  }
+  // Add transport options so that ice_renomination can be set properly to the
+  // respective transport option.
+  if (session_options->transport_options.empty()) {
+    std::vector<std::string> content_names = {
+        cricket::CN_AUDIO, cricket::CN_VIDEO, cricket::CN_DATA};

[pthatcher1, 2016/08/22 23:02:29]

Isn't it kind of dangerous to assume we want these three content names?  Would
it be cleaner to add a MediaSessionOptions.ice_renomination?  Then in
MediaSessionDescriptionFactory::AddVideoContentForOffer (and similar), we could
have cricket::GetTransportOptions copy from MediaSessionOptions.ice_renomination
to TransportOptions.ice_renomination in one place, and not have to do it 3
places here.  And we wouldn't have to assume content names here.

[honghaiz3, 2016/08/23 00:52:27]

Done.

+    for (auto& content_name : content_names) {
+      session_options->transport_options[content_name] = MakeTransportOptions();
+    }
+  }
+}
+
 void PeerConnection::FinishOptionsForAnswer(
     cricket::MediaSessionOptions* session_options) {
   // TODO(deadbeef): Once we have transceivers, enumerate them here instead of
   // ContentInfos.
-  if (session_->remote_description()) {
-    // Initialize the transport_options map.
-    for (const cricket::ContentInfo& content :
-         session_->remote_description()->description()->contents()) {
-      session_options->transport_options[content.name] =
-          cricket::TransportOptions();
-    }
-  }
   AddSendStreams(session_options, senders_, rtp_data_channels_);
   session_options->bundle_enabled =
       session_options->bundle_enabled &&
@@ -1683,8 +1698,7 @@ void PeerConnection::FinishOptionsForAnswer(
 bool PeerConnection::GetOptionsForAnswer(
     const MediaConstraintsInterface* constraints,
     cricket::MediaSessionOptions* session_options) {
-  session_options->recv_audio = false;
-  session_options->recv_video = false;
+  InitializeMediaSessionOptions(false, session_options);
   if (!ParseConstraintsForAnswer(constraints, session_options)) {
     return false;
   }
@@ -1697,8 +1711,7 @@ bool PeerConnection::GetOptionsForAnswer(
 bool PeerConnection::GetOptionsForAnswer(
     const RTCOfferAnswerOptions& options,
     cricket::MediaSessionOptions* session_options) {
-  session_options->recv_audio = false;
-  session_options->recv_video = false;
+  InitializeMediaSessionOptions(false, session_options);
   if (!ExtractMediaSessionOptions(options, false, session_options)) {
     return false;
   }