Reland "Pass NetworkMonitorFactory through PeerConnectionFactory."

This is a reland of 003c9be817

Found some downstream code that relies on
NetworkMonitorFactory::SetFactory, so I'm adding those methods back
temporarily. BasicNetworkManager will fall back to the static factory
if the one passed into PeerConnectionFactory is null.

Original change's description:
> Pass NetworkMonitorFactory through PeerConnectionFactory.
>
> Previously the instance was set through a static method, which was
> really only done because it was difficult to add new
> PeerConnectionFactory construction arguments at the time.
>
> Now that we have PeerConnectionFactoryDependencies it's easy to clean
> this up.
>
> I'm doing this because I plan to add a NetworkMonitor implementation
> for iOS, and don't want to inherit this ugliness.
>
> Bug: webrtc:9883
> Change-Id: Id94dc061ab1c7186b81af8547393a6e336ff04c2
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/180241
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
> Commit-Queue: Taylor <deadbeef@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#31815}

TBR=hta@webrtc.org, sakal@webrtc.org

Bug: webrtc:9883
Change-Id: I2e817c423f21936f87532a9694eb9a0a1b70c212
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/180722
Reviewed-by: Taylor <deadbeef@webrtc.org>
Commit-Queue: Taylor <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31824}

api/DEPS

@@ -128,7 +128,7 @@ specific_include_rules = {
     "+media/base/media_config.h",
     "+media/base/media_engine.h",
     "+p2p/base/port_allocator.h",
-    "+rtc_base/network.h",
+    "+rtc_base/network_monitor_factory.h",
     "+rtc_base/rtc_certificate.h",
     "+rtc_base/rtc_certificate_generator.h",
     "+rtc_base/socket_address.h",

api/peer_connection_interface.h

@@ -113,7 +113,7 @@
 // inject a PacketSocketFactory and/or NetworkManager, and not expose
 // PortAllocator in the PeerConnection api.
 #include "p2p/base/port_allocator.h"  // nogncheck
-#include "rtc_base/network.h"
+#include "rtc_base/network_monitor_factory.h"
 #include "rtc_base/rtc_certificate.h"
 #include "rtc_base/rtc_certificate_generator.h"
 #include "rtc_base/socket_address.h"
@@ -1326,6 +1326,10 @@ struct RTC_EXPORT PeerConnectionFactoryDependencies final {
   std::unique_ptr<NetworkStatePredictorFactoryInterface>
       network_state_predictor_factory;
   std::unique_ptr<NetworkControllerFactoryInterface> network_controller_factory;
+  // This will only be used if CreatePeerConnection is called without a
+  // |port_allocator|, causing the default allocator and network manager to be
+  // used.
+  std::unique_ptr<rtc::NetworkMonitorFactory> network_monitor_factory;
   std::unique_ptr<NetEqFactory> neteq_factory;
   std::unique_ptr<WebRtcKeyValueConfig> trials;
 };

pc/peer_connection_factory.cc

@@ -74,6 +74,7 @@ PeerConnectionFactory::PeerConnectionFactory(
       worker_thread_(dependencies.worker_thread),
       signaling_thread_(dependencies.signaling_thread),
       task_queue_factory_(std::move(dependencies.task_queue_factory)),
+      network_monitor_factory_(std::move(dependencies.network_monitor_factory)),
       media_engine_(std::move(dependencies.media_engine)),
       call_factory_(std::move(dependencies.call_factory)),
       event_log_factory_(std::move(dependencies.event_log_factory)),
@@ -131,7 +132,10 @@ bool PeerConnectionFactory::Initialize() {
   RTC_DCHECK(signaling_thread_->IsCurrent());
   rtc::InitRandom(rtc::Time32());
 
-  default_network_manager_.reset(new rtc::BasicNetworkManager());
+  // If network_monitor_factory_ is non-null, it will be used to create a
+  // network monitor while on the network thread.
+  default_network_manager_.reset(
+      new rtc::BasicNetworkManager(network_monitor_factory_.get()));
   if (!default_network_manager_) {
     return false;
   }

pc/peer_connection_factory.h

@@ -113,6 +113,7 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface {
   const std::unique_ptr<TaskQueueFactory> task_queue_factory_;
   Options options_;
   std::unique_ptr<cricket::ChannelManager> channel_manager_;
+  const std::unique_ptr<rtc::NetworkMonitorFactory> network_monitor_factory_;
   std::unique_ptr<rtc::BasicNetworkManager> default_network_manager_;
   std::unique_ptr<rtc::BasicPacketSocketFactory> default_socket_factory_;
   std::unique_ptr<cricket::MediaEngineInterface> media_engine_;

rtc_base/BUILD.gn

@@ -705,6 +705,8 @@ rtc_source_set("threading") {
   #   "message_handler.h",
   #   "network_monitor.cc",
   #   "network_monitor.h",
+  #   "network_monitor_factory.cc",
+  #   "network_monitor_factory.h",
   #   "physical_socket_server.cc",
   #   "physical_socket_server.h",
   #   "signal_thread.cc",
@@ -853,6 +855,8 @@ rtc_library("rtc_base") {
     "network_constants.h",
     "network_monitor.cc",
     "network_monitor.h",
+    "network_monitor_factory.cc",
+    "network_monitor_factory.h",
     "network_route.cc",
     "network_route.h",
     "null_socket_server.cc",

rtc_base/network.cc

@@ -470,12 +470,16 @@ Network* NetworkManagerBase::GetNetworkFromAddress(
   return nullptr;
 }
 
-BasicNetworkManager::BasicNetworkManager()
-    : thread_(nullptr), sent_first_update_(false), start_count_(0) {}
+BasicNetworkManager::BasicNetworkManager() {}
+
+BasicNetworkManager::BasicNetworkManager(
+    NetworkMonitorFactory* network_monitor_factory)
+    : network_monitor_factory_(network_monitor_factory) {}
 
 BasicNetworkManager::~BasicNetworkManager() {}
 
 void BasicNetworkManager::OnNetworksChanged() {
+  RTC_DCHECK_RUN_ON(thread_);
   RTC_LOG(LS_INFO) << "Network change was observed";
   UpdateNetworksOnce();
 }
@@ -799,6 +803,8 @@ bool BasicNetworkManager::IsIgnoredNetwork(const Network& network) const {
 
 void BasicNetworkManager::StartUpdating() {
   thread_ = Thread::Current();
+  // Redundant but necessary for thread annotations.
+  RTC_DCHECK_RUN_ON(thread_);
   if (start_count_) {
     // If network interfaces are already discovered and signal is sent,
     // we should trigger network signal immediately for the new clients
@@ -813,7 +819,7 @@ void BasicNetworkManager::StartUpdating() {
 }
 
 void BasicNetworkManager::StopUpdating() {
-  RTC_DCHECK(Thread::Current() == thread_);
+  RTC_DCHECK_RUN_ON(thread_);
   if (!start_count_)
     return;
 
@@ -826,9 +832,12 @@ void BasicNetworkManager::StopUpdating() {
 }
 
 void BasicNetworkManager::StartNetworkMonitor() {
-  NetworkMonitorFactory* factory = NetworkMonitorFactory::GetFactory();
+  NetworkMonitorFactory* factory = network_monitor_factory_;
   if (factory == nullptr) {
-    return;
+    factory = NetworkMonitorFactory::GetFactory();
+    if (factory == nullptr) {
+      return;
+    }
   }
   if (!network_monitor_) {
     network_monitor_.reset(factory->CreateNetworkMonitor());
@@ -849,6 +858,7 @@ void BasicNetworkManager::StopNetworkMonitor() {
 }
 
 void BasicNetworkManager::OnMessage(Message* msg) {
+  RTC_DCHECK_RUN_ON(thread_);
   switch (msg->message_id) {
     case kUpdateNetworksMessage: {
       UpdateNetworksContinually();
@@ -864,7 +874,6 @@ void BasicNetworkManager::OnMessage(Message* msg) {
 }
 
 IPAddress BasicNetworkManager::QueryDefaultLocalAddress(int family) const {
-  RTC_DCHECK(thread_ == Thread::Current());
   RTC_DCHECK(thread_->socketserver() != nullptr);
   RTC_DCHECK(family == AF_INET || family == AF_INET6);
 
@@ -893,8 +902,6 @@ void BasicNetworkManager::UpdateNetworksOnce() {
   if (!start_count_)
     return;
 
-  RTC_DCHECK(Thread::Current() == thread_);
-
   NetworkList list;
   if (!CreateNetworks(false, &list)) {
     SignalError();
@@ -918,6 +925,7 @@ void BasicNetworkManager::UpdateNetworksContinually() {
 }
 
 void BasicNetworkManager::DumpNetworks() {
+  RTC_DCHECK_RUN_ON(thread_);
   NetworkList list;
   GetNetworks(&list);
   RTC_LOG(LS_INFO) << "NetworkManager detected " << list.size() << " networks:";

rtc_base/network.h

@@ -23,8 +23,11 @@
 #include "rtc_base/mdns_responder_interface.h"
 #include "rtc_base/message_handler.h"
 #include "rtc_base/network_monitor.h"
+#include "rtc_base/network_monitor_factory.h"
+#include "rtc_base/synchronization/sequence_checker.h"
 #include "rtc_base/system/rtc_export.h"
 #include "rtc_base/third_party/sigslot/sigslot.h"
+#include "rtc_base/thread_annotations.h"
 
 #if defined(WEBRTC_POSIX)
 struct ifaddrs;
@@ -221,6 +224,7 @@ class RTC_EXPORT BasicNetworkManager : public NetworkManagerBase,
                                        public sigslot::has_slots<> {
  public:
   BasicNetworkManager();
+  explicit BasicNetworkManager(NetworkMonitorFactory* network_monitor_factory);
   ~BasicNetworkManager() override;
 
   void StartUpdating() override;
@@ -234,7 +238,9 @@ class RTC_EXPORT BasicNetworkManager : public NetworkManagerBase,
 
   // Sets the network ignore list, which is empty by default. Any network on the
   // ignore list will be filtered from network enumeration results.
+  // Should be called only before initialization.
   void set_network_ignore_list(const std::vector<std::string>& list) {
+    RTC_DCHECK(thread_ == nullptr);
     network_ignore_list_ = list;
   }
 
@@ -244,41 +250,45 @@ class RTC_EXPORT BasicNetworkManager : public NetworkManagerBase,
   void ConvertIfAddrs(ifaddrs* interfaces,
                       IfAddrsConverter* converter,
                       bool include_ignored,
-                      NetworkList* networks) const;
+                      NetworkList* networks) const RTC_RUN_ON(thread_);
 #endif  // defined(WEBRTC_POSIX)
 
   // Creates a network object for each network available on the machine.
-  bool CreateNetworks(bool include_ignored, NetworkList* networks) const;
+  bool CreateNetworks(bool include_ignored, NetworkList* networks) const
+      RTC_RUN_ON(thread_);
 
   // Determines if a network should be ignored. This should only be determined
   // based on the network's property instead of any individual IP.
-  bool IsIgnoredNetwork(const Network& network) const;
+  bool IsIgnoredNetwork(const Network& network) const RTC_RUN_ON(thread_);
 
   // This function connects a UDP socket to a public address and returns the
   // local address associated it. Since it binds to the "any" address
   // internally, it returns the default local address on a multi-homed endpoint.
-  IPAddress QueryDefaultLocalAddress(int family) const;
+  IPAddress QueryDefaultLocalAddress(int family) const RTC_RUN_ON(thread_);
 
  private:
   friend class NetworkTest;
 
   // Creates a network monitor and listens for network updates.
-  void StartNetworkMonitor();
+  void StartNetworkMonitor() RTC_RUN_ON(thread_);
   // Stops and removes the network monitor.
-  void StopNetworkMonitor();
+  void StopNetworkMonitor() RTC_RUN_ON(thread_);
   // Called when it receives updates from the network monitor.
   void OnNetworksChanged();
 
   // Updates the networks and reschedules the next update.
-  void UpdateNetworksContinually();
+  void UpdateNetworksContinually() RTC_RUN_ON(thread_);
   // Only updates the networks; does not reschedule the next update.
-  void UpdateNetworksOnce();
+  void UpdateNetworksOnce() RTC_RUN_ON(thread_);
 
-  Thread* thread_;
-  bool sent_first_update_;
-  int start_count_;
+  Thread* thread_ = nullptr;
+  bool sent_first_update_ = true;
+  int start_count_ = 0;
   std::vector<std::string> network_ignore_list_;
-  std::unique_ptr<NetworkMonitorInterface> network_monitor_;
+  NetworkMonitorFactory* network_monitor_factory_ RTC_GUARDED_BY(thread_) =
+      nullptr;
+  std::unique_ptr<NetworkMonitorInterface> network_monitor_
+      RTC_GUARDED_BY(thread_);
 };
 
 // Represents a Unix-type network interface, with a name and single address.

rtc_base/network_monitor.cc

@@ -18,11 +18,6 @@
 
 namespace {
 const uint32_t UPDATE_NETWORKS_MESSAGE = 1;
-
-// This is set by NetworkMonitorFactory::SetFactory and the caller of
-// NetworkMonitorFactory::SetFactory must be responsible for calling
-// ReleaseFactory to destroy the factory.
-rtc::NetworkMonitorFactory* network_monitor_factory = nullptr;
 }  // namespace
 
 namespace rtc {
@@ -48,24 +43,4 @@ AdapterType NetworkMonitorBase::GetVpnUnderlyingAdapterType(
   return ADAPTER_TYPE_UNKNOWN;
 }
 
-NetworkMonitorFactory::NetworkMonitorFactory() {}
-NetworkMonitorFactory::~NetworkMonitorFactory() {}
-
-void NetworkMonitorFactory::SetFactory(NetworkMonitorFactory* factory) {
-  if (network_monitor_factory != nullptr) {
-    delete network_monitor_factory;
-  }
-  network_monitor_factory = factory;
-}
-
-void NetworkMonitorFactory::ReleaseFactory(NetworkMonitorFactory* factory) {
-  if (factory == network_monitor_factory) {
-    SetFactory(nullptr);
-  }
-}
-
-NetworkMonitorFactory* NetworkMonitorFactory::GetFactory() {
-  return network_monitor_factory;
-}
-
 }  // namespace rtc

rtc_base/network_monitor.h

@@ -12,6 +12,9 @@
 #define RTC_BASE_NETWORK_MONITOR_H_
 
 #include "rtc_base/network_constants.h"
+// TODO(deadbeef): Remove this include when downstream code stops using
+// NetworkMonitorFactory::SetFactory.
+#include "rtc_base/network_monitor_factory.h"
 #include "rtc_base/third_party/sigslot/sigslot.h"
 #include "rtc_base/thread.h"
 
@@ -98,26 +101,6 @@ class NetworkMonitorBase : public NetworkMonitorInterface,
   Thread* worker_thread_;
 };
 
-/*
- * NetworkMonitorFactory creates NetworkMonitors.
- */
-class NetworkMonitorFactory {
- public:
-  // This is not thread-safe; it should be called once (or once per audio/video
-  // call) during the call initialization.
-  static void SetFactory(NetworkMonitorFactory* factory);
-
-  static void ReleaseFactory(NetworkMonitorFactory* factory);
-  static NetworkMonitorFactory* GetFactory();
-
-  virtual NetworkMonitorInterface* CreateNetworkMonitor() = 0;
-
-  virtual ~NetworkMonitorFactory();
-
- protected:
-  NetworkMonitorFactory();
-};
-
 }  // namespace rtc
 
 #endif  // RTC_BASE_NETWORK_MONITOR_H_

rtc_base/network_monitor_factory.cc

@@ -0,0 +1,42 @@
+/*
+ *  Copyright 2020 The WebRTC Project Authors. All rights reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "rtc_base/network_monitor_factory.h"
+
+namespace {
+// This is set by NetworkMonitorFactory::SetFactory and the caller of
+// NetworkMonitorFactory::SetFactory must be responsible for calling
+// ReleaseFactory to destroy the factory.
+rtc::NetworkMonitorFactory* network_monitor_factory = nullptr;
+}  // namespace
+
+namespace rtc {
+
+NetworkMonitorFactory::NetworkMonitorFactory() {}
+NetworkMonitorFactory::~NetworkMonitorFactory() {}
+
+void NetworkMonitorFactory::SetFactory(NetworkMonitorFactory* factory) {
+  if (network_monitor_factory != nullptr) {
+    delete network_monitor_factory;
+  }
+  network_monitor_factory = factory;
+}
+
+void NetworkMonitorFactory::ReleaseFactory(NetworkMonitorFactory* factory) {
+  if (factory == network_monitor_factory) {
+    SetFactory(nullptr);
+  }
+}
+
+NetworkMonitorFactory* NetworkMonitorFactory::GetFactory() {
+  return network_monitor_factory;
+}
+
+}  // namespace rtc

rtc_base/network_monitor_factory.h

@@ -0,0 +1,46 @@
+/*
+ *  Copyright 2020 The WebRTC Project Authors. All rights reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef RTC_BASE_NETWORK_MONITOR_FACTORY_H_
+#define RTC_BASE_NETWORK_MONITOR_FACTORY_H_
+
+namespace rtc {
+
+// Forward declaring this so it's not part of the API surface; it's only
+// expected to be used by Android/iOS SDK code.
+class NetworkMonitorInterface;
+
+/*
+ * NetworkMonitorFactory creates NetworkMonitors.
+ * Note that CreateNetworkMonitor is expected to be called on the network
+ * thread with the returned object only being used on that thread thereafter.
+ */
+class NetworkMonitorFactory {
+ public:
+  // This is not thread-safe; it should be called once (or once per audio/video
+  // call) during the call initialization.
+  // DEPRECATED: Should pass NetworkMonitorFactory through
+  // PeerConnectionFactoryDependencies instead.
+  static void SetFactory(NetworkMonitorFactory* factory);
+
+  static void ReleaseFactory(NetworkMonitorFactory* factory);
+  static NetworkMonitorFactory* GetFactory();
+
+  virtual NetworkMonitorInterface* CreateNetworkMonitor() = 0;
+
+  virtual ~NetworkMonitorFactory();
+
+ protected:
+  NetworkMonitorFactory();
+};
+
+}  // namespace rtc
+
+#endif  // RTC_BASE_NETWORK_MONITOR_FACTORY_H_

rtc_base/network_unittest.cc

@@ -19,6 +19,7 @@
 #include "rtc_base/checks.h"
 #include "rtc_base/net_helpers.h"
 #include "rtc_base/network_monitor.h"
+#include "rtc_base/network_monitor_factory.h"
 #if defined(WEBRTC_POSIX)
 #include <net/if.h>
 #include <sys/types.h>
@@ -100,18 +101,27 @@ class NetworkTest : public ::testing::Test, public sigslot::has_slots<> {
 
   bool IsIgnoredNetwork(BasicNetworkManager& network_manager,
                         const Network& network) {
+    RTC_DCHECK_RUN_ON(network_manager.thread_);
     return network_manager.IsIgnoredNetwork(network);
   }
 
+  IPAddress QueryDefaultLocalAddress(BasicNetworkManager& network_manager,
+                                     int family) {
+    RTC_DCHECK_RUN_ON(network_manager.thread_);
+    return network_manager.QueryDefaultLocalAddress(family);
+  }
+
   NetworkManager::NetworkList GetNetworks(
       const BasicNetworkManager& network_manager,
       bool include_ignored) {
+    RTC_DCHECK_RUN_ON(network_manager.thread_);
     NetworkManager::NetworkList list;
     network_manager.CreateNetworks(include_ignored, &list);
     return list;
   }
 
   FakeNetworkMonitor* GetNetworkMonitor(BasicNetworkManager& network_manager) {
+    RTC_DCHECK_RUN_ON(network_manager.thread_);
     return static_cast<FakeNetworkMonitor*>(
         network_manager.network_monitor_.get());
   }
@@ -136,6 +146,7 @@ class NetworkTest : public ::testing::Test, public sigslot::has_slots<> {
                                  struct ifaddrs* interfaces,
                                  bool include_ignored,
                                  NetworkManager::NetworkList* networks) {
+    RTC_DCHECK_RUN_ON(network_manager.thread_);
     // Use the base IfAddrsConverter for test cases.
     std::unique_ptr<IfAddrsConverter> ifaddrs_converter(new IfAddrsConverter());
     network_manager.ConvertIfAddrs(interfaces, ifaddrs_converter.get(),
@@ -247,6 +258,8 @@ class NetworkTest : public ::testing::Test, public sigslot::has_slots<> {
 
 class TestBasicNetworkManager : public BasicNetworkManager {
  public:
+  TestBasicNetworkManager(NetworkMonitorFactory* network_monitor_factory)
+      : BasicNetworkManager(network_monitor_factory) {}
   using BasicNetworkManager::QueryDefaultLocalAddress;
   using BasicNetworkManager::set_default_local_addresses;
 };
@@ -268,6 +281,7 @@ TEST_F(NetworkTest, TestIsIgnoredNetworkIgnoresIPsStartingWith0) {
   Network ipv4_network2("test_eth1", "Test Network Adapter 2",
                         IPAddress(0x010000U), 24, ADAPTER_TYPE_ETHERNET);
   BasicNetworkManager network_manager;
+  network_manager.StartUpdating();
   EXPECT_FALSE(IsIgnoredNetwork(network_manager, ipv4_network1));
   EXPECT_TRUE(IsIgnoredNetwork(network_manager, ipv4_network2));
 }
@@ -278,14 +292,18 @@ TEST_F(NetworkTest, TestIgnoreList) {
                     24);
   Network include_me("include_me", "Include me please!", IPAddress(0x12345600U),
                      24);
-  BasicNetworkManager network_manager;
-  EXPECT_FALSE(IsIgnoredNetwork(network_manager, ignore_me));
-  EXPECT_FALSE(IsIgnoredNetwork(network_manager, include_me));
+  BasicNetworkManager default_network_manager;
+  default_network_manager.StartUpdating();
+  EXPECT_FALSE(IsIgnoredNetwork(default_network_manager, ignore_me));
+  EXPECT_FALSE(IsIgnoredNetwork(default_network_manager, include_me));
+
+  BasicNetworkManager ignoring_network_manager;
   std::vector<std::string> ignore_list;
   ignore_list.push_back("ignore_me");
-  network_manager.set_network_ignore_list(ignore_list);
-  EXPECT_TRUE(IsIgnoredNetwork(network_manager, ignore_me));
-  EXPECT_FALSE(IsIgnoredNetwork(network_manager, include_me));
+  ignoring_network_manager.set_network_ignore_list(ignore_list);
+  ignoring_network_manager.StartUpdating();
+  EXPECT_TRUE(IsIgnoredNetwork(ignoring_network_manager, ignore_me));
+  EXPECT_FALSE(IsIgnoredNetwork(ignoring_network_manager, include_me));
 }
 
 // Test is failing on Windows opt: b/11288214
@@ -649,6 +667,7 @@ TEST_F(NetworkTest, TestMultiplePublicNetworksOnOneInterfaceMerge) {
 // Test that DumpNetworks does not crash.
 TEST_F(NetworkTest, TestCreateAndDumpNetworks) {
   BasicNetworkManager manager;
+  manager.StartUpdating();
   NetworkManager::NetworkList list = GetNetworks(manager, true);
   bool changed;
   MergeNetworkList(manager, list, &changed);
@@ -657,6 +676,7 @@ TEST_F(NetworkTest, TestCreateAndDumpNetworks) {
 
 TEST_F(NetworkTest, TestIPv6Toggle) {
   BasicNetworkManager manager;
+  manager.StartUpdating();
   bool ipv6_found = false;
   NetworkManager::NetworkList list;
   list = GetNetworks(manager, true);
@@ -753,6 +773,7 @@ TEST_F(NetworkTest, TestConvertIfAddrsNoAddress) {
 
   NetworkManager::NetworkList result;
   BasicNetworkManager manager;
+  manager.StartUpdating();
   CallConvertIfAddrs(manager, &list, true, &result);
   EXPECT_TRUE(result.empty());
 }
@@ -768,6 +789,7 @@ TEST_F(NetworkTest, TestConvertIfAddrsMultiAddressesOnOneInterface) {
                         "FFFF:FFFF:FFFF:FFFF::", 0);
   NetworkManager::NetworkList result;
   BasicNetworkManager manager;
+  manager.StartUpdating();
   CallConvertIfAddrs(manager, list, true, &result);
   EXPECT_EQ(1U, result.size());
   bool changed;
@@ -787,46 +809,35 @@ TEST_F(NetworkTest, TestConvertIfAddrsNotRunning) {
 
   NetworkManager::NetworkList result;
   BasicNetworkManager manager;
+  manager.StartUpdating();
   CallConvertIfAddrs(manager, &list, true, &result);
   EXPECT_TRUE(result.empty());
 }
 
-// Tests that the network type can be updated after the network monitor is
-// started.
+// Tests that the network type can be determined from the network monitor when
+// it would otherwise be unknown.
 TEST_F(NetworkTest, TestGetAdapterTypeFromNetworkMonitor) {
-  char if_name1[20] = "wifi0";
-  std::string ipv6_address1 = "1000:2000:3000:4000:0:0:0:1";
-  std::string ipv6_address2 = "1000:2000:3000:8000:0:0:0:1";
+  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;
-  // A network created before the network monitor is started will get
-  // UNKNOWN type.
-  ifaddrs* addr_list =
-      InstallIpv6Network(if_name1, ipv6_address1, ipv6_mask, manager);
-  EXPECT_EQ(ADAPTER_TYPE_UNKNOWN, GetAdapterType(manager));
+  BasicNetworkManager manager_without_monitor;
+  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,
+                                          manager_without_monitor);
+  EXPECT_EQ(ADAPTER_TYPE_UNKNOWN, GetAdapterType(manager_without_monitor));
   ReleaseIfAddrs(addr_list);
-  // Note: Do not call ClearNetworks here in order to test that the type
-  // of an existing network can be changed after the network monitor starts
-  // and detects the network type correctly.
 
-  // After the network monitor starts, the type will be updated.
-  FakeNetworkMonitorFactory* factory = new FakeNetworkMonitorFactory();
-  NetworkMonitorFactory::SetFactory(factory);
-  // This brings up the hook with the network monitor.
-  manager.StartUpdating();
+  // With the fake network monitor the type should be correctly determined.
+  FakeNetworkMonitorFactory factory;
+  BasicNetworkManager manager_with_monitor(&factory);
+  manager_with_monitor.StartUpdating();
   // Add the same ipv6 address as before but it has the right network type
   // detected by the network monitor now.
-  addr_list = InstallIpv6Network(if_name1, ipv6_address1, ipv6_mask, manager);
-  EXPECT_EQ(ADAPTER_TYPE_WIFI, GetAdapterType(manager));
+  addr_list = InstallIpv6Network(if_name, ipv6_address, ipv6_mask,
+                                 manager_with_monitor);
+  EXPECT_EQ(ADAPTER_TYPE_WIFI, GetAdapterType(manager_with_monitor));
   ReleaseIfAddrs(addr_list);
-  ClearNetworks(manager);
-
-  // Add another network with the type inferred from the network monitor.
-  char if_name2[20] = "cellular0";
-  addr_list = InstallIpv6Network(if_name2, ipv6_address2, ipv6_mask, manager);
-  EXPECT_EQ(ADAPTER_TYPE_CELLULAR, GetAdapterType(manager));
-  ReleaseIfAddrs(addr_list);
-  ClearNetworks(manager);
 }
 
 // Test that the network type can be determined based on name matching in
@@ -839,6 +850,7 @@ TEST_F(NetworkTest, TestGetAdapterTypeFromNameMatching) {
   std::string ipv6_address2 = "1000:2000:3000:8000:0:0:0:1";
   std::string ipv6_mask = "FFFF:FFFF:FFFF:FFFF::";
   BasicNetworkManager manager;
+  manager.StartUpdating();
 
   // IPSec interface; name is in form "ipsec<index>".
   char if_name[20] = "ipsec11";
@@ -1022,11 +1034,10 @@ TEST_F(NetworkTest, TestIPv6Selection) {
 }
 
 TEST_F(NetworkTest, TestNetworkMonitoring) {
-  BasicNetworkManager manager;
+  FakeNetworkMonitorFactory factory;
+  BasicNetworkManager manager(&factory);
   manager.SignalNetworksChanged.connect(static_cast<NetworkTest*>(this),
                                         &NetworkTest::OnNetworksChanged);
-  FakeNetworkMonitorFactory* factory = new FakeNetworkMonitorFactory();
-  NetworkMonitorFactory::SetFactory(factory);
   manager.StartUpdating();
   FakeNetworkMonitor* network_monitor = GetNetworkMonitor(manager);
   EXPECT_TRUE(network_monitor && network_monitor->started());
@@ -1043,8 +1054,6 @@ TEST_F(NetworkTest, TestNetworkMonitoring) {
   // Network manager is stopped.
   manager.StopUpdating();
   EXPECT_FALSE(GetNetworkMonitor(manager)->started());
-
-  NetworkMonitorFactory::ReleaseFactory(factory);
 }
 
 // Fails on Android: https://bugs.chromium.org/p/webrtc/issues/detail?id=4364.
@@ -1055,11 +1064,10 @@ TEST_F(NetworkTest, TestNetworkMonitoring) {
 #endif
 TEST_F(NetworkTest, MAYBE_DefaultLocalAddress) {
   IPAddress ip;
-  TestBasicNetworkManager manager;
+  FakeNetworkMonitorFactory factory;
+  TestBasicNetworkManager manager(&factory);
   manager.SignalNetworksChanged.connect(static_cast<NetworkTest*>(this),
                                         &NetworkTest::OnNetworksChanged);
-  FakeNetworkMonitorFactory* factory = new FakeNetworkMonitorFactory();
-  NetworkMonitorFactory::SetFactory(factory);
   manager.StartUpdating();
   EXPECT_TRUE_WAIT(callback_called_, 1000);
 
@@ -1070,12 +1078,12 @@ TEST_F(NetworkTest, MAYBE_DefaultLocalAddress) {
   EXPECT_TRUE(!networks.empty());
   for (const auto* network : networks) {
     if (network->GetBestIP().family() == AF_INET) {
-      EXPECT_TRUE(manager.QueryDefaultLocalAddress(AF_INET) != IPAddress());
+      EXPECT_TRUE(QueryDefaultLocalAddress(manager, AF_INET) != IPAddress());
     } else if (network->GetBestIP().family() == AF_INET6 &&
                !IPIsLoopback(network->GetBestIP())) {
       // Existence of an IPv6 loopback address doesn't mean it has IPv6 network
       // enabled.
-      EXPECT_TRUE(manager.QueryDefaultLocalAddress(AF_INET6) != IPAddress());
+      EXPECT_TRUE(QueryDefaultLocalAddress(manager, AF_INET6) != IPAddress());
     }
   }
 

sdk/android/native_api/base/network_monitor.h

@@ -15,7 +15,7 @@
 
 #include <memory>
 
-#include "rtc_base/network_monitor.h"
+#include "rtc_base/network_monitor_factory.h"
 
 namespace webrtc {
 

sdk/android/native_api/peerconnection/peer_connection_factory.cc

@@ -23,11 +23,10 @@ jobject NativeToJavaPeerConnectionFactory(
     rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface> pcf,
     std::unique_ptr<rtc::Thread> network_thread,
     std::unique_ptr<rtc::Thread> worker_thread,
-    std::unique_ptr<rtc::Thread> signaling_thread,
-    rtc::NetworkMonitorFactory* network_monitor_factory) {
+    std::unique_ptr<rtc::Thread> signaling_thread) {
   return webrtc::jni::NativeToJavaPeerConnectionFactory(
       jni, pcf, std::move(network_thread), std::move(worker_thread),
-      std::move(signaling_thread), network_monitor_factory);
+      std::move(signaling_thread));
 }
 
 }  // namespace webrtc

sdk/android/native_api/peerconnection/peer_connection_factory.h

@@ -26,8 +26,7 @@ jobject NativeToJavaPeerConnectionFactory(
     rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface> pcf,
     std::unique_ptr<rtc::Thread> network_thread,
     std::unique_ptr<rtc::Thread> worker_thread,
-    std::unique_ptr<rtc::Thread> signaling_thread,
-    rtc::NetworkMonitorFactory* network_monitor_factory = nullptr);
+    std::unique_ptr<rtc::Thread> signaling_thread);
 
 }  // namespace webrtc
 

sdk/android/native_unittests/peerconnection/peer_connection_factory_unittest.cc

@@ -100,7 +100,7 @@ TEST(PeerConnectionFactoryTest, NativeToJavaPeerConnectionFactory) {
 
   jobject java_factory = NativeToJavaPeerConnectionFactory(
       jni, factory, std::move(network_thread), std::move(worker_thread),
-      std::move(signaling_thread), nullptr /* network_monitor_factory */);
+      std::move(signaling_thread));
 
   RTC_LOG(INFO) << java_factory;
 

sdk/android/src/jni/android_network_monitor.h

@@ -18,6 +18,7 @@
 
 #include "absl/types/optional.h"
 #include "rtc_base/network_monitor.h"
+#include "rtc_base/network_monitor_factory.h"
 #include "rtc_base/thread_checker.h"
 #include "sdk/android/src/jni/jni_helpers.h"
 

sdk/android/src/jni/pc/owned_factory_and_threads.cc

@@ -19,19 +19,11 @@ OwnedFactoryAndThreads::OwnedFactoryAndThreads(
     std::unique_ptr<rtc::Thread> network_thread,
     std::unique_ptr<rtc::Thread> worker_thread,
     std::unique_ptr<rtc::Thread> signaling_thread,
-    rtc::NetworkMonitorFactory* network_monitor_factory,
     const rtc::scoped_refptr<PeerConnectionFactoryInterface>& factory)
     : network_thread_(std::move(network_thread)),
       worker_thread_(std::move(worker_thread)),
       signaling_thread_(std::move(signaling_thread)),
-      network_monitor_factory_(network_monitor_factory),
       factory_(factory) {}
 
-OwnedFactoryAndThreads::~OwnedFactoryAndThreads() {
-  if (network_monitor_factory_ != nullptr) {
-    rtc::NetworkMonitorFactory::ReleaseFactory(network_monitor_factory_);
-  }
-}
-
 }  // namespace jni
 }  // namespace webrtc

sdk/android/src/jni/pc/owned_factory_and_threads.h

@@ -33,25 +33,19 @@ class OwnedFactoryAndThreads {
       std::unique_ptr<rtc::Thread> network_thread,
       std::unique_ptr<rtc::Thread> worker_thread,
       std::unique_ptr<rtc::Thread> signaling_thread,
-      rtc::NetworkMonitorFactory* network_monitor_factory,
       const rtc::scoped_refptr<PeerConnectionFactoryInterface>& factory);
 
-  ~OwnedFactoryAndThreads();
+  ~OwnedFactoryAndThreads() = default;
 
   PeerConnectionFactoryInterface* factory() { return factory_.get(); }
   rtc::Thread* network_thread() { return network_thread_.get(); }
   rtc::Thread* signaling_thread() { return signaling_thread_.get(); }
   rtc::Thread* worker_thread() { return worker_thread_.get(); }
-  rtc::NetworkMonitorFactory* network_monitor_factory() {
-    return network_monitor_factory_;
-  }
-  void clear_network_monitor_factory() { network_monitor_factory_ = nullptr; }
 
  private:
   const std::unique_ptr<rtc::Thread> network_thread_;
   const std::unique_ptr<rtc::Thread> worker_thread_;
   const std::unique_ptr<rtc::Thread> signaling_thread_;
-  rtc::NetworkMonitorFactory* network_monitor_factory_;
   const rtc::scoped_refptr<PeerConnectionFactoryInterface> factory_;
 };
 

sdk/android/src/jni/pc/peer_connection_factory.cc

@@ -138,11 +138,10 @@ ScopedJavaLocalRef<jobject> NativeToScopedJavaPeerConnectionFactory(
     rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface> pcf,
     std::unique_ptr<rtc::Thread> network_thread,
     std::unique_ptr<rtc::Thread> worker_thread,
-    std::unique_ptr<rtc::Thread> signaling_thread,
-    rtc::NetworkMonitorFactory* network_monitor_factory) {
+    std::unique_ptr<rtc::Thread> signaling_thread) {
   OwnedFactoryAndThreads* owned_factory = new OwnedFactoryAndThreads(
       std::move(network_thread), std::move(worker_thread),
-      std::move(signaling_thread), network_monitor_factory, pcf);
+      std::move(signaling_thread), pcf);
 
   ScopedJavaLocalRef<jobject> j_pcf = Java_PeerConnectionFactory_Constructor(
       env, NativeToJavaPointer(owned_factory));
@@ -172,17 +171,15 @@ PeerConnectionFactoryInterface* PeerConnectionFactoryFromJava(jlong j_p) {
 // Set in PeerConnectionFactory_initializeAndroidGlobals().
 static bool factory_static_initialized = false;
 
-
 jobject NativeToJavaPeerConnectionFactory(
     JNIEnv* jni,
     rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface> pcf,
     std::unique_ptr<rtc::Thread> network_thread,
     std::unique_ptr<rtc::Thread> worker_thread,
-    std::unique_ptr<rtc::Thread> signaling_thread,
-    rtc::NetworkMonitorFactory* network_monitor_factory) {
+    std::unique_ptr<rtc::Thread> signaling_thread) {
   return NativeToScopedJavaPeerConnectionFactory(
              jni, pcf, std::move(network_thread), std::move(worker_thread),
-             std::move(signaling_thread), network_monitor_factory)
+             std::move(signaling_thread))
       .Release();
 }
 
@@ -284,18 +281,9 @@ ScopedJavaLocalRef<jobject> CreatePeerConnectionFactoryForJava(
   signaling_thread->SetName("signaling_thread", NULL);
   RTC_CHECK(signaling_thread->Start()) << "Failed to start thread";
 
-  rtc::NetworkMonitorFactory* network_monitor_factory = nullptr;
-
   const absl::optional<PeerConnectionFactoryInterface::Options> options =
       JavaToNativePeerConnectionFactoryOptions(jni, joptions);
 
-  // Do not create network_monitor_factory only if the options are
-  // provided and disable_network_monitor therein is set to true.
-  if (!(options && options->disable_network_monitor)) {
-    network_monitor_factory = new AndroidNetworkMonitorFactory();
-    rtc::NetworkMonitorFactory::SetFactory(network_monitor_factory);
-  }
-
   PeerConnectionFactoryDependencies dependencies;
   dependencies.network_thread = network_thread.get();
   dependencies.worker_thread = worker_thread.get();
@@ -310,6 +298,10 @@ ScopedJavaLocalRef<jobject> CreatePeerConnectionFactoryForJava(
   dependencies.network_state_predictor_factory =
       std::move(network_state_predictor_factory);
   dependencies.neteq_factory = std::move(neteq_factory);
+  if (!(options && options->disable_network_monitor)) {
+    dependencies.network_monitor_factory =
+        std::make_unique<AndroidNetworkMonitorFactory>();
+  }
 
   cricket::MediaEngineDependencies media_dependencies;
   media_dependencies.task_queue_factory = dependencies.task_queue_factory.get();
@@ -336,7 +328,7 @@ ScopedJavaLocalRef<jobject> CreatePeerConnectionFactoryForJava(
 
   return NativeToScopedJavaPeerConnectionFactory(
       jni, factory, std::move(network_thread), std::move(worker_thread),
-      std::move(signaling_thread), network_monitor_factory);
+      std::move(signaling_thread));
 }
 
 static ScopedJavaLocalRef<jobject>

sdk/android/src/jni/pc/peer_connection_factory.h

@@ -24,8 +24,7 @@ jobject NativeToJavaPeerConnectionFactory(
     rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface> pcf,
     std::unique_ptr<rtc::Thread> network_thread,
     std::unique_ptr<rtc::Thread> worker_thread,
-    std::unique_ptr<rtc::Thread> signaling_thread,
-    rtc::NetworkMonitorFactory* network_monitor_factory = nullptr);
+    std::unique_ptr<rtc::Thread> signaling_thread);
 
 }  // namespace jni
 }  // namespace webrtc