diff --git a/p2p/base/fakeportallocator.h b/p2p/base/fakeportallocator.h index 3f0a1d4f8c..76df3df693 100644 --- a/p2p/base/fakeportallocator.h +++ b/p2p/base/fakeportallocator.h @@ -18,11 +18,12 @@ #include "p2p/base/basicpacketsocketfactory.h" #include "p2p/base/portallocator.h" #include "p2p/base/udpport.h" +#include "rtc_base/bind.h" #include "rtc_base/nethelpers.h" +#include "rtc_base/thread.h" namespace rtc { class SocketFactory; -class Thread; } namespace cricket { @@ -219,12 +220,9 @@ class FakePortAllocator : public cricket::PortAllocator { owned_factory_.reset(new rtc::BasicPacketSocketFactory(network_thread_)); factory_ = owned_factory_.get(); } - } - - void Initialize() override { - // Port allocator should be initialized on the network thread. - RTC_CHECK(network_thread_->IsCurrent()); - initialized_ = true; + network_thread_->Invoke(RTC_FROM_HERE, + rtc::Bind(&PortAllocator::Initialize, + static_cast(this))); } void SetNetworkIgnoreMask(int network_ignore_mask) override {} @@ -245,7 +243,6 @@ class FakePortAllocator : public cricket::PortAllocator { rtc::Thread* network_thread_; rtc::PacketSocketFactory* factory_; std::unique_ptr owned_factory_; - bool initialized_ = false; }; } // namespace cricket diff --git a/p2p/base/p2ptransportchannel_unittest.cc b/p2p/base/p2ptransportchannel_unittest.cc index 2b0efacc2d..9d9df530eb 100644 --- a/p2p/base/p2ptransportchannel_unittest.cc +++ b/p2p/base/p2ptransportchannel_unittest.cc @@ -155,6 +155,7 @@ cricket::BasicPortAllocator* CreateBasicPortAllocator( cricket::BasicPortAllocator* allocator = new cricket::BasicPortAllocator(network_manager); + allocator->Initialize(); allocator->SetConfiguration(stun_servers, turn_servers, 0, false); return allocator; } diff --git a/p2p/base/portallocator.cc b/p2p/base/portallocator.cc index e766fa732c..d9383610d6 100644 --- a/p2p/base/portallocator.cc +++ b/p2p/base/portallocator.cc @@ -107,9 +107,19 @@ PortAllocator::PortAllocator() max_ipv6_networks_(kDefaultMaxIPv6Networks), step_delay_(kDefaultStepDelay), allow_tcp_listen_(true), - candidate_filter_(CF_ALL) {} + candidate_filter_(CF_ALL) { + // The allocator will be attached to a thread in Initialize. + thread_checker_.DetachFromThread(); +} -PortAllocator::~PortAllocator() = default; +void PortAllocator::Initialize() { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + initialized_ = true; +} + +PortAllocator::~PortAllocator() { + CheckRunOnValidThreadIfInitialized(); +} bool PortAllocator::SetConfiguration( const ServerAddresses& stun_servers, @@ -118,6 +128,7 @@ bool PortAllocator::SetConfiguration( bool prune_turn_ports, webrtc::TurnCustomizer* turn_customizer, const rtc::Optional& stun_candidate_keepalive_interval) { + CheckRunOnValidThreadIfInitialized(); bool ice_servers_changed = (stun_servers != stun_servers_ || turn_servers != turn_servers_); stun_servers_ = stun_servers; @@ -181,6 +192,7 @@ std::unique_ptr PortAllocator::CreateSession( int component, const std::string& ice_ufrag, const std::string& ice_pwd) { + CheckRunOnValidThreadAndInitialized(); auto session = std::unique_ptr( CreateSessionInternal(content_name, component, ice_ufrag, ice_pwd)); session->SetCandidateFilter(candidate_filter()); @@ -192,6 +204,7 @@ std::unique_ptr PortAllocator::TakePooledSession( int component, const std::string& ice_ufrag, const std::string& ice_pwd) { + CheckRunOnValidThreadAndInitialized(); RTC_DCHECK(!ice_ufrag.empty()); RTC_DCHECK(!ice_pwd.empty()); if (pooled_sessions_.empty()) { @@ -208,6 +221,7 @@ std::unique_ptr PortAllocator::TakePooledSession( } const PortAllocatorSession* PortAllocator::GetPooledSession() const { + CheckRunOnValidThreadAndInitialized(); if (pooled_sessions_.empty()) { return nullptr; } @@ -215,15 +229,18 @@ const PortAllocatorSession* PortAllocator::GetPooledSession() const { } void PortAllocator::FreezeCandidatePool() { + CheckRunOnValidThreadAndInitialized(); candidate_pool_frozen_ = true; } void PortAllocator::DiscardCandidatePool() { + CheckRunOnValidThreadIfInitialized(); pooled_sessions_.clear(); } void PortAllocator::GetCandidateStatsFromPooledSessions( CandidateStatsList* candidate_stats_list) { + CheckRunOnValidThreadAndInitialized(); for (const auto& session : pooled_sessions()) { session->GetCandidateStatsFromReadyPorts(candidate_stats_list); } diff --git a/p2p/base/portallocator.h b/p2p/base/portallocator.h index 95052c2db1..f985b2171b 100644 --- a/p2p/base/portallocator.h +++ b/p2p/base/portallocator.h @@ -22,6 +22,7 @@ #include "rtc_base/proxyinfo.h" #include "rtc_base/sigslot.h" #include "rtc_base/thread.h" +#include "rtc_base/thread_checker.h" namespace webrtc { class MetricsObserverInterface; @@ -326,19 +327,18 @@ class PortAllocatorSession : public sigslot::has_slots<> { }; // Every method of PortAllocator (including the destructor) must be called on -// the same thread, except for the constructor which may be called on any -// thread. +// the same thread after Initialize is called. // -// This allows constructing a PortAllocator subclass on one thread and -// passing it into an object that uses it on a different thread. +// This allows a PortAllocator subclass to be constructed and configured on one +// thread, and passed into an object that uses it on a different thread. class PortAllocator : public sigslot::has_slots<> { public: PortAllocator(); ~PortAllocator() override; - // This should be called on the PortAllocator's thread before the - // PortAllocator is used. Subclasses may override this if necessary. - virtual void Initialize() {} + // This MUST be called on the PortAllocator's thread after finishing + // constructing and configuring the PortAllocator subclasses. + virtual void Initialize(); // Set STUN and TURN servers to be used in future sessions, and set // candidate pool size, as described in JSEP. @@ -360,14 +360,23 @@ class PortAllocator : public sigslot::has_slots<> { const rtc::Optional& stun_candidate_keepalive_interval = rtc::nullopt); - const ServerAddresses& stun_servers() const { return stun_servers_; } + const ServerAddresses& stun_servers() const { + CheckRunOnValidThreadIfInitialized(); + return stun_servers_; + } const std::vector& turn_servers() const { + CheckRunOnValidThreadIfInitialized(); return turn_servers_; } - int candidate_pool_size() const { return candidate_pool_size_; } + int candidate_pool_size() const { + CheckRunOnValidThreadIfInitialized(); + return candidate_pool_size_; + } + const rtc::Optional& stun_candidate_keepalive_interval() const { + CheckRunOnValidThreadIfInitialized(); return stun_candidate_keepalive_interval_; } @@ -410,23 +419,48 @@ class PortAllocator : public sigslot::has_slots<> { // Discard any remaining pooled sessions. void DiscardCandidatePool(); - uint32_t flags() const { return flags_; } - void set_flags(uint32_t flags) { flags_ = flags; } + uint32_t flags() const { + CheckRunOnValidThreadIfInitialized(); + return flags_; + } + + void set_flags(uint32_t flags) { + CheckRunOnValidThreadIfInitialized(); + flags_ = flags; + } // These three methods are deprecated. If connections need to go through a // proxy, the application should create a BasicPortAllocator given a custom // PacketSocketFactory that creates proxy sockets. - const std::string& user_agent() const { return agent_; } - const rtc::ProxyInfo& proxy() const { return proxy_; } + const std::string& user_agent() const { + CheckRunOnValidThreadIfInitialized(); + return agent_; + } + + const rtc::ProxyInfo& proxy() const { + CheckRunOnValidThreadIfInitialized(); + return proxy_; + } + void set_proxy(const std::string& agent, const rtc::ProxyInfo& proxy) { + CheckRunOnValidThreadIfInitialized(); agent_ = agent; proxy_ = proxy; } // Gets/Sets the port range to use when choosing client ports. - int min_port() const { return min_port_; } - int max_port() const { return max_port_; } + int min_port() const { + CheckRunOnValidThreadIfInitialized(); + return min_port_; + } + + int max_port() const { + CheckRunOnValidThreadIfInitialized(); + return max_port_; + } + bool SetPortRange(int min_port, int max_port) { + CheckRunOnValidThreadIfInitialized(); if (min_port > max_port) { return false; } @@ -444,8 +478,15 @@ class PortAllocator : public sigslot::has_slots<> { // ICE down. We should work on making our ICE logic smarter (for example, // prioritizing pinging connections that are most likely to work) so that // every network interface can be used without impacting ICE's speed. - void set_max_ipv6_networks(int networks) { max_ipv6_networks_ = networks; } - int max_ipv6_networks() { return max_ipv6_networks_; } + void set_max_ipv6_networks(int networks) { + CheckRunOnValidThreadIfInitialized(); + max_ipv6_networks_ = networks; + } + + int max_ipv6_networks() { + CheckRunOnValidThreadIfInitialized(); + return max_ipv6_networks_; + } // Delay between different candidate gathering phases (UDP, TURN, TCP). // Defaults to 1 second, but PeerConnection sets it to 50ms. @@ -453,30 +494,59 @@ class PortAllocator : public sigslot::has_slots<> { // STUN transactions at once, but that's already happening if you configure // multiple STUN servers or have multiple network interfaces. We should // implement some global pacing logic instead if that's our goal. - uint32_t step_delay() const { return step_delay_; } - void set_step_delay(uint32_t delay) { step_delay_ = delay; } + uint32_t step_delay() const { + CheckRunOnValidThreadIfInitialized(); + return step_delay_; + } + + void set_step_delay(uint32_t delay) { + CheckRunOnValidThreadIfInitialized(); + step_delay_ = delay; + } + + bool allow_tcp_listen() const { + CheckRunOnValidThreadIfInitialized(); + return allow_tcp_listen_; + } - bool allow_tcp_listen() const { return allow_tcp_listen_; } void set_allow_tcp_listen(bool allow_tcp_listen) { + CheckRunOnValidThreadIfInitialized(); allow_tcp_listen_ = allow_tcp_listen; } - uint32_t candidate_filter() { return candidate_filter_; } + uint32_t candidate_filter() { + CheckRunOnValidThreadIfInitialized(); + return candidate_filter_; + } + void set_candidate_filter(uint32_t filter) { + CheckRunOnValidThreadIfInitialized(); candidate_filter_ = filter; } - bool prune_turn_ports() const { return prune_turn_ports_; } + bool prune_turn_ports() const { + CheckRunOnValidThreadIfInitialized(); + return prune_turn_ports_; + } // Gets/Sets the Origin value used for WebRTC STUN requests. - const std::string& origin() const { return origin_; } - void set_origin(const std::string& origin) { origin_ = origin; } + const std::string& origin() const { + CheckRunOnValidThreadIfInitialized(); + return origin_; + } + + void set_origin(const std::string& origin) { + CheckRunOnValidThreadIfInitialized(); + origin_ = origin; + } void SetMetricsObserver(webrtc::MetricsObserverInterface* observer) { + CheckRunOnValidThreadIfInitialized(); metrics_observer_ = observer; } webrtc::TurnCustomizer* turn_customizer() { + CheckRunOnValidThreadIfInitialized(); return turn_customizer_; } @@ -503,6 +573,17 @@ class PortAllocator : public sigslot::has_slots<> { return pooled_sessions_; } + // The following thread checks are only done in DCHECK for the consistency + // with the exsiting thread checks. + void CheckRunOnValidThreadIfInitialized() const { + RTC_DCHECK(!initialized_ || thread_checker_.CalledOnValidThread()); + } + + void CheckRunOnValidThreadAndInitialized() const { + RTC_DCHECK(initialized_ && thread_checker_.CalledOnValidThread()); + } + + bool initialized_ = false; uint32_t flags_; std::string agent_; rtc::ProxyInfo proxy_; @@ -513,6 +594,7 @@ class PortAllocator : public sigslot::has_slots<> { bool allow_tcp_listen_; uint32_t candidate_filter_; std::string origin_; + rtc::ThreadChecker thread_checker_; private: ServerAddresses stun_servers_; diff --git a/p2p/client/basicportallocator.cc b/p2p/client/basicportallocator.cc index 1b3947b6cf..a25a937ddb 100644 --- a/p2p/client/basicportallocator.cc +++ b/p2p/client/basicportallocator.cc @@ -186,6 +186,7 @@ void BasicPortAllocator::OnIceRegathering(PortAllocatorSession* session, } BasicPortAllocator::~BasicPortAllocator() { + CheckRunOnValidThreadIfInitialized(); // Our created port allocator sessions depend on us, so destroy our remaining // pooled sessions before anything else. DiscardCandidatePool(); @@ -195,12 +196,14 @@ void BasicPortAllocator::SetNetworkIgnoreMask(int network_ignore_mask) { // TODO(phoglund): implement support for other types than loopback. // See https://code.google.com/p/webrtc/issues/detail?id=4288. // Then remove set_network_ignore_list from NetworkManager. + CheckRunOnValidThreadIfInitialized(); network_ignore_mask_ = network_ignore_mask; } PortAllocatorSession* BasicPortAllocator::CreateSessionInternal( const std::string& content_name, int component, const std::string& ice_ufrag, const std::string& ice_pwd) { + CheckRunOnValidThreadAndInitialized(); PortAllocatorSession* session = new BasicPortAllocatorSession( this, content_name, component, ice_ufrag, ice_pwd); session->SignalIceRegathering.connect(this, @@ -209,6 +212,7 @@ PortAllocatorSession* BasicPortAllocator::CreateSessionInternal( } void BasicPortAllocator::AddTurnServer(const RelayServerConfig& turn_server) { + CheckRunOnValidThreadAndInitialized(); std::vector new_turn_servers = turn_servers(); new_turn_servers.push_back(turn_server); SetConfiguration(stun_servers(), new_turn_servers, candidate_pool_size(), diff --git a/p2p/client/basicportallocator.h b/p2p/client/basicportallocator.h index fb74ba5d74..1822b7da2f 100644 --- a/p2p/client/basicportallocator.h +++ b/p2p/client/basicportallocator.h @@ -47,13 +47,22 @@ class BasicPortAllocator : public PortAllocator { // Set to kDefaultNetworkIgnoreMask by default. void SetNetworkIgnoreMask(int network_ignore_mask) override; - int network_ignore_mask() const { return network_ignore_mask_; } + int network_ignore_mask() const { + CheckRunOnValidThreadIfInitialized(); + return network_ignore_mask_; + } - rtc::NetworkManager* network_manager() const { return network_manager_; } + rtc::NetworkManager* network_manager() const { + CheckRunOnValidThreadIfInitialized(); + return network_manager_; + } // If socket_factory() is set to NULL each PortAllocatorSession // creates its own socket factory. - rtc::PacketSocketFactory* socket_factory() { return socket_factory_; } + rtc::PacketSocketFactory* socket_factory() { + CheckRunOnValidThreadIfInitialized(); + return socket_factory_; + } PortAllocatorSession* CreateSessionInternal( const std::string& content_name, @@ -65,6 +74,7 @@ class BasicPortAllocator : public PortAllocator { void AddTurnServer(const RelayServerConfig& turn_server); RelayPortFactoryInterface* relay_port_factory() { + CheckRunOnValidThreadIfInitialized(); return relay_port_factory_; } diff --git a/p2p/client/basicportallocator_unittest.cc b/p2p/client/basicportallocator_unittest.cc index f39e61e3ee..6ed13b5798 100644 --- a/p2p/client/basicportallocator_unittest.cc +++ b/p2p/client/basicportallocator_unittest.cc @@ -166,6 +166,7 @@ class BasicPortAllocatorTestBase : public testing::Test, allocator_.reset(new BasicPortAllocator(&network_manager_, stun_servers, kRelayUdpIntAddr, kRelayTcpIntAddr, kRelaySslTcpIntAddr)); + allocator_->Initialize(); allocator_->set_step_delay(kMinimumStepDelay); } @@ -201,6 +202,7 @@ class BasicPortAllocatorTestBase : public testing::Test, // Endpoint is on the public network. No STUN or TURN. void ResetWithNoServersOrNat() { allocator_.reset(new BasicPortAllocator(&network_manager_)); + allocator_->Initialize(); allocator_->set_step_delay(kMinimumStepDelay); } // Endpoint is behind a NAT, with STUN specified. @@ -483,7 +485,8 @@ class BasicPortAllocatorTestBase : public testing::Test, } allocator_.reset(new BasicPortAllocator( &network_manager_, nat_socket_factory_.get(), stun_servers)); - allocator().set_step_delay(kMinimumStepDelay); + allocator_->Initialize(); + allocator_->set_step_delay(kMinimumStepDelay); } std::unique_ptr vss_; @@ -579,6 +582,7 @@ class BasicPortAllocatorTest : public FakeClockBase, AddInterface(kClientAddr, "net1"); AddInterface(kClientIPv6Addr, "net1"); allocator_.reset(new BasicPortAllocator(&network_manager_)); + allocator_->Initialize(); allocator_->SetConfiguration(allocator_->stun_servers(), allocator_->turn_servers(), 0, true); AddTurnServers(kTurnUdpIntIPv6Addr, rtc::SocketAddress()); @@ -616,6 +620,7 @@ class BasicPortAllocatorTest : public FakeClockBase, turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP); AddInterface(kClientAddr); allocator_.reset(new BasicPortAllocator(&network_manager_)); + allocator_->Initialize(); allocator_->SetConfiguration(allocator_->stun_servers(), allocator_->turn_servers(), 0, true); AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr); @@ -662,6 +667,7 @@ class BasicPortAllocatorTest : public FakeClockBase, AddInterface(kClientAddr2, "net2", rtc::ADAPTER_TYPE_CELLULAR); AddInterface(kClientIPv6Addr2, "net2", rtc::ADAPTER_TYPE_CELLULAR); allocator_.reset(new BasicPortAllocator(&network_manager_)); + allocator_->Initialize(); allocator_->SetConfiguration(allocator_->stun_servers(), allocator_->turn_servers(), 0, true); // Have both UDP/TCP and IPv4/IPv6 TURN ports. @@ -1651,6 +1657,7 @@ TEST_F(BasicPortAllocatorTest, TestSharedSocketWithoutNatUsingTurn) { turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP); AddInterface(kClientAddr); allocator_.reset(new BasicPortAllocator(&network_manager_)); + allocator_->Initialize(); AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr); @@ -1755,6 +1762,7 @@ TEST_F(BasicPortAllocatorTestWithRealClock, PROTO_UDP); AddInterface(kClientAddr); allocator_.reset(new BasicPortAllocator(&network_manager_)); + allocator_->Initialize(); RelayServerConfig turn_server(RELAY_TURN); RelayCredentials credentials(kTurnUsername, kTurnPassword); turn_server.credentials = credentials; diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 1f63874e3b..90242a87a7 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -1980,11 +1980,6 @@ void PeerConnection::SetLocalDescription( PostSetSessionDescriptionSuccess(observer); - // According to JSEP, after setLocalDescription, changing the candidate pool - // size is not allowed, and changing the set of ICE servers will not result - // in new candidates being gathered. - port_allocator_->FreezeCandidatePool(); - // MaybeStartGathering needs to be called after posting // MSG_SET_SESSIONDESCRIPTION_SUCCESS, so that we don't signal any candidates // before signaling that SetLocalDescription completed. @@ -2821,6 +2816,9 @@ bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration, RTCError* error) { TRACE_EVENT0("webrtc", "PeerConnection::SetConfiguration"); + // According to JSEP, after setLocalDescription, changing the candidate pool + // size is not allowed, and changing the set of ICE servers will not result + // in new candidates being gathered. if (local_description() && configuration.ice_candidate_pool_size != configuration_.ice_candidate_pool_size) { RTC_LOG(LS_ERROR) << "Can't change candidate pool size after calling " @@ -2978,23 +2976,12 @@ bool PeerConnection::RemoveIceCandidates( void PeerConnection::RegisterUMAObserver(UMAObserver* observer) { TRACE_EVENT0("webrtc", "PeerConnection::RegisterUmaObserver"); - uma_observer_ = observer; - - if (transport_controller()) { - transport_controller()->SetMetricsObserver(uma_observer_); - } - - for (auto transceiver : transceivers_) { - auto* channel = transceiver->internal()->channel(); - if (channel) { - channel->SetMetricsObserver(uma_observer_); - } - } - + network_thread()->Invoke( + RTC_FROM_HERE, + rtc::Bind(&PeerConnection::SetMetricObserver_n, this, observer)); // Send information about IPv4/IPv6 status. if (uma_observer_) { - port_allocator_->SetMetricsObserver(uma_observer_); - if (port_allocator_->flags() & cricket::PORTALLOCATOR_ENABLE_IPV6) { + if (port_allocator_flags_ & cricket::PORTALLOCATOR_ENABLE_IPV6) { uma_observer_->IncrementEnumCounter( kEnumCounterAddressFamily, kPeerConnection_IPv6, kPeerConnectionAddressFamilyCounter_Max); @@ -3027,6 +3014,25 @@ void PeerConnection::RegisterUMAObserver(UMAObserver* observer) { } } +void PeerConnection::SetMetricObserver_n(UMAObserver* observer) { + RTC_DCHECK(network_thread()->IsCurrent()); + uma_observer_ = observer; + if (transport_controller()) { + transport_controller()->SetMetricsObserver(uma_observer_); + } + + for (auto transceiver : transceivers_) { + auto* channel = transceiver->internal()->channel(); + if (channel) { + channel->SetMetricsObserver(uma_observer_); + } + } + + if (uma_observer_) { + port_allocator_->SetMetricsObserver(uma_observer_); + } +} + RTCError PeerConnection::SetBitrate(const BitrateParameters& bitrate) { if (!worker_thread()->IsCurrent()) { return worker_thread()->Invoke( @@ -4613,44 +4619,43 @@ bool PeerConnection::InitializePortAllocator_n( } port_allocator_->Initialize(); - // 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 | - cricket::PORTALLOCATOR_ENABLE_IPV6_ON_WIFI; + port_allocator_flags_ = port_allocator_->flags(); + port_allocator_flags_ |= cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET | + cricket::PORTALLOCATOR_ENABLE_IPV6 | + cricket::PORTALLOCATOR_ENABLE_IPV6_ON_WIFI; // If the disable-IPv6 flag was specified, we'll not override it // by experiment. if (configuration.disable_ipv6) { - portallocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6); + port_allocator_flags_ &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6); } else if (webrtc::field_trial::FindFullName("WebRTC-IPv6Default") .find("Disabled") == 0) { - portallocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6); + port_allocator_flags_ &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6); } if (configuration.disable_ipv6_on_wifi) { - portallocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6_ON_WIFI); + port_allocator_flags_ &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6_ON_WIFI); RTC_LOG(LS_INFO) << "IPv6 candidates on Wi-Fi are disabled."; } if (configuration.tcp_candidate_policy == kTcpCandidatePolicyDisabled) { - portallocator_flags |= cricket::PORTALLOCATOR_DISABLE_TCP; + port_allocator_flags_ |= cricket::PORTALLOCATOR_DISABLE_TCP; RTC_LOG(LS_INFO) << "TCP candidates are disabled."; } if (configuration.candidate_network_policy == kCandidateNetworkPolicyLowCost) { - portallocator_flags |= cricket::PORTALLOCATOR_DISABLE_COSTLY_NETWORKS; + port_allocator_flags_ |= cricket::PORTALLOCATOR_DISABLE_COSTLY_NETWORKS; RTC_LOG(LS_INFO) << "Do not gather candidates on high-cost networks"; } if (configuration.disable_link_local_networks) { - portallocator_flags |= cricket::PORTALLOCATOR_DISABLE_LINK_LOCAL_NETWORKS; + port_allocator_flags_ |= cricket::PORTALLOCATOR_DISABLE_LINK_LOCAL_NETWORKS; RTC_LOG(LS_INFO) << "Disable candidates on link-local network interfaces."; } - port_allocator_->set_flags(portallocator_flags); + port_allocator_->set_flags(port_allocator_flags_); // No step delay is used while allocating ports. port_allocator_->set_step_delay(cricket::kMinimumStepDelay); port_allocator_->set_candidate_filter( @@ -4676,6 +4681,12 @@ bool PeerConnection::ReconfigurePortAllocator_n( rtc::Optional stun_candidate_keepalive_interval) { port_allocator_->set_candidate_filter( ConvertIceTransportTypeToCandidateFilter(type)); + // According to JSEP, after setLocalDescription, changing the candidate pool + // size is not allowed, and changing the set of ICE servers will not result + // in new candidates being gathered. + if (local_description()) { + port_allocator_->FreezeCandidatePool(); + } // Call this last since it may create pooled allocator sessions using the // candidate filter set above. return port_allocator_->SetConfiguration( @@ -5077,7 +5088,10 @@ rtc::Optional PeerConnection::sctp_transport_name() const { cricket::CandidateStatsList PeerConnection::GetPooledCandidateStats() const { cricket::CandidateStatsList candidate_states_list; - port_allocator_->GetCandidateStatsFromPooledSessions(&candidate_states_list); + network_thread()->Invoke( + RTC_FROM_HERE, + rtc::Bind(&cricket::PortAllocator::GetCandidateStatsFromPooledSessions, + port_allocator_.get(), &candidate_states_list)); return candidate_states_list; } diff --git a/pc/peerconnection.h b/pc/peerconnection.h index fa46a4e151..401ffd49b4 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -654,6 +654,8 @@ class PeerConnection : public PeerConnectionInternal, webrtc::TurnCustomizer* turn_customizer, rtc::Optional stun_candidate_keepalive_interval); + void SetMetricObserver_n(UMAObserver* observer); + // Starts output of an RTC event log to the given output object. // This function should only be called from the worker thread. bool StartRtcEventLog_w(std::unique_ptr output, @@ -904,6 +906,7 @@ class PeerConnection : public PeerConnectionInternal, PeerConnectionInterface::RTCConfiguration configuration_; std::unique_ptr port_allocator_; + int port_allocator_flags_ = 0; // One PeerConnection has only one RTCP CNAME. // https://tools.ietf.org/html/draft-ietf-rtcweb-rtp-usage-26#section-4.9 diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index f7ac830a65..f05ece4efb 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -3182,8 +3182,14 @@ class PeerConnectionIntegrationIceStatesTest } void SetPortAllocatorFlags() { - caller()->port_allocator()->set_flags(port_allocator_flags_); - callee()->port_allocator()->set_flags(port_allocator_flags_); + network_thread()->Invoke( + RTC_FROM_HERE, + rtc::Bind(&cricket::PortAllocator::set_flags, + caller()->port_allocator(), port_allocator_flags_)); + network_thread()->Invoke( + RTC_FROM_HERE, + rtc::Bind(&cricket::PortAllocator::set_flags, + callee()->port_allocator(), port_allocator_flags_)); } std::vector CallerAddresses() { diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc index 8ee39c945b..acb0dbdd04 100644 --- a/pc/peerconnectioninterface_unittest.cc +++ b/pc/peerconnectioninterface_unittest.cc @@ -1414,31 +1414,6 @@ TEST_P(PeerConnectionInterfaceTest, EXPECT_TRUE(raw_port_allocator->prune_turn_ports()); } -// Test that the PeerConnection initializes the port allocator passed into it, -// and on the correct thread. -TEST_P(PeerConnectionInterfaceTest, - CreatePeerConnectionInitializesPortAllocatorOnNetworkThread) { - std::unique_ptr network_thread( - rtc::Thread::CreateWithSocketServer()); - network_thread->Start(); - rtc::scoped_refptr pc_factory( - webrtc::CreatePeerConnectionFactory( - network_thread.get(), rtc::Thread::Current(), rtc::Thread::Current(), - fake_audio_capture_module_, - webrtc::CreateBuiltinAudioEncoderFactory(), - webrtc::CreateBuiltinAudioDecoderFactory(), nullptr, nullptr)); - std::unique_ptr port_allocator( - new cricket::FakePortAllocator(network_thread.get(), nullptr)); - cricket::FakePortAllocator* raw_port_allocator = port_allocator.get(); - PeerConnectionInterface::RTCConfiguration config; - rtc::scoped_refptr pc( - pc_factory->CreatePeerConnection( - config, nullptr, std::move(port_allocator), nullptr, &observer_)); - // FakePortAllocator RTC_CHECKs that it's initialized on the right thread, - // so all we have to do here is check that it's initialized. - EXPECT_TRUE(raw_port_allocator->initialized()); -} - // Check that GetConfiguration returns the configuration the PeerConnection was // constructed with, before SetConfiguration is called. TEST_P(PeerConnectionInterfaceTest, GetConfigurationAfterCreatePeerConnection) {