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}
This commit is contained in:
Tommi 2023-09-01 12:45:06 +02:00 committed by WebRTC LUCI CQ
parent 5866e1a0ed
commit 3756e29b15
9 changed files with 70 additions and 96 deletions

View File

@ -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<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 @@ 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<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 @@ 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);

View File

@ -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<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,
@ -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<RelayServerConfig>(), 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,

View File

@ -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<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 @@ 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<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;

View File

@ -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<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 @@ 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<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 @@ class BasicPortAllocatorTest : public FakeClockBase,
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 @@ 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<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, TestSharedSocketWithNat) {
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 @@ 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<rtc::BasicPacketSocketFactory>(fss_.get())));
allocator_.reset(new BasicPortAllocator(&network_manager_, &socket_factory_));
allocator_->Initialize();
RelayServerConfig turn_server;
RelayCredentials credentials(kTurnUsername, kTurnPassword);

View File

@ -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<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 @@ class PeerConnectionBundleBaseTest : public ::testing::Test {
}
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_;

View File

@ -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<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 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
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 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
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 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
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 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
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_;
};

View File

@ -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<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 @@ class PeerConnectionIceBaseTest : public ::testing::Test {
}
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_;

View File

@ -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<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 @@ class PeerConnectionRampUpTest : public ::testing::Test {
// 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

View File

@ -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<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 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
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;