diff --git a/api/DEPS b/api/DEPS index 4fc132bdc0..4b93438c3e 100644 --- a/api/DEPS +++ b/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", diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 52d0d8789a..b1beb844e1 100644 --- a/api/peer_connection_interface.h +++ b/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 network_state_predictor_factory; std::unique_ptr 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 network_monitor_factory; std::unique_ptr neteq_factory; std::unique_ptr trials; }; diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc index d3b7fcda8d..d79e438152 100644 --- a/pc/peer_connection_factory.cc +++ b/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; } diff --git a/pc/peer_connection_factory.h b/pc/peer_connection_factory.h index 58859a0296..3932562d22 100644 --- a/pc/peer_connection_factory.h +++ b/pc/peer_connection_factory.h @@ -113,6 +113,7 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { const std::unique_ptr task_queue_factory_; Options options_; std::unique_ptr channel_manager_; + const std::unique_ptr network_monitor_factory_; std::unique_ptr default_network_manager_; std::unique_ptr default_socket_factory_; std::unique_ptr media_engine_; diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 73bca85efa..64548e6f80 100644 --- a/rtc_base/BUILD.gn +++ b/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", diff --git a/rtc_base/network.cc b/rtc_base/network.cc index 64aee4bdae..3b05c68172 100644 --- a/rtc_base/network.cc +++ b/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,12 +832,11 @@ void BasicNetworkManager::StopUpdating() { } void BasicNetworkManager::StartNetworkMonitor() { - NetworkMonitorFactory* factory = NetworkMonitorFactory::GetFactory(); - if (factory == nullptr) { + if (network_monitor_factory_ == nullptr) { return; } if (!network_monitor_) { - network_monitor_.reset(factory->CreateNetworkMonitor()); + network_monitor_.reset(network_monitor_factory_->CreateNetworkMonitor()); if (!network_monitor_) { return; } @@ -849,6 +854,7 @@ void BasicNetworkManager::StopNetworkMonitor() { } void BasicNetworkManager::OnMessage(Message* msg) { + RTC_DCHECK_RUN_ON(thread_); switch (msg->message_id) { case kUpdateNetworksMessage: { UpdateNetworksContinually(); @@ -864,7 +870,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 +898,6 @@ void BasicNetworkManager::UpdateNetworksOnce() { if (!start_count_) return; - RTC_DCHECK(Thread::Current() == thread_); - NetworkList list; if (!CreateNetworks(false, &list)) { SignalError(); @@ -918,6 +921,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:"; diff --git a/rtc_base/network.h b/rtc_base/network.h index a67d2a2339..26ef628d8a 100644 --- a/rtc_base/network.h +++ b/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& 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 network_ignore_list_; - std::unique_ptr network_monitor_; + NetworkMonitorFactory* network_monitor_factory_ RTC_GUARDED_BY(thread_) = + nullptr; + std::unique_ptr network_monitor_ + RTC_GUARDED_BY(thread_); }; // Represents a Unix-type network interface, with a name and single address. diff --git a/rtc_base/network_monitor.cc b/rtc_base/network_monitor.cc index 4eb52901f3..8fd5f786d3 100644 --- a/rtc_base/network_monitor.cc +++ b/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 diff --git a/rtc_base/network_monitor.h b/rtc_base/network_monitor.h index ed4464db55..d0f342480f 100644 --- a/rtc_base/network_monitor.h +++ b/rtc_base/network_monitor.h @@ -98,26 +98,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_ diff --git a/rtc_base/network_monitor_factory.cc b/rtc_base/network_monitor_factory.cc new file mode 100644 index 0000000000..9fac4d95a0 --- /dev/null +++ b/rtc_base/network_monitor_factory.cc @@ -0,0 +1,18 @@ +/* + * 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 rtc { + +NetworkMonitorFactory::NetworkMonitorFactory() {} +NetworkMonitorFactory::~NetworkMonitorFactory() {} + +} // namespace rtc diff --git a/rtc_base/network_monitor_factory.h b/rtc_base/network_monitor_factory.h new file mode 100644 index 0000000000..dadcd4aa8a --- /dev/null +++ b/rtc_base/network_monitor_factory.h @@ -0,0 +1,37 @@ +/* + * 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: + virtual NetworkMonitorInterface* CreateNetworkMonitor() = 0; + + virtual ~NetworkMonitorFactory(); + + protected: + NetworkMonitorFactory(); +}; + +} // namespace rtc + +#endif // RTC_BASE_NETWORK_MONITOR_FACTORY_H_ diff --git a/rtc_base/network_unittest.cc b/rtc_base/network_unittest.cc index cd693563e7..29a89ba1be 100644 --- a/rtc_base/network_unittest.cc +++ b/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 #include @@ -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( 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 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 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". 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(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(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()); } } diff --git a/sdk/android/native_api/base/network_monitor.h b/sdk/android/native_api/base/network_monitor.h index 135ebb1e86..d0d354961f 100644 --- a/sdk/android/native_api/base/network_monitor.h +++ b/sdk/android/native_api/base/network_monitor.h @@ -15,7 +15,7 @@ #include -#include "rtc_base/network_monitor.h" +#include "rtc_base/network_monitor_factory.h" namespace webrtc { diff --git a/sdk/android/native_api/peerconnection/peer_connection_factory.cc b/sdk/android/native_api/peerconnection/peer_connection_factory.cc index e6839754ac..4e742d1b7a 100644 --- a/sdk/android/native_api/peerconnection/peer_connection_factory.cc +++ b/sdk/android/native_api/peerconnection/peer_connection_factory.cc @@ -23,11 +23,10 @@ jobject NativeToJavaPeerConnectionFactory( rtc::scoped_refptr pcf, std::unique_ptr network_thread, std::unique_ptr worker_thread, - std::unique_ptr signaling_thread, - rtc::NetworkMonitorFactory* network_monitor_factory) { + std::unique_ptr 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 diff --git a/sdk/android/native_api/peerconnection/peer_connection_factory.h b/sdk/android/native_api/peerconnection/peer_connection_factory.h index 889d6092e7..00550a9b12 100644 --- a/sdk/android/native_api/peerconnection/peer_connection_factory.h +++ b/sdk/android/native_api/peerconnection/peer_connection_factory.h @@ -26,8 +26,7 @@ jobject NativeToJavaPeerConnectionFactory( rtc::scoped_refptr pcf, std::unique_ptr network_thread, std::unique_ptr worker_thread, - std::unique_ptr signaling_thread, - rtc::NetworkMonitorFactory* network_monitor_factory = nullptr); + std::unique_ptr signaling_thread); } // namespace webrtc diff --git a/sdk/android/native_unittests/peerconnection/peer_connection_factory_unittest.cc b/sdk/android/native_unittests/peerconnection/peer_connection_factory_unittest.cc index 54613f9f57..75535d052b 100644 --- a/sdk/android/native_unittests/peerconnection/peer_connection_factory_unittest.cc +++ b/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; diff --git a/sdk/android/src/jni/android_network_monitor.h b/sdk/android/src/jni/android_network_monitor.h index 1d795df991..815b72d64d 100644 --- a/sdk/android/src/jni/android_network_monitor.h +++ b/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" diff --git a/sdk/android/src/jni/pc/owned_factory_and_threads.cc b/sdk/android/src/jni/pc/owned_factory_and_threads.cc index e42b117e57..5e00ece8ce 100644 --- a/sdk/android/src/jni/pc/owned_factory_and_threads.cc +++ b/sdk/android/src/jni/pc/owned_factory_and_threads.cc @@ -19,19 +19,11 @@ OwnedFactoryAndThreads::OwnedFactoryAndThreads( std::unique_ptr network_thread, std::unique_ptr worker_thread, std::unique_ptr signaling_thread, - rtc::NetworkMonitorFactory* network_monitor_factory, const rtc::scoped_refptr& 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 diff --git a/sdk/android/src/jni/pc/owned_factory_and_threads.h b/sdk/android/src/jni/pc/owned_factory_and_threads.h index 845d4dbd70..e87879c13f 100644 --- a/sdk/android/src/jni/pc/owned_factory_and_threads.h +++ b/sdk/android/src/jni/pc/owned_factory_and_threads.h @@ -33,25 +33,19 @@ class OwnedFactoryAndThreads { std::unique_ptr network_thread, std::unique_ptr worker_thread, std::unique_ptr signaling_thread, - rtc::NetworkMonitorFactory* network_monitor_factory, const rtc::scoped_refptr& 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 network_thread_; const std::unique_ptr worker_thread_; const std::unique_ptr signaling_thread_; - rtc::NetworkMonitorFactory* network_monitor_factory_; const rtc::scoped_refptr factory_; }; diff --git a/sdk/android/src/jni/pc/peer_connection_factory.cc b/sdk/android/src/jni/pc/peer_connection_factory.cc index 9a42a80ef8..2392db2403 100644 --- a/sdk/android/src/jni/pc/peer_connection_factory.cc +++ b/sdk/android/src/jni/pc/peer_connection_factory.cc @@ -138,11 +138,10 @@ ScopedJavaLocalRef NativeToScopedJavaPeerConnectionFactory( rtc::scoped_refptr pcf, std::unique_ptr network_thread, std::unique_ptr worker_thread, - std::unique_ptr signaling_thread, - rtc::NetworkMonitorFactory* network_monitor_factory) { + std::unique_ptr 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 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 pcf, std::unique_ptr network_thread, std::unique_ptr worker_thread, - std::unique_ptr signaling_thread, - rtc::NetworkMonitorFactory* network_monitor_factory) { + std::unique_ptr 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 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 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 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(); + } cricket::MediaEngineDependencies media_dependencies; media_dependencies.task_queue_factory = dependencies.task_queue_factory.get(); @@ -336,7 +328,7 @@ ScopedJavaLocalRef 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 diff --git a/sdk/android/src/jni/pc/peer_connection_factory.h b/sdk/android/src/jni/pc/peer_connection_factory.h index 904352f425..5bfdb7a808 100644 --- a/sdk/android/src/jni/pc/peer_connection_factory.h +++ b/sdk/android/src/jni/pc/peer_connection_factory.h @@ -24,8 +24,7 @@ jobject NativeToJavaPeerConnectionFactory( rtc::scoped_refptr pcf, std::unique_ptr network_thread, std::unique_ptr worker_thread, - std::unique_ptr signaling_thread, - rtc::NetworkMonitorFactory* network_monitor_factory = nullptr); + std::unique_ptr signaling_thread); } // namespace jni } // namespace webrtc