diff --git a/p2p/base/fakeportallocator.h b/p2p/base/fakeportallocator.h index 76df3df693..3f0a1d4f8c 100644 --- a/p2p/base/fakeportallocator.h +++ b/p2p/base/fakeportallocator.h @@ -18,12 +18,11 @@ #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 { @@ -220,9 +219,12 @@ class FakePortAllocator : public cricket::PortAllocator { owned_factory_.reset(new rtc::BasicPacketSocketFactory(network_thread_)); factory_ = owned_factory_.get(); } - network_thread_->Invoke(RTC_FROM_HERE, - rtc::Bind(&PortAllocator::Initialize, - static_cast(this))); + } + + void Initialize() override { + // Port allocator should be initialized on the network thread. + RTC_CHECK(network_thread_->IsCurrent()); + initialized_ = true; } void SetNetworkIgnoreMask(int network_ignore_mask) override {} @@ -243,6 +245,7 @@ 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 9d9df530eb..2b0efacc2d 100644 --- a/p2p/base/p2ptransportchannel_unittest.cc +++ b/p2p/base/p2ptransportchannel_unittest.cc @@ -155,7 +155,6 @@ 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 d9383610d6..e766fa732c 100644 --- a/p2p/base/portallocator.cc +++ b/p2p/base/portallocator.cc @@ -107,19 +107,9 @@ PortAllocator::PortAllocator() max_ipv6_networks_(kDefaultMaxIPv6Networks), step_delay_(kDefaultStepDelay), allow_tcp_listen_(true), - candidate_filter_(CF_ALL) { - // The allocator will be attached to a thread in Initialize. - thread_checker_.DetachFromThread(); -} + candidate_filter_(CF_ALL) {} -void PortAllocator::Initialize() { - RTC_DCHECK(thread_checker_.CalledOnValidThread()); - initialized_ = true; -} - -PortAllocator::~PortAllocator() { - CheckRunOnValidThreadIfInitialized(); -} +PortAllocator::~PortAllocator() = default; bool PortAllocator::SetConfiguration( const ServerAddresses& stun_servers, @@ -128,7 +118,6 @@ 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; @@ -192,7 +181,6 @@ 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()); @@ -204,7 +192,6 @@ 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()) { @@ -221,7 +208,6 @@ std::unique_ptr PortAllocator::TakePooledSession( } const PortAllocatorSession* PortAllocator::GetPooledSession() const { - CheckRunOnValidThreadAndInitialized(); if (pooled_sessions_.empty()) { return nullptr; } @@ -229,18 +215,15 @@ 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 f985b2171b..95052c2db1 100644 --- a/p2p/base/portallocator.h +++ b/p2p/base/portallocator.h @@ -22,7 +22,6 @@ #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; @@ -327,18 +326,19 @@ class PortAllocatorSession : public sigslot::has_slots<> { }; // Every method of PortAllocator (including the destructor) must be called on -// the same thread after Initialize is called. +// the same thread, except for the constructor which may be called on any +// 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. +// This allows constructing a PortAllocator subclass on one thread and +// passing it into an object that uses it on a different thread. class PortAllocator : public sigslot::has_slots<> { public: PortAllocator(); ~PortAllocator() override; - // This MUST be called on the PortAllocator's thread after finishing - // constructing and configuring the PortAllocator subclasses. - virtual void Initialize(); + // This should be called on the PortAllocator's thread before the + // PortAllocator is used. Subclasses may override this if necessary. + virtual void Initialize() {} // Set STUN and TURN servers to be used in future sessions, and set // candidate pool size, as described in JSEP. @@ -360,23 +360,14 @@ class PortAllocator : public sigslot::has_slots<> { const rtc::Optional& stun_candidate_keepalive_interval = rtc::nullopt); - const ServerAddresses& stun_servers() const { - CheckRunOnValidThreadIfInitialized(); - return stun_servers_; - } + const ServerAddresses& stun_servers() const { return stun_servers_; } const std::vector& turn_servers() const { - CheckRunOnValidThreadIfInitialized(); return turn_servers_; } - int candidate_pool_size() const { - CheckRunOnValidThreadIfInitialized(); - return candidate_pool_size_; - } - + int candidate_pool_size() const { return candidate_pool_size_; } const rtc::Optional& stun_candidate_keepalive_interval() const { - CheckRunOnValidThreadIfInitialized(); return stun_candidate_keepalive_interval_; } @@ -419,48 +410,23 @@ class PortAllocator : public sigslot::has_slots<> { // Discard any remaining pooled sessions. void DiscardCandidatePool(); - uint32_t flags() const { - CheckRunOnValidThreadIfInitialized(); - return flags_; - } - - void set_flags(uint32_t flags) { - CheckRunOnValidThreadIfInitialized(); - flags_ = flags; - } + uint32_t flags() const { return flags_; } + void set_flags(uint32_t flags) { 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 { - CheckRunOnValidThreadIfInitialized(); - return agent_; - } - - const rtc::ProxyInfo& proxy() const { - CheckRunOnValidThreadIfInitialized(); - return proxy_; - } - + const std::string& user_agent() const { return agent_; } + const rtc::ProxyInfo& proxy() const { 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 { - CheckRunOnValidThreadIfInitialized(); - return min_port_; - } - - int max_port() const { - CheckRunOnValidThreadIfInitialized(); - return max_port_; - } - + int min_port() const { return min_port_; } + int max_port() const { return max_port_; } bool SetPortRange(int min_port, int max_port) { - CheckRunOnValidThreadIfInitialized(); if (min_port > max_port) { return false; } @@ -478,15 +444,8 @@ 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) { - CheckRunOnValidThreadIfInitialized(); - max_ipv6_networks_ = networks; - } - - int max_ipv6_networks() { - CheckRunOnValidThreadIfInitialized(); - return max_ipv6_networks_; - } + void set_max_ipv6_networks(int networks) { max_ipv6_networks_ = networks; } + int max_ipv6_networks() { return max_ipv6_networks_; } // Delay between different candidate gathering phases (UDP, TURN, TCP). // Defaults to 1 second, but PeerConnection sets it to 50ms. @@ -494,59 +453,30 @@ 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 { - 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_; - } + uint32_t step_delay() const { return step_delay_; } + void set_step_delay(uint32_t delay) { step_delay_ = delay; } + 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() { - CheckRunOnValidThreadIfInitialized(); - return candidate_filter_; - } - + uint32_t candidate_filter() { return candidate_filter_; } void set_candidate_filter(uint32_t filter) { - CheckRunOnValidThreadIfInitialized(); candidate_filter_ = filter; } - bool prune_turn_ports() const { - CheckRunOnValidThreadIfInitialized(); - return prune_turn_ports_; - } + bool prune_turn_ports() const { return prune_turn_ports_; } // Gets/Sets the Origin value used for WebRTC STUN requests. - const std::string& origin() const { - CheckRunOnValidThreadIfInitialized(); - return origin_; - } - - void set_origin(const std::string& origin) { - CheckRunOnValidThreadIfInitialized(); - origin_ = origin; - } + const std::string& origin() const { return origin_; } + void set_origin(const std::string& origin) { origin_ = origin; } void SetMetricsObserver(webrtc::MetricsObserverInterface* observer) { - CheckRunOnValidThreadIfInitialized(); metrics_observer_ = observer; } webrtc::TurnCustomizer* turn_customizer() { - CheckRunOnValidThreadIfInitialized(); return turn_customizer_; } @@ -573,17 +503,6 @@ 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_; @@ -594,7 +513,6 @@ 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 a25a937ddb..1b3947b6cf 100644 --- a/p2p/client/basicportallocator.cc +++ b/p2p/client/basicportallocator.cc @@ -186,7 +186,6 @@ 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(); @@ -196,14 +195,12 @@ 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, @@ -212,7 +209,6 @@ 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 1822b7da2f..fb74ba5d74 100644 --- a/p2p/client/basicportallocator.h +++ b/p2p/client/basicportallocator.h @@ -47,22 +47,13 @@ class BasicPortAllocator : public PortAllocator { // Set to kDefaultNetworkIgnoreMask by default. void SetNetworkIgnoreMask(int network_ignore_mask) override; - int network_ignore_mask() const { - CheckRunOnValidThreadIfInitialized(); - return network_ignore_mask_; - } + int network_ignore_mask() const { return network_ignore_mask_; } - rtc::NetworkManager* network_manager() const { - CheckRunOnValidThreadIfInitialized(); - return network_manager_; - } + rtc::NetworkManager* network_manager() const { return network_manager_; } // If socket_factory() is set to NULL each PortAllocatorSession // creates its own socket factory. - rtc::PacketSocketFactory* socket_factory() { - CheckRunOnValidThreadIfInitialized(); - return socket_factory_; - } + rtc::PacketSocketFactory* socket_factory() { return socket_factory_; } PortAllocatorSession* CreateSessionInternal( const std::string& content_name, @@ -74,7 +65,6 @@ 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 6ed13b5798..f39e61e3ee 100644 --- a/p2p/client/basicportallocator_unittest.cc +++ b/p2p/client/basicportallocator_unittest.cc @@ -166,7 +166,6 @@ class BasicPortAllocatorTestBase : public testing::Test, allocator_.reset(new BasicPortAllocator(&network_manager_, stun_servers, kRelayUdpIntAddr, kRelayTcpIntAddr, kRelaySslTcpIntAddr)); - allocator_->Initialize(); allocator_->set_step_delay(kMinimumStepDelay); } @@ -202,7 +201,6 @@ 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. @@ -485,8 +483,7 @@ class BasicPortAllocatorTestBase : public testing::Test, } allocator_.reset(new BasicPortAllocator( &network_manager_, nat_socket_factory_.get(), stun_servers)); - allocator_->Initialize(); - allocator_->set_step_delay(kMinimumStepDelay); + allocator().set_step_delay(kMinimumStepDelay); } std::unique_ptr vss_; @@ -582,7 +579,6 @@ 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()); @@ -620,7 +616,6 @@ 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); @@ -667,7 +662,6 @@ 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. @@ -1657,7 +1651,6 @@ TEST_F(BasicPortAllocatorTest, TestSharedSocketWithoutNatUsingTurn) { turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP); AddInterface(kClientAddr); allocator_.reset(new BasicPortAllocator(&network_manager_)); - allocator_->Initialize(); AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr); @@ -1762,7 +1755,6 @@ 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 90242a87a7..1f63874e3b 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -1980,6 +1980,11 @@ 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. @@ -2816,9 +2821,6 @@ 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 " @@ -2976,12 +2978,23 @@ bool PeerConnection::RemoveIceCandidates( void PeerConnection::RegisterUMAObserver(UMAObserver* observer) { TRACE_EVENT0("webrtc", "PeerConnection::RegisterUmaObserver"); - network_thread()->Invoke( - RTC_FROM_HERE, - rtc::Bind(&PeerConnection::SetMetricObserver_n, this, observer)); + 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_); + } + } + // Send information about IPv4/IPv6 status. if (uma_observer_) { - if (port_allocator_flags_ & cricket::PORTALLOCATOR_ENABLE_IPV6) { + port_allocator_->SetMetricsObserver(uma_observer_); + if (port_allocator_->flags() & cricket::PORTALLOCATOR_ENABLE_IPV6) { uma_observer_->IncrementEnumCounter( kEnumCounterAddressFamily, kPeerConnection_IPv6, kPeerConnectionAddressFamilyCounter_Max); @@ -3014,25 +3027,6 @@ 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( @@ -4619,43 +4613,44 @@ bool PeerConnection::InitializePortAllocator_n( } port_allocator_->Initialize(); + // To handle both internal and externally created port allocator, we will // enable BUNDLE here. - port_allocator_flags_ = port_allocator_->flags(); - port_allocator_flags_ |= cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET | - cricket::PORTALLOCATOR_ENABLE_IPV6 | - cricket::PORTALLOCATOR_ENABLE_IPV6_ON_WIFI; + int portallocator_flags = port_allocator_->flags(); + portallocator_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) { - port_allocator_flags_ &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6); + portallocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6); } else if (webrtc::field_trial::FindFullName("WebRTC-IPv6Default") .find("Disabled") == 0) { - port_allocator_flags_ &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6); + portallocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6); } if (configuration.disable_ipv6_on_wifi) { - port_allocator_flags_ &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6_ON_WIFI); + portallocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6_ON_WIFI); RTC_LOG(LS_INFO) << "IPv6 candidates on Wi-Fi are disabled."; } if (configuration.tcp_candidate_policy == kTcpCandidatePolicyDisabled) { - port_allocator_flags_ |= cricket::PORTALLOCATOR_DISABLE_TCP; + portallocator_flags |= cricket::PORTALLOCATOR_DISABLE_TCP; RTC_LOG(LS_INFO) << "TCP candidates are disabled."; } if (configuration.candidate_network_policy == kCandidateNetworkPolicyLowCost) { - port_allocator_flags_ |= cricket::PORTALLOCATOR_DISABLE_COSTLY_NETWORKS; + portallocator_flags |= cricket::PORTALLOCATOR_DISABLE_COSTLY_NETWORKS; RTC_LOG(LS_INFO) << "Do not gather candidates on high-cost networks"; } if (configuration.disable_link_local_networks) { - port_allocator_flags_ |= cricket::PORTALLOCATOR_DISABLE_LINK_LOCAL_NETWORKS; + portallocator_flags |= cricket::PORTALLOCATOR_DISABLE_LINK_LOCAL_NETWORKS; RTC_LOG(LS_INFO) << "Disable candidates on link-local network interfaces."; } - port_allocator_->set_flags(port_allocator_flags_); + port_allocator_->set_flags(portallocator_flags); // No step delay is used while allocating ports. port_allocator_->set_step_delay(cricket::kMinimumStepDelay); port_allocator_->set_candidate_filter( @@ -4681,12 +4676,6 @@ 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( @@ -5088,10 +5077,7 @@ rtc::Optional PeerConnection::sctp_transport_name() const { cricket::CandidateStatsList PeerConnection::GetPooledCandidateStats() const { cricket::CandidateStatsList candidate_states_list; - network_thread()->Invoke( - RTC_FROM_HERE, - rtc::Bind(&cricket::PortAllocator::GetCandidateStatsFromPooledSessions, - port_allocator_.get(), &candidate_states_list)); + port_allocator_->GetCandidateStatsFromPooledSessions(&candidate_states_list); return candidate_states_list; } diff --git a/pc/peerconnection.h b/pc/peerconnection.h index 401ffd49b4..fa46a4e151 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -654,8 +654,6 @@ 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, @@ -906,7 +904,6 @@ 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 f05ece4efb..f7ac830a65 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -3182,14 +3182,8 @@ class PeerConnectionIntegrationIceStatesTest } void SetPortAllocatorFlags() { - 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_)); + caller()->port_allocator()->set_flags(port_allocator_flags_); + callee()->port_allocator()->set_flags(port_allocator_flags_); } std::vector CallerAddresses() { diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc index acb0dbdd04..8ee39c945b 100644 --- a/pc/peerconnectioninterface_unittest.cc +++ b/pc/peerconnectioninterface_unittest.cc @@ -1414,6 +1414,31 @@ 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) {