Delete BasicPacketSocketFactory constructor with thread argument
In callers where it's non-trivial to explicitly pass the right
SocketFactory, pull the call to rtc::Thread::socketserver() into the
caller, with a TODO comment.
Bug: webrtc:13145
Change-Id: I029d3adca385d822180e089f016c3778e0d4fd0c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231227
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35063}
diff --git a/p2p/base/basic_packet_socket_factory.cc b/p2p/base/basic_packet_socket_factory.cc
index d206497..c23b5d9 100644
--- a/p2p/base/basic_packet_socket_factory.cc
+++ b/p2p/base/basic_packet_socket_factory.cc
@@ -25,16 +25,12 @@
#include "rtc_base/socket_adapters.h"
#include "rtc_base/socket_server.h"
#include "rtc_base/ssl_adapter.h"
-#include "rtc_base/thread.h"
namespace rtc {
-BasicPacketSocketFactory::BasicPacketSocketFactory(Thread* thread)
- : thread_(thread), socket_factory_(NULL) {}
-
BasicPacketSocketFactory::BasicPacketSocketFactory(
SocketFactory* socket_factory)
- : thread_(NULL), socket_factory_(socket_factory) {}
+ : socket_factory_(socket_factory) {}
BasicPacketSocketFactory::~BasicPacketSocketFactory() {}
@@ -43,7 +39,7 @@
uint16_t min_port,
uint16_t max_port) {
// UDP sockets are simple.
- Socket* socket = socket_factory()->CreateSocket(address.family(), SOCK_DGRAM);
+ Socket* socket = socket_factory_->CreateSocket(address.family(), SOCK_DGRAM);
if (!socket) {
return NULL;
}
@@ -67,7 +63,7 @@
}
Socket* socket =
- socket_factory()->CreateSocket(local_address.family(), SOCK_STREAM);
+ socket_factory_->CreateSocket(local_address.family(), SOCK_STREAM);
if (!socket) {
return NULL;
}
@@ -104,7 +100,7 @@
const std::string& user_agent,
const PacketSocketTcpOptions& tcp_options) {
Socket* socket =
- socket_factory()->CreateSocket(local_address.family(), SOCK_STREAM);
+ socket_factory_->CreateSocket(local_address.family(), SOCK_STREAM);
if (!socket) {
return NULL;
}
@@ -215,13 +211,4 @@
return ret;
}
-SocketFactory* BasicPacketSocketFactory::socket_factory() {
- if (thread_) {
- RTC_DCHECK(thread_ == Thread::Current());
- return thread_->socketserver();
- } else {
- return socket_factory_;
- }
-}
-
} // namespace rtc
diff --git a/p2p/base/basic_packet_socket_factory.h b/p2p/base/basic_packet_socket_factory.h
index 368c976..030eb82 100644
--- a/p2p/base/basic_packet_socket_factory.h
+++ b/p2p/base/basic_packet_socket_factory.h
@@ -19,11 +19,9 @@
namespace rtc {
class SocketFactory;
-class Thread;
class BasicPacketSocketFactory : public PacketSocketFactory {
public:
- explicit BasicPacketSocketFactory(Thread* thread);
explicit BasicPacketSocketFactory(SocketFactory* socket_factory);
~BasicPacketSocketFactory() override;
@@ -49,9 +47,6 @@
uint16_t min_port,
uint16_t max_port);
- SocketFactory* socket_factory();
-
- Thread* thread_;
SocketFactory* socket_factory_;
};
diff --git a/p2p/base/fake_port_allocator.h b/p2p/base/fake_port_allocator.h
index efe9a53..83b27db 100644
--- a/p2p/base/fake_port_allocator.h
+++ b/p2p/base/fake_port_allocator.h
@@ -208,11 +208,13 @@
class FakePortAllocator : public cricket::PortAllocator {
public:
+ // TODO(bugs.webrtc.org/13145): Require non-null `factory`.
FakePortAllocator(rtc::Thread* network_thread,
rtc::PacketSocketFactory* factory)
: network_thread_(network_thread), factory_(factory) {
if (factory_ == NULL) {
- owned_factory_.reset(new rtc::BasicPacketSocketFactory(network_thread_));
+ owned_factory_.reset(new rtc::BasicPacketSocketFactory(
+ network_thread_ ? network_thread_->socketserver() : nullptr));
factory_ = owned_factory_.get();
}
diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc
index 7d7bfdb..9c4bc0f 100644
--- a/p2p/base/port_unittest.cc
+++ b/p2p/base/port_unittest.cc
@@ -395,7 +395,7 @@
PortTest()
: ss_(new rtc::VirtualSocketServer()),
main_(ss_.get()),
- socket_factory_(rtc::Thread::Current()),
+ socket_factory_(ss_.get()),
nat_factory1_(ss_.get(), kNatAddr1, SocketAddress()),
nat_factory2_(ss_.get(), kNatAddr2, SocketAddress()),
nat_socket_factory1_(&nat_factory1_),
diff --git a/p2p/base/stun_port_unittest.cc b/p2p/base/stun_port_unittest.cc
index 36733cc..5ee707b 100644
--- a/p2p/base/stun_port_unittest.cc
+++ b/p2p/base/stun_port_unittest.cc
@@ -48,7 +48,7 @@
: ss_(new rtc::VirtualSocketServer()),
thread_(ss_.get()),
network_("unittest", "unittest", kLocalAddr.ipaddr(), 32),
- socket_factory_(rtc::Thread::Current()),
+ socket_factory_(ss_.get()),
stun_server_1_(cricket::TestStunServer::Create(ss_.get(), kStunAddr1)),
stun_server_2_(cricket::TestStunServer::Create(ss_.get(), kStunAddr2)),
done_(false),
diff --git a/p2p/base/tcp_port_unittest.cc b/p2p/base/tcp_port_unittest.cc
index 2c9fbce..79f1314 100644
--- a/p2p/base/tcp_port_unittest.cc
+++ b/p2p/base/tcp_port_unittest.cc
@@ -61,7 +61,7 @@
TCPPortTest()
: ss_(new rtc::VirtualSocketServer()),
main_(ss_.get()),
- socket_factory_(rtc::Thread::Current()),
+ socket_factory_(ss_.get()),
username_(rtc::CreateRandomString(ICE_UFRAG_LENGTH)),
password_(rtc::CreateRandomString(ICE_PWD_LENGTH)) {}
diff --git a/p2p/base/test_turn_server.h b/p2p/base/test_turn_server.h
index 479ca8b..e1deb59 100644
--- a/p2p/base/test_turn_server.h
+++ b/p2p/base/test_turn_server.h
@@ -58,8 +58,11 @@
const std::string& common_name = "test turn server")
: server_(thread), thread_(thread) {
AddInternalSocket(int_addr, int_protocol, ignore_bad_cert, common_name);
- server_.SetExternalSocketFactory(new rtc::BasicPacketSocketFactory(thread),
- udp_ext_addr);
+ // TODO(bugs.webrtc.org/13145): Take a SocketFactory as argument, so we
+ // don't need thread_->socketserver().
+ server_.SetExternalSocketFactory(
+ new rtc::BasicPacketSocketFactory(thread_->socketserver()),
+ udp_ext_addr);
server_.set_realm(kTestRealm);
server_.set_software(kTestSoftware);
server_.set_auth_hook(this);
diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc
index 7a7092d..7854fdb 100644
--- a/p2p/base/turn_port_unittest.cc
+++ b/p2p/base/turn_port_unittest.cc
@@ -172,7 +172,7 @@
TurnPortTest()
: ss_(new TurnPortTestVirtualSocketServer()),
main_(ss_.get()),
- socket_factory_(rtc::Thread::Current()),
+ socket_factory_(ss_.get()),
turn_server_(&main_, kTurnUdpIntAddr, kTurnUdpExtAddr),
turn_ready_(false),
turn_error_(false),
diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc
index 6284625..2a601d5 100644
--- a/p2p/client/basic_port_allocator.cc
+++ b/p2p/client/basic_port_allocator.cc
@@ -378,7 +378,7 @@
state_ = SessionState::GATHERING;
if (!socket_factory_) {
owned_socket_factory_.reset(
- new rtc::BasicPacketSocketFactory(network_thread_));
+ new rtc::BasicPacketSocketFactory(network_thread_->socketserver()));
socket_factory_ = owned_socket_factory_.get();
}
diff --git a/pc/connection_context.cc b/pc/connection_context.cc
index 6fdcac3..691dd90 100644
--- a/pc/connection_context.cc
+++ b/pc/connection_context.cc
@@ -121,8 +121,12 @@
default_network_manager_ = std::make_unique<rtc::BasicNetworkManager>(
network_monitor_factory_.get());
- default_socket_factory_ =
- std::make_unique<rtc::BasicPacketSocketFactory>(network_thread());
+ // TODO(bugs.webrtc.org/13145): Either require that a PacketSocketFactory
+ // always is injected (with no need to construct this default factory), or get
+ // the appropriate underlying SocketFactory without going through the
+ // rtc::Thread::socketserver() accessor.
+ default_socket_factory_ = std::make_unique<rtc::BasicPacketSocketFactory>(
+ network_thread()->socketserver());
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&]() {
channel_manager_ = cricket::ChannelManager::Create(
diff --git a/rtc_tools/network_tester/test_controller.cc b/rtc_tools/network_tester/test_controller.cc
index 85a5a57..9c0d324 100644
--- a/rtc_tools/network_tester/test_controller.cc
+++ b/rtc_tools/network_tester/test_controller.cc
@@ -23,7 +23,9 @@
int max_port,
const std::string& config_file_path,
const std::string& log_file_path)
- : socket_factory_(rtc::ThreadManager::Instance()->WrapCurrentThread()),
+ // TODO(bugs.webrtc.org/13145): Add a SocketFactory argument.
+ : socket_factory_(
+ rtc::ThreadManager::Instance()->WrapCurrentThread()->socketserver()),
config_file_path_(config_file_path),
packet_logger_(log_file_path),
local_test_done_(false),
