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..544e13bac66f76c2116404c5718d7fc72daaa388 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -443,6 +443,7 @@ std::string GenerateRtcpCname() {
 bool ExtractMediaSessionOptions(
     const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options,
     bool is_offer,
+    bool ice_renomination,
     cricket::MediaSessionOptions* session_options) {
   typedef PeerConnectionInterface::RTCOfferAnswerOptions RTCOfferAnswerOptions;
   if (!IsValidOfferToReceiveMedia(rtc_options.offer_to_receive_audio) ||
@@ -468,8 +469,19 @@ bool ExtractMediaSessionOptions(
 
   session_options->vad_enabled = rtc_options.voice_activity_detection;
   session_options->bundle_enabled = rtc_options.use_rtp_mux;
+  // 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};

[Taylor Brandstetter, 2016/08/17 22:01:59]

This won't work interoperably. The offer could contain any content names;
"CN_AUDIO" etc. are just defaults.

I would recommend setting TransportOptions::ice_renomination where the
TransportOptions are initialized. Meaning in GetOptionsForOffer and
InitializeOptionsForAnswer (you're already doing the latter). There could be a
helper function "MakeTransportOptions" that just does:

TransportOptions options;
options.ice_renomination = ice_renomination_;
return options;

[honghaiz3, 2016/08/19 18:42:00]

I don't think that would work. 
When we initialize an answer, it can extract the content name from the remote
description. But when we initialize an offer, there is no existing local
description or remote one. 
I think "audio", "video", "data" might be default in the standard, but they are
hard-coded in the offer by webrtc and they are unlikely to change. If they will
ever change, the test I wrote should capture it. 
We could make it work with arbitrarily chosen content, but that will need some
convoluted changes. 

[Taylor Brandstetter, 2016/08/20 00:52:13]

Sorry, I just noticed that GetOptionsForOffer *does* add the content names from
the current local description. So there's nothing lacking.

I'd just suggest moving this code into GetOptionsForOffer where the other
TransportOptions are added, since I believe ExtractMediaSessionOptions is meant
as "extract media session options from RTCOfferAnswerOptions."

[honghaiz3, 2016/08/22 18:46:44]

Done.

+    for (auto& content_name : content_names) {
+      session_options->transport_options[content_name] =
+          cricket::TransportOptions();
+    }
+  }
   for (auto& kv : session_options->transport_options) {
     kv.second.ice_restart = rtc_options.ice_restart;
+    kv.second.ice_renomination = ice_renomination;
   }
 
   return true;
@@ -513,16 +525,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;
-  }
-
   if (!constraints) {
     return true;
   }
@@ -631,6 +633,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 +1264,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;
 }
 
@@ -1613,7 +1619,8 @@ bool PeerConnection::GetOptionsForOffer(
           cricket::TransportOptions();
     }
   }

[Taylor Brandstetter, 2016/08/20 00:52:13]

Here's where I think it would make sense to add the default TransportOptions
(for CN_AUDIO, CN_VIDEO) and initialize ice_renomination to the proper value.

-  if (!ExtractMediaSessionOptions(rtc_options, true, session_options)) {
+  if (!ExtractMediaSessionOptions(rtc_options, true, ice_renomination_,
+                                  session_options)) {
     return false;
   }
 
@@ -1649,18 +1656,25 @@ bool PeerConnection::GetOptionsForOffer(
   return true;
 }
 
-void PeerConnection::FinishOptionsForAnswer(
+void PeerConnection::InitializeOptionsForAnswer(
     cricket::MediaSessionOptions* session_options) {
-  // TODO(deadbeef): Once we have transceivers, enumerate them here instead of
-  // ContentInfos.
+  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] =
-          cricket::TransportOptions();
+      cricket::TransportOptions options;
+      options.ice_renomination = ice_renomination_;
+      session_options->transport_options[content.name] = options;
     }
   }
+}
+
+void PeerConnection::FinishOptionsForAnswer(
+    cricket::MediaSessionOptions* session_options) {
+  // TODO(deadbeef): Once we have transceivers, enumerate them here instead of
+  // ContentInfos.
   AddSendStreams(session_options, senders_, rtp_data_channels_);
   session_options->bundle_enabled =
       session_options->bundle_enabled &&
@@ -1683,8 +1697,7 @@ void PeerConnection::FinishOptionsForAnswer(
 bool PeerConnection::GetOptionsForAnswer(
     const MediaConstraintsInterface* constraints,
     cricket::MediaSessionOptions* session_options) {
-  session_options->recv_audio = false;
-  session_options->recv_video = false;
+  InitializeOptionsForAnswer(session_options);
   if (!ParseConstraintsForAnswer(constraints, session_options)) {
     return false;
   }
@@ -1697,9 +1710,9 @@ bool PeerConnection::GetOptionsForAnswer(
 bool PeerConnection::GetOptionsForAnswer(
     const RTCOfferAnswerOptions& options,
     cricket::MediaSessionOptions* session_options) {
-  session_options->recv_audio = false;
-  session_options->recv_video = false;
-  if (!ExtractMediaSessionOptions(options, false, session_options)) {
+  InitializeOptionsForAnswer(session_options);
+  if (!ExtractMediaSessionOptions(options, false, ice_renomination_,
+                                  session_options)) {
     return false;
   }
   session_options->rtcp_cname = rtcp_cname_;