Pass a SocketFactory to BasicNetworkManager constructor

Used by QueryDefaultLocalAddress, instead of relying on the update
thread's associated socket server.

This is not the only use of rtc::Thread::socketserver() in the
BasicNetworkManager class. It also interacts with the thread's
socket server to call set_network_binder. That is unchanged by this cl,
perhaps those calls can be moved to the caller of StartNetworkMonitor and
StopNetworkMonitor.

Bug: webrtc:13145
Change-Id: If109c2dcb0e74b183e10bb3db7a5aefcc95d1a8c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232613
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35118}

pc/connection_context.cc

@@ -119,7 +119,7 @@ ConnectionContext::ConnectionContext(
   // If network_monitor_factory_ is non-null, it will be used to create a
   // network monitor while on the network thread.
   default_network_manager_ = std::make_unique<rtc::BasicNetworkManager>(
-      network_monitor_factory_.get());
+      network_monitor_factory_.get(), network_thread()->socketserver());
 
   // TODO(bugs.webrtc.org/13145): Either require that a PacketSocketFactory
   // always is injected (with no need to construct this default factory), or get

rtc_base/nat_unittest.cc

@@ -219,7 +219,8 @@ bool TestConnectivity(const SocketAddress& src, const IPAddress& dst) {
 }
 
 void TestPhysicalInternal(const SocketAddress& int_addr) {
-  BasicNetworkManager network_manager;
+  PhysicalSocketServer socket_server;
+  BasicNetworkManager network_manager(nullptr, &socket_server);
   network_manager.StartUpdating();
   // Process pending messages so the network list is updated.
   Thread::Current()->ProcessMessages(0);

rtc_base/network.cc

@@ -508,11 +508,18 @@ bool NetworkManagerBase::IsVpnMacAddress(
   return false;
 }
 
-BasicNetworkManager::BasicNetworkManager() : BasicNetworkManager(nullptr) {}
+BasicNetworkManager::BasicNetworkManager()
+    : BasicNetworkManager(nullptr, nullptr) {}
 
 BasicNetworkManager::BasicNetworkManager(
     NetworkMonitorFactory* network_monitor_factory)
+    : BasicNetworkManager(network_monitor_factory, nullptr) {}
+
+BasicNetworkManager::BasicNetworkManager(
+    NetworkMonitorFactory* network_monitor_factory,
+    SocketFactory* socket_factory)
     : network_monitor_factory_(network_monitor_factory),
+      socket_factory_(socket_factory),
       allow_mac_based_ipv6_(
           webrtc::field_trial::IsEnabled("WebRTC-AllowMACBasedIPv6")),
       bind_using_ifname_(
@@ -963,11 +970,18 @@ void BasicNetworkManager::OnMessage(Message* msg) {
 }
 
 IPAddress BasicNetworkManager::QueryDefaultLocalAddress(int family) const {
-  RTC_DCHECK(thread_->socketserver() != nullptr);
   RTC_DCHECK(family == AF_INET || family == AF_INET6);
 
+  // TODO(bugs.webrtc.org/13145): Delete support for null `socket_factory_`,
+  // require socket factory to be provided to constructor.
+  SocketFactory* socket_factory = socket_factory_;
+  if (!socket_factory) {
+    socket_factory = thread_->socketserver();
+  }
+  RTC_DCHECK(socket_factory);
+
   std::unique_ptr<Socket> socket(
-      thread_->socketserver()->CreateSocket(family, SOCK_DGRAM));
+      socket_factory->CreateSocket(family, SOCK_DGRAM));
   if (!socket) {
     RTC_LOG_ERR(LERROR) << "Socket creation failed";
     return IPAddress();

rtc_base/network.h

@@ -26,6 +26,7 @@
 #include "rtc_base/message_handler.h"
 #include "rtc_base/network_monitor.h"
 #include "rtc_base/network_monitor_factory.h"
+#include "rtc_base/socket_factory.h"
 #include "rtc_base/system/rtc_export.h"
 #include "rtc_base/third_party/sigslot/sigslot.h"
 #include "rtc_base/thread_annotations.h"
@@ -255,7 +256,12 @@ class RTC_EXPORT BasicNetworkManager : public NetworkManagerBase,
                                        public sigslot::has_slots<> {
  public:
   BasicNetworkManager();
+
+  ABSL_DEPRECATED(
+      "Use the version with socket_factory, see bugs.webrtc.org/13145")
   explicit BasicNetworkManager(NetworkMonitorFactory* network_monitor_factory);
+  BasicNetworkManager(NetworkMonitorFactory* network_monitor_factory,
+                      SocketFactory* socket_factory);
   ~BasicNetworkManager() override;
 
   void StartUpdating() override;
@@ -331,8 +337,8 @@ class RTC_EXPORT BasicNetworkManager : public NetworkManagerBase,
   bool sent_first_update_ = true;
   int start_count_ = 0;
   std::vector<std::string> network_ignore_list_;
-  NetworkMonitorFactory* network_monitor_factory_ RTC_GUARDED_BY(thread_) =
+  NetworkMonitorFactory* const network_monitor_factory_;
-      nullptr;
+  SocketFactory* const socket_factory_;
   std::unique_ptr<NetworkMonitorInterface> network_monitor_
       RTC_GUARDED_BY(thread_);
   bool allow_mac_based_ipv6_ RTC_GUARDED_BY(thread_) = false;

rtc_base/network_unittest.cc

@@ -22,6 +22,7 @@
 #include "rtc_base/net_helpers.h"
 #include "rtc_base/network_monitor.h"
 #include "rtc_base/network_monitor_factory.h"
+#include "rtc_base/physical_socket_server.h"
 #if defined(WEBRTC_POSIX)
 #include <net/if.h>
 #include <sys/types.h>
@@ -313,8 +314,9 @@ class NetworkTest : public ::testing::Test, public sigslot::has_slots<> {
 
 class TestBasicNetworkManager : public BasicNetworkManager {
  public:
-  TestBasicNetworkManager(NetworkMonitorFactory* network_monitor_factory)
+  TestBasicNetworkManager(NetworkMonitorFactory* network_monitor_factory,
-      : BasicNetworkManager(network_monitor_factory) {}
+                          SocketFactory* socket_factory)
+      : BasicNetworkManager(network_monitor_factory, socket_factory) {}
   using BasicNetworkManager::QueryDefaultLocalAddress;
   using BasicNetworkManager::set_default_local_addresses;
 };
@@ -398,7 +400,8 @@ TEST_F(NetworkTest, DISABLED_TestCreateNetworks) {
 // Test StartUpdating() and StopUpdating(). network_permission_state starts with
 // ALLOWED.
 TEST_F(NetworkTest, TestUpdateNetworks) {
-  BasicNetworkManager manager;
+  PhysicalSocketServer socket_server;
+  BasicNetworkManager manager(nullptr, &socket_server);
   manager.SignalNetworksChanged.connect(static_cast<NetworkTest*>(this),
                                         &NetworkTest::OnNetworksChanged);
   EXPECT_EQ(NetworkManager::ENUMERATION_ALLOWED,
@@ -875,7 +878,8 @@ TEST_F(NetworkTest, TestGetAdapterTypeFromNetworkMonitor) {
   char if_name[20] = "wifi0";
   std::string ipv6_address = "1000:2000:3000:4000:0:0:0:1";
   std::string ipv6_mask = "FFFF:FFFF:FFFF:FFFF::";
-  BasicNetworkManager manager_without_monitor;
+  PhysicalSocketServer socket_server;
+  BasicNetworkManager manager_without_monitor(nullptr, &socket_server);
   manager_without_monitor.StartUpdating();
   // A network created without a network monitor will get UNKNOWN type.
   ifaddrs* addr_list = InstallIpv6Network(if_name, ipv6_address, ipv6_mask,
@@ -885,7 +889,7 @@ TEST_F(NetworkTest, TestGetAdapterTypeFromNetworkMonitor) {
 
   // With the fake network monitor the type should be correctly determined.
   FakeNetworkMonitorFactory factory;
-  BasicNetworkManager manager_with_monitor(&factory);
+  BasicNetworkManager manager_with_monitor(&factory, &socket_server);
   manager_with_monitor.StartUpdating();
   // Add the same ipv6 address as before but it has the right network type
   // detected by the network monitor now.
@@ -981,7 +985,8 @@ TEST_F(NetworkTest, TestNetworkMonitorIsAdapterAvailable) {
 
   // Sanity check that both interfaces are included by default.
   FakeNetworkMonitorFactory factory;
-  BasicNetworkManager manager(&factory);
+  PhysicalSocketServer socket_server;
+  BasicNetworkManager manager(&factory, &socket_server);
   manager.StartUpdating();
   CallConvertIfAddrs(manager, list, /*include_ignored=*/false, &result);
   EXPECT_EQ(2u, result.size());
@@ -1125,7 +1130,8 @@ TEST_F(NetworkTest, TestIPv6Selection) {
 
 TEST_F(NetworkTest, TestNetworkMonitoring) {
   FakeNetworkMonitorFactory factory;
-  BasicNetworkManager manager(&factory);
+  PhysicalSocketServer socket_server;
+  BasicNetworkManager manager(&factory, &socket_server);
   manager.SignalNetworksChanged.connect(static_cast<NetworkTest*>(this),
                                         &NetworkTest::OnNetworksChanged);
   manager.StartUpdating();
@@ -1155,7 +1161,8 @@ TEST_F(NetworkTest, TestNetworkMonitoring) {
 TEST_F(NetworkTest, MAYBE_DefaultLocalAddress) {
   IPAddress ip;
   FakeNetworkMonitorFactory factory;
-  TestBasicNetworkManager manager(&factory);
+  PhysicalSocketServer socket_server;
+  TestBasicNetworkManager manager(&factory, &socket_server);
   manager.SignalNetworksChanged.connect(static_cast<NetworkTest*>(this),
                                         &NetworkTest::OnNetworksChanged);
   manager.StartUpdating();
@@ -1327,7 +1334,8 @@ TEST_F(NetworkTest, WebRTC_BindUsingInterfaceName) {
 
   // Sanity check that both interfaces are included by default.
   FakeNetworkMonitorFactory factory;
-  BasicNetworkManager manager(&factory);
+  PhysicalSocketServer socket_server;
+  BasicNetworkManager manager(&factory, &socket_server);
   manager.StartUpdating();
   CallConvertIfAddrs(manager, list, /*include_ignored=*/false, &result);
   EXPECT_EQ(2u, result.size());