Remove another ctor from BasicPortAllocator
This constructor isn't used in production. Removing it further
made the construction state of the class simpler, allowed for removal
of the separate Init() method and making more members const.
Bug: none
Change-Id: Ibc8516a01ce7e385207251d841d21bb7b72c9d9a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/318281
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40678}
diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc
index e0b837c..e414e3f 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -170,7 +170,7 @@
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 @@
std::vector<cricket::RelayServerConfig> turn_servers(1, turn_server);
std::unique_ptr<cricket::BasicPortAllocator> allocator =
- std::make_unique<cricket::BasicPortAllocator>(
- network_manager,
- std::make_unique<rtc::BasicPacketSocketFactory>(socket_server));
+ std::make_unique<cricket::BasicPortAllocator>(network_manager,
+ socket_factory);
allocator->Initialize();
allocator->SetConfiguration(stun_servers, turn_servers, 0, webrtc::NO_PRUNE);
return allocator.release();
@@ -282,6 +281,7 @@
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 @@
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 @@
std::unique_ptr<rtc::VirtualSocketServer> vss_;
std::unique_ptr<rtc::NATSocketServer> nss_;
std::unique_ptr<rtc::FirewallSocketServer> ss_;
+ std::unique_ptr<rtc::BasicPacketSocketFactory> socket_factory_;
+
rtc::AutoSocketServerThread main_;
rtc::scoped_refptr<PendingTaskSafetyFlag> safety_ =
PendingTaskSafetyFlag::Create();
@@ -5039,9 +5041,9 @@
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 ccdd37d..b6cbf1f 100644
--- a/p2p/client/basic_port_allocator.cc
+++ b/p2p/client/basic_port_allocator.cc
@@ -169,40 +169,32 @@
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<RelayServerConfig>(), 0,
webrtc::NO_PRUNE, customizer);
}
BasicPortAllocator::BasicPortAllocator(
rtc::NetworkManager* network_manager,
- std::unique_ptr<rtc::PacketSocketFactory> 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,
const ServerAddresses& stun_servers,
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<RelayServerConfig>(), 0,
webrtc::NO_PRUNE, nullptr);
}
@@ -276,15 +268,6 @@
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 7e30012..95bbdb1 100644
--- a/p2p/client/basic_port_allocator.h
+++ b/p2p/client/basic_port_allocator.h
@@ -41,10 +41,6 @@
webrtc::TurnCustomizer* customizer = nullptr,
RelayPortFactoryInterface* relay_port_factory = nullptr,
const webrtc::FieldTrialsView* field_trials = nullptr);
- BasicPortAllocator(
- rtc::NetworkManager* network_manager,
- std::unique_ptr<rtc::PacketSocketFactory> 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 @@
// creates its own socket factory.
rtc::PacketSocketFactory* socket_factory() {
CheckRunOnValidThreadIfInitialized();
- return socket_factory_.get();
+ return socket_factory_;
}
PortAllocatorSession* CreateSessionInternal(
@@ -91,24 +87,20 @@
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<const webrtc::FieldTrialsView,
webrtc::FieldTrialBasedConfig>
field_trials_;
rtc::NetworkManager* network_manager_;
- const webrtc::AlwaysValidPointerNoDefault<rtc::PacketSocketFactory>
- 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<RelayPortFactoryInterface> default_relay_port_factory_;
+ const std::unique_ptr<RelayPortFactoryInterface> 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 7b2bba4..55222a1 100644
--- a/p2p/client/basic_port_allocator_unittest.cc
+++ b/p2p/client/basic_port_allocator_unittest.cc
@@ -211,9 +211,8 @@
}
// Endpoint is on the public network. No STUN or TURN.
void ResetWithNoServersOrNat() {
- allocator_.reset(new BasicPortAllocator(
- &network_manager_,
- std::make_unique<rtc::BasicPacketSocketFactory>(fss_.get())));
+ allocator_.reset(
+ new BasicPortAllocator(&network_manager_, &socket_factory_));
allocator_->Initialize();
allocator_->SetIceTiebreaker(kTiebreakerDefault);
allocator_->set_step_delay(kMinimumStepDelay);
@@ -608,9 +607,8 @@
// Add two IP addresses on the same interface.
AddInterface(kClientAddr, "net1");
AddInterface(kClientIPv6Addr, "net1");
- allocator_.reset(new BasicPortAllocator(
- &network_manager_,
- std::make_unique<rtc::BasicPacketSocketFactory>(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 @@
bool tcp_pruned) {
turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP);
AddInterface(kClientAddr);
- allocator_.reset(new BasicPortAllocator(
- &network_manager_,
- std::make_unique<rtc::BasicPacketSocketFactory>(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 @@
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<rtc::BasicPacketSocketFactory>(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, TestSharedSocketWithoutNatUsingTurn) {
turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP);
AddInterface(kClientAddr);
- allocator_.reset(new BasicPortAllocator(
- &network_manager_,
- std::make_unique<rtc::BasicPacketSocketFactory>(fss_.get())));
+ allocator_.reset(new BasicPortAllocator(&network_manager_, &socket_factory_));
allocator_->Initialize();
AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr);
@@ -1810,9 +1804,7 @@
turn_server_.AddInternalSocket(rtc::SocketAddress("127.0.0.1", 3478),
PROTO_UDP);
AddInterface(kClientAddr);
- allocator_.reset(new BasicPortAllocator(
- &network_manager_,
- std::make_unique<rtc::BasicPacketSocketFactory>(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 715a800..e5ef16f 100644
--- a/pc/peer_connection_bundle_unittest.cc
+++ b/pc/peer_connection_bundle_unittest.cc
@@ -213,6 +213,7 @@
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 @@
WrapperPtr CreatePeerConnection(const RTCConfiguration& config) {
auto* fake_network = NewFakeNetwork();
auto port_allocator = std::make_unique<cricket::BasicPortAllocator>(
- fake_network,
- std::make_unique<rtc::BasicPacketSocketFactory>(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 @@
}
std::unique_ptr<rtc::VirtualSocketServer> vss_;
+ std::unique_ptr<rtc::BasicPacketSocketFactory> socket_factory_;
rtc::AutoSocketServerThread main_;
rtc::scoped_refptr<PeerConnectionFactoryInterface> pc_factory_;
std::vector<std::unique_ptr<rtc::FakeNetworkManager>> fake_networks_;
diff --git a/pc/peer_connection_histogram_unittest.cc b/pc/peer_connection_histogram_unittest.cc
index 0efc1c5..68a4dbc 100644
--- a/pc/peer_connection_histogram_unittest.cc
+++ b/pc/peer_connection_histogram_unittest.cc
@@ -242,7 +242,9 @@
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 @@
fake_network->AddInterface(NextLocalAddress());
std::unique_ptr<cricket::BasicPortAllocator> port_allocator(
- new cricket::BasicPortAllocator(
- fake_network,
- std::make_unique<rtc::BasicPacketSocketFactory>(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 @@
fake_network->AddInterface(kPrivateLocalAddress);
auto port_allocator = std::make_unique<cricket::BasicPortAllocator>(
- fake_network,
- std::make_unique<rtc::BasicPacketSocketFactory>(vss_.get()));
+ fake_network, socket_factory_.get());
RTCConfiguration config;
config.sdp_semantics = SdpSemantics::kUnifiedPlan;
return CreatePeerConnection(config,
@@ -310,8 +309,7 @@
fake_network->AddInterface(kPrivateIpv6LocalAddress);
auto port_allocator = std::make_unique<cricket::BasicPortAllocator>(
- fake_network,
- std::make_unique<rtc::BasicPacketSocketFactory>(vss_.get()));
+ fake_network, socket_factory_.get());
RTCConfiguration config;
config.sdp_semantics = SdpSemantics::kUnifiedPlan;
@@ -344,8 +342,7 @@
auto fake_network = NewFakeNetwork();
fake_network->AddInterface(NextLocalAddress());
deps.allocator = std::make_unique<cricket::BasicPortAllocator>(
- fake_network,
- std::make_unique<rtc::BasicPacketSocketFactory>(vss_.get()));
+ fake_network, socket_factory_.get());
}
auto observer = std::make_unique<ObserverForUsageHistogramTest>();
@@ -388,6 +385,7 @@
std::vector<std::unique_ptr<rtc::FakeNetworkManager>> fake_networks_;
int next_local_address_ = 0;
std::unique_ptr<rtc::VirtualSocketServer> vss_;
+ std::unique_ptr<rtc::BasicPacketSocketFactory> socket_factory_;
rtc::AutoSocketServerThread main_;
};
diff --git a/pc/peer_connection_ice_unittest.cc b/pc/peer_connection_ice_unittest.cc
index 6e823c8..532583f 100644
--- a/pc/peer_connection_ice_unittest.cc
+++ b/pc/peer_connection_ice_unittest.cc
@@ -149,6 +149,7 @@
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 @@
WrapperPtr CreatePeerConnection(const RTCConfiguration& config) {
auto* fake_network = NewFakeNetwork();
auto port_allocator = std::make_unique<cricket::BasicPortAllocator>(
- fake_network,
- std::make_unique<rtc::BasicPacketSocketFactory>(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 @@
}
std::unique_ptr<rtc::VirtualSocketServer> vss_;
+ std::unique_ptr<rtc::BasicPacketSocketFactory> socket_factory_;
rtc::AutoSocketServerThread main_;
rtc::scoped_refptr<PeerConnectionFactoryInterface> pc_factory_;
std::vector<std::unique_ptr<rtc::FakeNetworkManager>> fake_networks_;
diff --git a/pc/peer_connection_rampup_tests.cc b/pc/peer_connection_rampup_tests.cc
index 34b61b4..545a1d5 100644
--- a/pc/peer_connection_rampup_tests.cc
+++ b/pc/peer_connection_rampup_tests.cc
@@ -160,6 +160,8 @@
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 @@
auto observer = std::make_unique<MockPeerConnectionObserver>();
webrtc::PeerConnectionDependencies dependencies(observer.get());
cricket::BasicPortAllocator* port_allocator =
- new cricket::BasicPortAllocator(
- fake_network_manager,
- std::make_unique<rtc::BasicPacketSocketFactory>(
- 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<cricket::BasicPortAllocator>(port_allocator);
@@ -344,6 +345,8 @@
// up queue.
std::unique_ptr<rtc::VirtualSocketServer> virtual_socket_server_;
std::unique_ptr<rtc::FirewallSocketServer> firewall_socket_server_;
+ std::unique_ptr<rtc::BasicPacketSocketFactory> firewall_socket_factory_;
+
std::unique_ptr<rtc::Thread> network_thread_;
std::unique_ptr<rtc::Thread> 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 c450ccb..43033d2 100644
--- a/pc/test/integration_test_helpers.h
+++ b/pc/test/integration_test_helpers.h
@@ -757,10 +757,11 @@
fake_network_manager_.reset(new rtc::FakeNetworkManager());
fake_network_manager_->AddInterface(kDefaultLocalAddress);
+ socket_factory_.reset(new rtc::BasicPacketSocketFactory(socket_server));
+
std::unique_ptr<cricket::PortAllocator> port_allocator(
- new cricket::BasicPortAllocator(
- fake_network_manager_.get(),
- std::make_unique<rtc::BasicPacketSocketFactory>(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 @@
std::string debug_name_;
std::unique_ptr<rtc::FakeNetworkManager> fake_network_manager_;
+ std::unique_ptr<rtc::BasicPacketSocketFactory> socket_factory_;
// Reference to the mDNS responder owned by `fake_network_manager_` after set.
webrtc::FakeMdnsResponder* mdns_responder_ = nullptr;
