diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index e0b837ca5c..e414e3f558 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -170,7 +170,7 @@ cricket::Candidate CreateUdpCandidate(absl::string_view type, cricket::BasicPortAllocator* CreateBasicPortAllocator( rtc::NetworkManager* network_manager, - rtc::SocketServer* socket_server, + rtc::PacketSocketFactory* socket_factory, const cricket::ServerAddresses& stun_servers, const rtc::SocketAddress& turn_server_udp, const rtc::SocketAddress& turn_server_tcp) { @@ -187,9 +187,8 @@ cricket::BasicPortAllocator* CreateBasicPortAllocator( std::vector turn_servers(1, turn_server); std::unique_ptr allocator = - std::make_unique( - network_manager, - std::make_unique(socket_server)); + std::make_unique(network_manager, + socket_factory); allocator->Initialize(); allocator->SetConfiguration(stun_servers, turn_servers, 0, webrtc::NO_PRUNE); return allocator.release(); @@ -282,6 +281,7 @@ class P2PTransportChannelTestBase : public ::testing::Test, vss_(new rtc::VirtualSocketServer()), nss_(new rtc::NATSocketServer(vss_.get())), ss_(new rtc::FirewallSocketServer(nss_.get())), + socket_factory_(new rtc::BasicPacketSocketFactory(ss_.get())), main_(ss_.get()), stun_server_(TestStunServer::Create(ss_.get(), kStunAddr)), turn_server_(&main_, ss_.get(), kTurnUdpIntAddr, kTurnUdpExtAddr), @@ -300,11 +300,11 @@ class P2PTransportChannelTestBase : public ::testing::Test, ServerAddresses stun_servers; stun_servers.insert(kStunAddr); ep1_.allocator_.reset(CreateBasicPortAllocator( - &ep1_.network_manager_, ss_.get(), stun_servers, kTurnUdpIntAddr, - rtc::SocketAddress())); + &ep1_.network_manager_, socket_factory_.get(), stun_servers, + kTurnUdpIntAddr, rtc::SocketAddress())); ep2_.allocator_.reset(CreateBasicPortAllocator( - &ep2_.network_manager_, ss_.get(), stun_servers, kTurnUdpIntAddr, - rtc::SocketAddress())); + &ep2_.network_manager_, socket_factory_.get(), stun_servers, + kTurnUdpIntAddr, rtc::SocketAddress())); ep1_.SetIceTiebreaker(kTiebreakerDefault); ep1_.allocator_->SetIceTiebreaker(kTiebreakerDefault); @@ -1018,6 +1018,8 @@ class P2PTransportChannelTestBase : public ::testing::Test, std::unique_ptr vss_; std::unique_ptr nss_; std::unique_ptr ss_; + std::unique_ptr socket_factory_; + rtc::AutoSocketServerThread main_; rtc::scoped_refptr safety_ = PendingTaskSafetyFlag::Create(); @@ -5039,9 +5041,9 @@ class P2PTransportChannelMostLikelyToWorkFirstTest kTurnUdpIntAddr, kTurnUdpExtAddr) { network_manager_.AddInterface(kPublicAddrs[0]); - allocator_.reset( - CreateBasicPortAllocator(&network_manager_, ss(), ServerAddresses(), - kTurnUdpIntAddr, rtc::SocketAddress())); + allocator_.reset(CreateBasicPortAllocator( + &network_manager_, packet_socket_factory(), ServerAddresses(), + kTurnUdpIntAddr, rtc::SocketAddress())); allocator_->set_flags(allocator_->flags() | PORTALLOCATOR_DISABLE_STUN | PORTALLOCATOR_DISABLE_TCP); allocator_->set_step_delay(kMinimumStepDelay); diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc index ccdd37dc41..b6cbf1fff9 100644 --- a/p2p/client/basic_port_allocator.cc +++ b/p2p/client/basic_port_allocator.cc @@ -169,28 +169,19 @@ BasicPortAllocator::BasicPortAllocator( const webrtc::FieldTrialsView* field_trials) : field_trials_(field_trials), network_manager_(network_manager), - socket_factory_(socket_factory) { - Init(relay_port_factory); - RTC_DCHECK(relay_port_factory_ != nullptr); - RTC_DCHECK(network_manager_ != nullptr); - RTC_CHECK(socket_factory_ != nullptr); + socket_factory_(socket_factory), + default_relay_port_factory_(relay_port_factory ? nullptr + : new TurnPortFactory()), + relay_port_factory_(relay_port_factory + ? relay_port_factory + : default_relay_port_factory_.get()) { + RTC_CHECK(socket_factory_); + RTC_DCHECK(relay_port_factory_); + RTC_DCHECK(network_manager_); SetConfiguration(ServerAddresses(), std::vector(), 0, webrtc::NO_PRUNE, customizer); } -BasicPortAllocator::BasicPortAllocator( - rtc::NetworkManager* network_manager, - std::unique_ptr owned_socket_factory, - const webrtc::FieldTrialsView* field_trials) - : field_trials_(field_trials), - network_manager_(network_manager), - socket_factory_(std::move(owned_socket_factory)) { - Init(nullptr); - RTC_DCHECK(relay_port_factory_ != nullptr); - RTC_DCHECK(network_manager_ != nullptr); - RTC_CHECK(socket_factory_ != nullptr); -} - BasicPortAllocator::BasicPortAllocator( rtc::NetworkManager* network_manager, rtc::PacketSocketFactory* socket_factory, @@ -198,11 +189,12 @@ BasicPortAllocator::BasicPortAllocator( const webrtc::FieldTrialsView* field_trials) : field_trials_(field_trials), network_manager_(network_manager), - socket_factory_(socket_factory) { - Init(nullptr); - RTC_DCHECK(relay_port_factory_ != nullptr); - RTC_DCHECK(network_manager_ != nullptr); - RTC_CHECK(socket_factory_ != nullptr); + socket_factory_(socket_factory), + default_relay_port_factory_(new TurnPortFactory()), + relay_port_factory_(default_relay_port_factory_.get()) { + RTC_CHECK(socket_factory_); + RTC_DCHECK(relay_port_factory_); + RTC_DCHECK(network_manager_); SetConfiguration(stun_servers, std::vector(), 0, webrtc::NO_PRUNE, nullptr); } @@ -276,15 +268,6 @@ void BasicPortAllocator::AddTurnServerForTesting( turn_port_prune_policy(), turn_customizer()); } -void BasicPortAllocator::Init(RelayPortFactoryInterface* relay_port_factory) { - if (relay_port_factory != nullptr) { - relay_port_factory_ = relay_port_factory; - } else { - default_relay_port_factory_.reset(new TurnPortFactory()); - relay_port_factory_ = default_relay_port_factory_.get(); - } -} - // BasicPortAllocatorSession BasicPortAllocatorSession::BasicPortAllocatorSession( BasicPortAllocator* allocator, diff --git a/p2p/client/basic_port_allocator.h b/p2p/client/basic_port_allocator.h index 7e30012e47..95bbdb183e 100644 --- a/p2p/client/basic_port_allocator.h +++ b/p2p/client/basic_port_allocator.h @@ -41,10 +41,6 @@ class RTC_EXPORT BasicPortAllocator : public PortAllocator { webrtc::TurnCustomizer* customizer = nullptr, RelayPortFactoryInterface* relay_port_factory = nullptr, const webrtc::FieldTrialsView* field_trials = nullptr); - BasicPortAllocator( - rtc::NetworkManager* network_manager, - std::unique_ptr owned_socket_factory, - const webrtc::FieldTrialsView* field_trials = nullptr); BasicPortAllocator(rtc::NetworkManager* network_manager, rtc::PacketSocketFactory* socket_factory, const ServerAddresses& stun_servers, @@ -64,7 +60,7 @@ class RTC_EXPORT BasicPortAllocator : public PortAllocator { // creates its own socket factory. rtc::PacketSocketFactory* socket_factory() { CheckRunOnValidThreadIfInitialized(); - return socket_factory_.get(); + return socket_factory_; } PortAllocatorSession* CreateSessionInternal( @@ -91,24 +87,20 @@ class RTC_EXPORT BasicPortAllocator : public PortAllocator { void OnIceRegathering(PortAllocatorSession* session, IceRegatheringReason reason); - // This function makes sure that relay_port_factory_ is set properly. - void Init(RelayPortFactoryInterface* relay_port_factory); - bool MdnsObfuscationEnabled() const override; webrtc::AlwaysValidPointer field_trials_; rtc::NetworkManager* network_manager_; - const webrtc::AlwaysValidPointerNoDefault - socket_factory_; + // Always externally-owned pointer to a socket factory. + rtc::PacketSocketFactory* const socket_factory_; int network_ignore_mask_ = rtc::kDefaultNetworkIgnoreMask; - // This is the factory being used. - RelayPortFactoryInterface* relay_port_factory_; - // This instance is created if caller does pass a factory. - std::unique_ptr default_relay_port_factory_; + const std::unique_ptr default_relay_port_factory_; + // This is the factory being used. + RelayPortFactoryInterface* const relay_port_factory_; }; struct PortConfiguration; diff --git a/p2p/client/basic_port_allocator_unittest.cc b/p2p/client/basic_port_allocator_unittest.cc index 7b2bba4293..55222a1be2 100644 --- a/p2p/client/basic_port_allocator_unittest.cc +++ b/p2p/client/basic_port_allocator_unittest.cc @@ -211,9 +211,8 @@ class BasicPortAllocatorTestBase : public ::testing::Test, } // Endpoint is on the public network. No STUN or TURN. void ResetWithNoServersOrNat() { - allocator_.reset(new BasicPortAllocator( - &network_manager_, - std::make_unique(fss_.get()))); + allocator_.reset( + new BasicPortAllocator(&network_manager_, &socket_factory_)); allocator_->Initialize(); allocator_->SetIceTiebreaker(kTiebreakerDefault); allocator_->set_step_delay(kMinimumStepDelay); @@ -608,9 +607,8 @@ class BasicPortAllocatorTest : public FakeClockBase, // Add two IP addresses on the same interface. AddInterface(kClientAddr, "net1"); AddInterface(kClientIPv6Addr, "net1"); - allocator_.reset(new BasicPortAllocator( - &network_manager_, - std::make_unique(fss_.get()))); + allocator_.reset( + new BasicPortAllocator(&network_manager_, &socket_factory_)); allocator_->Initialize(); allocator_->SetConfiguration(allocator_->stun_servers(), allocator_->turn_servers(), 0, @@ -651,9 +649,8 @@ class BasicPortAllocatorTest : public FakeClockBase, bool tcp_pruned) { turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP); AddInterface(kClientAddr); - allocator_.reset(new BasicPortAllocator( - &network_manager_, - std::make_unique(fss_.get()))); + allocator_.reset( + new BasicPortAllocator(&network_manager_, &socket_factory_)); allocator_->Initialize(); allocator_->SetConfiguration(allocator_->stun_servers(), allocator_->turn_servers(), 0, prune_policy); @@ -705,9 +702,8 @@ class BasicPortAllocatorTest : public FakeClockBase, AddInterface(kClientIPv6Addr, "net1", rtc::ADAPTER_TYPE_WIFI); AddInterface(kClientAddr2, "net2", rtc::ADAPTER_TYPE_CELLULAR); AddInterface(kClientIPv6Addr2, "net2", rtc::ADAPTER_TYPE_CELLULAR); - allocator_.reset(new BasicPortAllocator( - &network_manager_, - std::make_unique(fss_.get()))); + allocator_.reset( + new BasicPortAllocator(&network_manager_, &socket_factory_)); allocator_->Initialize(); allocator_->SetConfiguration(allocator_->stun_servers(), allocator_->turn_servers(), 0, @@ -1673,9 +1669,7 @@ TEST_F(BasicPortAllocatorTest, TestSharedSocketWithNat) { TEST_F(BasicPortAllocatorTest, TestSharedSocketWithoutNatUsingTurn) { turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP); AddInterface(kClientAddr); - allocator_.reset(new BasicPortAllocator( - &network_manager_, - std::make_unique(fss_.get()))); + allocator_.reset(new BasicPortAllocator(&network_manager_, &socket_factory_)); allocator_->Initialize(); AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr); @@ -1810,9 +1804,7 @@ TEST_F(BasicPortAllocatorTestWithRealClock, turn_server_.AddInternalSocket(rtc::SocketAddress("127.0.0.1", 3478), PROTO_UDP); AddInterface(kClientAddr); - allocator_.reset(new BasicPortAllocator( - &network_manager_, - std::make_unique(fss_.get()))); + allocator_.reset(new BasicPortAllocator(&network_manager_, &socket_factory_)); allocator_->Initialize(); RelayServerConfig turn_server; RelayCredentials credentials(kTurnUsername, kTurnPassword); diff --git a/pc/peer_connection_bundle_unittest.cc b/pc/peer_connection_bundle_unittest.cc index 715a8002a2..e5ef16ff8a 100644 --- a/pc/peer_connection_bundle_unittest.cc +++ b/pc/peer_connection_bundle_unittest.cc @@ -213,6 +213,7 @@ class PeerConnectionBundleBaseTest : public ::testing::Test { explicit PeerConnectionBundleBaseTest(SdpSemantics sdp_semantics) : vss_(new rtc::VirtualSocketServer()), + socket_factory_(new rtc::BasicPacketSocketFactory(vss_.get())), main_(vss_.get()), sdp_semantics_(sdp_semantics) { #ifdef WEBRTC_ANDROID @@ -238,8 +239,7 @@ class PeerConnectionBundleBaseTest : public ::testing::Test { WrapperPtr CreatePeerConnection(const RTCConfiguration& config) { auto* fake_network = NewFakeNetwork(); auto port_allocator = std::make_unique( - fake_network, - std::make_unique(vss_.get())); + fake_network, socket_factory_.get()); port_allocator->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP | cricket::PORTALLOCATOR_DISABLE_RELAY); port_allocator->set_step_delay(cricket::kMinimumStepDelay); @@ -297,6 +297,7 @@ class PeerConnectionBundleBaseTest : public ::testing::Test { } std::unique_ptr vss_; + std::unique_ptr socket_factory_; rtc::AutoSocketServerThread main_; rtc::scoped_refptr pc_factory_; std::vector> fake_networks_; diff --git a/pc/peer_connection_histogram_unittest.cc b/pc/peer_connection_histogram_unittest.cc index 0efc1c5744..68a4dbc361 100644 --- a/pc/peer_connection_histogram_unittest.cc +++ b/pc/peer_connection_histogram_unittest.cc @@ -242,7 +242,9 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test { WrapperPtr; PeerConnectionUsageHistogramTest() - : vss_(new rtc::VirtualSocketServer()), main_(vss_.get()) { + : vss_(new rtc::VirtualSocketServer()), + socket_factory_(new rtc::BasicPacketSocketFactory(vss_.get())), + main_(vss_.get()) { webrtc::metrics::Reset(); } @@ -270,9 +272,7 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test { fake_network->AddInterface(NextLocalAddress()); std::unique_ptr port_allocator( - new cricket::BasicPortAllocator( - fake_network, - std::make_unique(vss_.get()))); + new cricket::BasicPortAllocator(fake_network, socket_factory_.get())); deps.async_dns_resolver_factory = std::move(resolver_factory); deps.allocator = std::move(port_allocator); @@ -295,8 +295,7 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test { fake_network->AddInterface(kPrivateLocalAddress); auto port_allocator = std::make_unique( - fake_network, - std::make_unique(vss_.get())); + fake_network, socket_factory_.get()); RTCConfiguration config; config.sdp_semantics = SdpSemantics::kUnifiedPlan; return CreatePeerConnection(config, @@ -310,8 +309,7 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test { fake_network->AddInterface(kPrivateIpv6LocalAddress); auto port_allocator = std::make_unique( - fake_network, - std::make_unique(vss_.get())); + fake_network, socket_factory_.get()); RTCConfiguration config; config.sdp_semantics = SdpSemantics::kUnifiedPlan; @@ -344,8 +342,7 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test { auto fake_network = NewFakeNetwork(); fake_network->AddInterface(NextLocalAddress()); deps.allocator = std::make_unique( - fake_network, - std::make_unique(vss_.get())); + fake_network, socket_factory_.get()); } auto observer = std::make_unique(); @@ -388,6 +385,7 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test { std::vector> fake_networks_; int next_local_address_ = 0; std::unique_ptr vss_; + std::unique_ptr socket_factory_; rtc::AutoSocketServerThread main_; }; diff --git a/pc/peer_connection_ice_unittest.cc b/pc/peer_connection_ice_unittest.cc index 6e823c8934..532583f307 100644 --- a/pc/peer_connection_ice_unittest.cc +++ b/pc/peer_connection_ice_unittest.cc @@ -149,6 +149,7 @@ class PeerConnectionIceBaseTest : public ::testing::Test { explicit PeerConnectionIceBaseTest(SdpSemantics sdp_semantics) : vss_(new rtc::VirtualSocketServer()), + socket_factory_(new rtc::BasicPacketSocketFactory(vss_.get())), main_(vss_.get()), sdp_semantics_(sdp_semantics) { #ifdef WEBRTC_ANDROID @@ -174,8 +175,7 @@ class PeerConnectionIceBaseTest : public ::testing::Test { WrapperPtr CreatePeerConnection(const RTCConfiguration& config) { auto* fake_network = NewFakeNetwork(); auto port_allocator = std::make_unique( - fake_network, - std::make_unique(vss_.get())); + fake_network, socket_factory_.get()); port_allocator->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP | cricket::PORTALLOCATOR_DISABLE_RELAY); port_allocator->set_step_delay(cricket::kMinimumStepDelay); @@ -330,6 +330,7 @@ class PeerConnectionIceBaseTest : public ::testing::Test { } std::unique_ptr vss_; + std::unique_ptr socket_factory_; rtc::AutoSocketServerThread main_; rtc::scoped_refptr pc_factory_; std::vector> fake_networks_; diff --git a/pc/peer_connection_rampup_tests.cc b/pc/peer_connection_rampup_tests.cc index 34b61b4e14..545a1d53d0 100644 --- a/pc/peer_connection_rampup_tests.cc +++ b/pc/peer_connection_rampup_tests.cc @@ -160,6 +160,8 @@ class PeerConnectionRampUpTest : public ::testing::Test { virtual_socket_server_(new rtc::VirtualSocketServer()), firewall_socket_server_( new rtc::FirewallSocketServer(virtual_socket_server_.get())), + firewall_socket_factory_( + new rtc::BasicPacketSocketFactory(firewall_socket_server_.get())), network_thread_(new rtc::Thread(firewall_socket_server_.get())), worker_thread_(rtc::Thread::Create()) { network_thread_->SetName("PCNetworkThread", this); @@ -201,10 +203,9 @@ class PeerConnectionRampUpTest : public ::testing::Test { auto observer = std::make_unique(); webrtc::PeerConnectionDependencies dependencies(observer.get()); cricket::BasicPortAllocator* port_allocator = - new cricket::BasicPortAllocator( - fake_network_manager, - std::make_unique( - firewall_socket_server_.get())); + new cricket::BasicPortAllocator(fake_network_manager, + firewall_socket_factory_.get()); + port_allocator->set_step_delay(cricket::kDefaultStepDelay); dependencies.allocator = std::unique_ptr(port_allocator); @@ -344,6 +345,8 @@ class PeerConnectionRampUpTest : public ::testing::Test { // up queue. std::unique_ptr virtual_socket_server_; std::unique_ptr firewall_socket_server_; + std::unique_ptr firewall_socket_factory_; + std::unique_ptr network_thread_; std::unique_ptr worker_thread_; // The `pc_factory` uses `network_thread_` & `worker_thread_`, so it must be diff --git a/pc/test/integration_test_helpers.h b/pc/test/integration_test_helpers.h index c450ccba26..43033d2a5c 100644 --- a/pc/test/integration_test_helpers.h +++ b/pc/test/integration_test_helpers.h @@ -757,10 +757,11 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver, fake_network_manager_.reset(new rtc::FakeNetworkManager()); fake_network_manager_->AddInterface(kDefaultLocalAddress); + socket_factory_.reset(new rtc::BasicPacketSocketFactory(socket_server)); + std::unique_ptr port_allocator( - new cricket::BasicPortAllocator( - fake_network_manager_.get(), - std::make_unique(socket_server))); + new cricket::BasicPortAllocator(fake_network_manager_.get(), + socket_factory_.get())); port_allocator_ = port_allocator.get(); fake_audio_capture_module_ = FakeAudioCaptureModule::Create(); if (!fake_audio_capture_module_) { @@ -1173,6 +1174,7 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver, std::string debug_name_; std::unique_ptr fake_network_manager_; + std::unique_ptr socket_factory_; // Reference to the mDNS responder owned by `fake_network_manager_` after set. webrtc::FakeMdnsResponder* mdns_responder_ = nullptr;