Relanding: Implement RTCConfiguration.iceCandidatePoolSize.

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 504 matching lines @@
   TRACE_EVENT0("webrtc", "PeerConnection::~PeerConnection");
   RTC_DCHECK(signaling_thread()->IsCurrent());
   // Need to detach RTP senders/receivers from WebRtcSession,
   // since it's about to be destroyed.
   for (const auto& sender : senders_) {
     sender->Stop();
   }
   for (const auto& receiver : receivers_) {
     receiver->Stop();
   }
+  // Destroy stats_ because it depends on session_.
+  stats_.reset(nullptr);
+  // Now destroy session_ before destroying other members,
+  // because its destruction fires signals (such as VoiceChannelDestroyed)
+  // which will trigger some final actions in PeerConnection...
+  session_.reset(nullptr);
+  // port_allocator_ lives on the worker thread and should be destroyed there.
+  worker_thread()->Invoke<void>([this] { port_allocator_.reset(nullptr); });
 }
 
 bool PeerConnection::Initialize(
     const PeerConnectionInterface::RTCConfiguration& configuration,
     std::unique_ptr<cricket::PortAllocator> allocator,
     std::unique_ptr<DtlsIdentityStoreInterface> dtls_identity_store,
     PeerConnectionObserver* observer) {
   TRACE_EVENT0("webrtc", "PeerConnection::Initialize");
   RTC_DCHECK(observer != nullptr);
   if (!observer) {
     return false;
   }
   observer_ = observer;
 
   port_allocator_ = std::move(allocator);
 
   cricket::ServerAddresses stun_servers;
   std::vector<cricket::RelayServerConfig> turn_servers;
   if (!ParseIceServers(configuration.servers, &stun_servers, &turn_servers)) {
     return false;
   }
-  port_allocator_->SetIceServers(stun_servers, turn_servers);
+  worker_thread()->Invoke<void>(rtc::Bind(
+      &cricket::PortAllocator::SetConfiguration, port_allocator_.get(),

[pthatcher1, 2016/05/05 21:51:24]

I'm a little confused.  Some things on PortAllocator are accessed on the worker
thread (SetConfiguration) and others on the signaling thread (flags, set_flags,
set_step_delay).  It would be really nice to have the port_allocator_ only
accessed on one thread, even if that means the port allocator does its own
thread hopping, or making all calls here do thread hopping. 

[Taylor Brandstetter, 2016/05/06 03:53:34]

That's the way it's always been. I agree it should be fixed, I just thought it
may be better done in a separate CL. It seems... *relatively* simple though so
I'll do it in this one (I can revert if you think it's too complex).

This makes me realize we need more PeerConnection tests. For example, making
sure the PeerConnection calls PortAllocator::SetConfiguration AFTER applying the
other settings to the port allocator. Don't l-g-t-m the CL until I've done that.

+      stun_servers, turn_servers, configuration.ice_candidate_pool_size));
 
   // To handle both internal and externally created port allocator, we will
   // enable BUNDLE here.
   int portallocator_flags = port_allocator_->flags();
   portallocator_flags |= cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET |
                          cricket::PORTALLOCATOR_ENABLE_IPV6;
   // If the disable-IPv6 flag was specified, we'll not override it
   // by experiment.
   if (configuration.disable_ipv6) {
     portallocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6);
@@ skipping 580 matching lines @@
     stats_->AddStream(new_stream);
     observer_->OnAddStream(new_stream);
   }
 
   UpdateEndedRemoteMediaStreams();
 
   SetSessionDescriptionMsg* msg = new SetSessionDescriptionMsg(observer);
   signaling_thread()->Post(this, MSG_SET_SESSIONDESCRIPTION_SUCCESS, msg);
 }
 
-bool PeerConnection::SetConfiguration(const RTCConfiguration& config) {
+bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration) {
   TRACE_EVENT0("webrtc", "PeerConnection::SetConfiguration");
   if (port_allocator_) {
     cricket::ServerAddresses stun_servers;
     std::vector<cricket::RelayServerConfig> turn_servers;
-    if (!ParseIceServers(config.servers, &stun_servers, &turn_servers)) {
+    if (!ParseIceServers(configuration.servers, &stun_servers, &turn_servers)) {
       return false;
     }
-    port_allocator_->SetIceServers(stun_servers, turn_servers);
+    worker_thread()->Invoke<void>(rtc::Bind(
+        &cricket::PortAllocator::SetConfiguration, port_allocator_.get(),
+        stun_servers, turn_servers, configuration.ice_candidate_pool_size));
   }
-  session_->SetIceConfig(session_->ParseIceConfig(config));
-  return session_->SetIceTransports(config.type);
+  session_->SetIceConfig(session_->ParseIceConfig(configuration));
+  return session_->SetIceTransports(configuration.type);
 }
 
 bool PeerConnection::AddIceCandidate(
     const IceCandidateInterface* ice_candidate) {
   TRACE_EVENT0("webrtc", "PeerConnection::AddIceCandidate");
   return session_->ProcessIceMessage(ice_candidate);
 }
 
 bool PeerConnection::RemoveIceCandidates(
     const std::vector<cricket::Candidate>& candidates) {
@@ skipping 889 matching lines @@
 DataChannel* PeerConnection::FindDataChannelBySid(int sid) const {
   for (const auto& channel : sctp_data_channels_) {
     if (channel->id() == sid) {
       return channel;
     }
   }
   return nullptr;
 }
 
 }  // namespace webrtc