From 003c9be817817ed0e3aef3f50c78ae5cb31bc0ff Mon Sep 17 00:00:00 2001 From: Taylor Brandstetter Date: Wed, 29 Jul 2020 21:10:28 +0000 Subject: [PATCH] Pass NetworkMonitorFactory through PeerConnectionFactory. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Sami Kalliomäki Commit-Queue: Taylor Cr-Commit-Position: refs/heads/master@{#31815} --- api/DEPS | 2 +- api/peer_connection_interface.h | 6 +- pc/peer_connection_factory.cc | 6 +- pc/peer_connection_factory.h | 1 + rtc_base/BUILD.gn | 4 + rtc_base/network.cc | 22 +++-- rtc_base/network.h | 34 ++++--- rtc_base/network_monitor.cc | 25 ----- rtc_base/network_monitor.h | 20 ---- rtc_base/network_monitor_factory.cc | 18 ++++ rtc_base/network_monitor_factory.h | 37 +++++++ rtc_base/network_unittest.cc | 98 ++++++++++--------- sdk/android/native_api/base/network_monitor.h | 2 +- .../peerconnection/peer_connection_factory.cc | 5 +- .../peerconnection/peer_connection_factory.h | 3 +- .../peer_connection_factory_unittest.cc | 2 +- sdk/android/src/jni/android_network_monitor.h | 1 + .../src/jni/pc/owned_factory_and_threads.cc | 8 -- .../src/jni/pc/owned_factory_and_threads.h | 8 +- .../src/jni/pc/peer_connection_factory.cc | 26 ++--- .../src/jni/pc/peer_connection_factory.h | 3 +- 21 files changed, 176 insertions(+), 155 deletions(-) create mode 100644 rtc_base/network_monitor_factory.cc create mode 100644 rtc_base/network_monitor_factory.h 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