Add signaling to support ICE renomination.

webrtc/api/peerconnection.cc

 /*
  *  Copyright 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 495 matching lines @@
   }
 
   if (FindConstraint(constraints, MediaConstraintsInterface::kUseRtpMux, &value,
                      &mandatory_constraints_satisfied)) {
     session_options->bundle_enabled = value;
   } else {
     // kUseRtpMux defaults to true according to spec.
     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;
   }
   return mandatory_constraints_satisfied == constraints->GetMandatory().size();
 }
 
 bool ParseIceServers(const PeerConnectionInterface::IceServers& servers,
                      cricket::ServerAddresses* stun_servers,
                      std::vector<cricket::RelayServerConfig>* turn_servers) {
   for (const webrtc::PeerConnectionInterface::IceServer& server : servers) {
@@ skipping 88 matching lines @@
 
   session_.reset(new WebRtcSession(
       media_controller_.get(), factory_->network_thread(),
       factory_->worker_thread(), factory_->signaling_thread(),
       port_allocator_.get(),
       std::unique_ptr<cricket::TransportController>(
           factory_->CreateTransportController(port_allocator_.get()))));
 
   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)) {
     return false;
   }
 
   // Register PeerConnection as receiver of local ice candidates.
   // All the callbacks will be posted to the application from PeerConnection.
   session_->RegisterIceObserver(this);
   session_->SignalState.connect(this, &PeerConnection::OnSessionStateChange);
@@ skipping 609 matching lines @@
     if (!network_thread()->Invoke<bool>(
             RTC_FROM_HERE,
             rtc::Bind(&PeerConnection::ReconfigurePortAllocator_n, this,
                       configuration))) {
       return false;
     }
   }
 
   // TODO(deadbeef): Shouldn't have to hop to the worker thread twice...
   session_->SetIceConfig(session_->ParseIceConfig(configuration));
+
+  ice_renomination_ = configuration.ice_renomination;
   return true;
 }
 
 bool PeerConnection::AddIceCandidate(
     const IceCandidateInterface* ice_candidate) {
   TRACE_EVENT0("webrtc", "PeerConnection::AddIceCandidate");
   if (IsClosed()) {
     return false;
   }
   return session_->ProcessIceMessage(ice_candidate);
@@ skipping 326 matching lines @@
   msg->error = error;
   signaling_thread()->Post(RTC_FROM_HERE, this,
                            MSG_CREATE_SESSIONDESCRIPTION_FAILED, msg);
 }
 
 bool PeerConnection::GetOptionsForOffer(
     const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options,
     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;
   }
 
   AddSendStreams(session_options, senders_, rtp_data_channels_);
   // Offer to receive audio/video if the constraint is not set and there are
   // send streams, or we're currently receiving.
   if (rtc_options.offer_to_receive_audio == RTCOfferAnswerOptions::kUndefined) {
     session_options->recv_audio =
         session_options->HasSendMediaStream(cricket::MEDIA_TYPE_AUDIO) ||
@@ skipping 16 matching lines @@
   // people won't try to use them.
   if (HasDataChannels() && session_->data_channel_type() != cricket::DCT_RTP) {
     session_options->data_channel_type = session_->data_channel_type();
   }
 
   session_options->rtcp_cname = rtcp_cname_;
   session_options->crypto_options = factory_->options().crypto_options;
   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 &&
       (session_options->has_audio() || session_options->has_video() ||
        session_options->has_data());
 
   // RTP data channel is handled in MediaSessionOptions::AddStream. SCTP streams
   // are not signaled in the SDP so does not go through that path and must be
   // handled here.
   // Intentionally unset the data channel type for RTP data channel. Otherwise
   // the RTP data channels would be successfully negotiated by default and the
   // unit tests in WebRtcDataBrowserTest will fail when building with chromium.
   // We want to leave RTP data channels broken, so people won't try to use them.
   if (session_->data_channel_type() != cricket::DCT_RTP) {
     session_options->data_channel_type = session_->data_channel_type();
   }
   session_options->crypto_options = factory_->options().crypto_options;
 }
 
 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;
   }
   session_options->rtcp_cname = rtcp_cname_;
 
   FinishOptionsForAnswer(session_options);
   return true;
 }
 
 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;
   }
   session_options->rtcp_cname = rtcp_cname_;
 
   FinishOptionsForAnswer(session_options);
   return true;
 }
 
 void PeerConnection::RemoveTracks(cricket::MediaType media_type) {
@@ skipping 611 matching lines @@
 
 bool PeerConnection::StartRtcEventLog_w(rtc::PlatformFile file,
                                         int64_t max_size_bytes) {
   return media_controller_->call_w()->StartEventLog(file, max_size_bytes);
 }
 
 void PeerConnection::StopRtcEventLog_w() {
   media_controller_->call_w()->StopEventLog();
 }
 }  // namespace webrtc