From 539f3e1a89c70f1007c5bc36c191f019a96bb134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Fri, 26 Nov 2021 16:33:19 +0100 Subject: [PATCH] Deprecate BasicNetworkManager default constructor Replaced with a constructor with a SocketFactory argument. Bug: webrtc:13145 Change-Id: I30db4ad089009284e1be8a6bbdadd5a671e93713 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/239180 Reviewed-by: Harald Alvestrand Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/main@{#35508} --- examples/stunprober/main.cc | 2 +- rtc_base/network.cc | 3 ++ rtc_base/network.h | 4 +- rtc_base/network_unittest.cc | 76 ++++++++++++++++++++++++------------ 4 files changed, 57 insertions(+), 28 deletions(-) diff --git a/examples/stunprober/main.cc b/examples/stunprober/main.cc index fa5825c82e..d0ed92cc34 100644 --- a/examples/stunprober/main.cc +++ b/examples/stunprober/main.cc @@ -128,7 +128,7 @@ int main(int argc, char* argv[]) { auto socket_factory = std::make_unique(&socket_server); std::unique_ptr network_manager( - new rtc::BasicNetworkManager()); + new rtc::BasicNetworkManager(&socket_server)); rtc::NetworkManager::NetworkList networks; network_manager->GetNetworks(&networks); auto prober = std::make_unique(socket_factory.get(), diff --git a/rtc_base/network.cc b/rtc_base/network.cc index eefee969d2..870f22a3a9 100644 --- a/rtc_base/network.cc +++ b/rtc_base/network.cc @@ -511,6 +511,9 @@ bool NetworkManagerBase::IsVpnMacAddress( BasicNetworkManager::BasicNetworkManager() : BasicNetworkManager(nullptr, nullptr) {} +BasicNetworkManager::BasicNetworkManager(SocketFactory* socket_factory) + : BasicNetworkManager(nullptr, socket_factory) {} + BasicNetworkManager::BasicNetworkManager( NetworkMonitorFactory* network_monitor_factory) : BasicNetworkManager(network_monitor_factory, nullptr) {} diff --git a/rtc_base/network.h b/rtc_base/network.h index 12103d7d30..0b462bdede 100644 --- a/rtc_base/network.h +++ b/rtc_base/network.h @@ -255,8 +255,10 @@ class RTC_EXPORT BasicNetworkManager : public NetworkManagerBase, public NetworkBinderInterface, public sigslot::has_slots<> { public: + ABSL_DEPRECATED( + "Use the version with socket_factory, see bugs.webrtc.org/13145") BasicNetworkManager(); - + explicit BasicNetworkManager(SocketFactory* socket_factory); ABSL_DEPRECATED( "Use the version with socket_factory, see bugs.webrtc.org/13145") explicit BasicNetworkManager(NetworkMonitorFactory* network_monitor_factory); diff --git a/rtc_base/network_unittest.cc b/rtc_base/network_unittest.cc index 87df0d3230..da2fe96bf6 100644 --- a/rtc_base/network_unittest.cc +++ b/rtc_base/network_unittest.cc @@ -337,7 +337,8 @@ TEST_F(NetworkTest, TestIsIgnoredNetworkIgnoresIPsStartingWith0) { IPAddress(0x12345600U), 24, ADAPTER_TYPE_ETHERNET); Network ipv4_network2("test_eth1", "Test Network Adapter 2", IPAddress(0x010000U), 24, ADAPTER_TYPE_ETHERNET); - BasicNetworkManager network_manager; + PhysicalSocketServer socket_server; + BasicNetworkManager network_manager(&socket_server); network_manager.StartUpdating(); EXPECT_FALSE(IsIgnoredNetwork(network_manager, ipv4_network1)); EXPECT_TRUE(IsIgnoredNetwork(network_manager, ipv4_network2)); @@ -349,12 +350,13 @@ TEST_F(NetworkTest, TestIgnoreList) { 24); Network include_me("include_me", "Include me please!", IPAddress(0x12345600U), 24); - BasicNetworkManager default_network_manager; + PhysicalSocketServer socket_server; + BasicNetworkManager default_network_manager(&socket_server); default_network_manager.StartUpdating(); EXPECT_FALSE(IsIgnoredNetwork(default_network_manager, ignore_me)); EXPECT_FALSE(IsIgnoredNetwork(default_network_manager, include_me)); - BasicNetworkManager ignoring_network_manager; + BasicNetworkManager ignoring_network_manager(&socket_server); std::vector ignore_list; ignore_list.push_back("ignore_me"); ignoring_network_manager.set_network_ignore_list(ignore_list); @@ -365,7 +367,8 @@ TEST_F(NetworkTest, TestIgnoreList) { // Test is failing on Windows opt: b/11288214 TEST_F(NetworkTest, DISABLED_TestCreateNetworks) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); NetworkManager::NetworkList result = GetNetworks(manager, true); // We should be able to bind to any addresses we find. NetworkManager::NetworkList::iterator it; @@ -440,7 +443,8 @@ TEST_F(NetworkTest, TestBasicMergeNetworkList) { IPAddress(0x00010000U), 16); ipv4_network1.AddIP(IPAddress(0x12345678)); ipv4_network2.AddIP(IPAddress(0x00010004)); - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); // Add ipv4_network1 to the list of networks. NetworkManager::NetworkList list; @@ -549,7 +553,8 @@ void SetupNetworks(NetworkManager::NetworkList* list) { // Test that the basic network merging case works. TEST_F(NetworkTest, TestIPv6MergeNetworkList) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.SignalNetworksChanged.connect(static_cast(this), &NetworkTest::OnNetworksChanged); NetworkManager::NetworkList original_list; @@ -570,7 +575,8 @@ TEST_F(NetworkTest, TestIPv6MergeNetworkList) { // merged, that the changed callback is not called, and that the original // objects remain in the result list. TEST_F(NetworkTest, TestNoChangeMerge) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.SignalNetworksChanged.connect(static_cast(this), &NetworkTest::OnNetworksChanged); NetworkManager::NetworkList original_list; @@ -598,7 +604,8 @@ TEST_F(NetworkTest, TestNoChangeMerge) { // a different IP. The original network should remain in the list, but have its // IP changed. TEST_F(NetworkTest, MergeWithChangedIP) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.SignalNetworksChanged.connect(static_cast(this), &NetworkTest::OnNetworksChanged); NetworkManager::NetworkList original_list; @@ -634,7 +641,8 @@ TEST_F(NetworkTest, MergeWithChangedIP) { // Testing a similar case to above, but checking that a network can be updated // with additional IPs (not just a replacement). TEST_F(NetworkTest, TestMultipleIPMergeNetworkList) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.SignalNetworksChanged.connect(static_cast(this), &NetworkTest::OnNetworksChanged); NetworkManager::NetworkList original_list; @@ -683,7 +691,8 @@ TEST_F(NetworkTest, TestMultipleIPMergeNetworkList) { // Test that merge correctly distinguishes multiple networks on an interface. TEST_F(NetworkTest, TestMultiplePublicNetworksOnOneInterfaceMerge) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.SignalNetworksChanged.connect(static_cast(this), &NetworkTest::OnNetworksChanged); NetworkManager::NetworkList original_list; @@ -724,7 +733,8 @@ TEST_F(NetworkTest, TestMultiplePublicNetworksOnOneInterfaceMerge) { // Test that DumpNetworks does not crash. TEST_F(NetworkTest, TestCreateAndDumpNetworks) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.StartUpdating(); NetworkManager::NetworkList list = GetNetworks(manager, true); bool changed; @@ -733,7 +743,8 @@ TEST_F(NetworkTest, TestCreateAndDumpNetworks) { } TEST_F(NetworkTest, TestIPv6Toggle) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.StartUpdating(); bool ipv6_found = false; NetworkManager::NetworkList list; @@ -755,7 +766,8 @@ TEST_F(NetworkTest, TestIPv6Toggle) { // Test that when network interfaces are sorted and given preference values, // IPv6 comes first. TEST_F(NetworkTest, IPv6NetworksPreferredOverIPv4) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); Network ipv4_network1("test_eth0", "Test Network Adapter 1", IPAddress(0x12345600U), 24); ipv4_network1.AddIP(IPAddress(0x12345600U)); @@ -784,7 +796,8 @@ TEST_F(NetworkTest, IPv6NetworksPreferredOverIPv4) { // When two interfaces are equivalent in everything but name, they're expected // to be preference-ordered by name. For example, "eth0" before "eth1". TEST_F(NetworkTest, NetworksSortedByInterfaceName) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); Network* eth0 = new Network("test_eth0", "Test Network Adapter 1", IPAddress(0x65432100U), 24); eth0->AddIP(IPAddress(0x65432100U)); @@ -830,7 +843,8 @@ TEST_F(NetworkTest, TestConvertIfAddrsNoAddress) { list.ifa_name = const_cast("test_iface"); NetworkManager::NetworkList result; - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.StartUpdating(); CallConvertIfAddrs(manager, &list, true, &result); EXPECT_TRUE(result.empty()); @@ -846,7 +860,8 @@ TEST_F(NetworkTest, TestConvertIfAddrsMultiAddressesOnOneInterface) { list = AddIpv6Address(list, if_name, "1000:2000:3000:4000:0:0:0:2", "FFFF:FFFF:FFFF:FFFF::", 0); NetworkManager::NetworkList result; - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.StartUpdating(); CallConvertIfAddrs(manager, list, true, &result); EXPECT_EQ(1U, result.size()); @@ -866,7 +881,8 @@ TEST_F(NetworkTest, TestConvertIfAddrsNotRunning) { list.ifa_netmask = &ifa_netmask; NetworkManager::NetworkList result; - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.StartUpdating(); CallConvertIfAddrs(manager, &list, true, &result); EXPECT_TRUE(result.empty()); @@ -908,7 +924,8 @@ TEST_F(NetworkTest, TestGetAdapterTypeFromNameMatching) { std::string ipv6_address1 = "1000:2000:3000:4000:0:0:0:1"; std::string ipv6_address2 = "1000:2000:3000:8000:0:0:0:1"; std::string ipv6_mask = "FFFF:FFFF:FFFF:FFFF::"; - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.StartUpdating(); // IPSec interface; name is in form "ipsec". @@ -1011,7 +1028,8 @@ TEST_F(NetworkTest, TestNetworkMonitorIsAdapterAvailable) { // Test MergeNetworkList successfully combines all IPs for the same // prefix/length into a single Network. TEST_F(NetworkTest, TestMergeNetworkList) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); NetworkManager::NetworkList list; // Create 2 IPAddress classes with only last digit different. @@ -1047,7 +1065,8 @@ TEST_F(NetworkTest, TestMergeNetworkList) { // Test that MergeNetworkList successfully detects the change if // a network becomes inactive and then active again. TEST_F(NetworkTest, TestMergeNetworkListWithInactiveNetworks) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); Network network1("test_wifi", "Test Network Adapter 1", IPAddress(0x12345600U), 24); Network network2("test_eth0", "Test Network Adapter 2", @@ -1226,7 +1245,8 @@ TEST_F(NetworkTest, MAYBE_DefaultLocalAddress) { // Test that MergeNetworkList does not set change = true // when changing from cellular_X to cellular_Y. TEST_F(NetworkTest, TestWhenNetworkListChangeReturnsChangedFlag) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); IPAddress ip1; EXPECT_TRUE(IPFromString("2400:4030:1:2c00:be30:0:0:1", &ip1)); @@ -1288,7 +1308,8 @@ TEST_F(NetworkTest, TestWhenNetworkListChangeReturnsChangedFlag) { TEST_F(NetworkTest, IgnoresMACBasedIPv6Address) { std::string ipv6_address = "2607:fc20:f340:1dc8:214:22ff:fe01:2345"; std::string ipv6_mask = "FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF"; - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.StartUpdating(); // IPSec interface; name is in form "ipsec". @@ -1307,7 +1328,8 @@ TEST_F(NetworkTest, WebRTC_AllowMACBasedIPv6Address) { "WebRTC-AllowMACBasedIPv6/Enabled/"); std::string ipv6_address = "2607:fc20:f340:1dc8:214:22ff:fe01:2345"; std::string ipv6_mask = "FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF"; - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.StartUpdating(); // IPSec interface; name is in form "ipsec". @@ -1400,8 +1422,9 @@ TEST_F(NetworkTest, NetworkCostVpn_VpnMoreExpensive) { } TEST_F(NetworkTest, VpnList) { + PhysicalSocketServer socket_server; { - BasicNetworkManager manager; + BasicNetworkManager manager(&socket_server); manager.set_vpn_list({NetworkMask(IPFromString("192.168.0.0"), 16)}); manager.StartUpdating(); EXPECT_TRUE(manager.IsConfiguredVpn(IPFromString("192.168.1.1"), 32)); @@ -1413,7 +1436,7 @@ TEST_F(NetworkTest, VpnList) { EXPECT_FALSE(manager.IsConfiguredVpn(IPFromString("192.168.0.0"), 15)); } { - BasicNetworkManager manager; + BasicNetworkManager manager(&socket_server); manager.set_vpn_list({NetworkMask(IPFromString("192.168.0.0"), 24)}); manager.StartUpdating(); EXPECT_FALSE(manager.IsConfiguredVpn(IPFromString("192.168.1.1"), 32)); @@ -1424,7 +1447,8 @@ TEST_F(NetworkTest, VpnList) { #if defined(WEBRTC_POSIX) // TODO(webrtc:13114): Implement the InstallIpv4Network for windows. TEST_F(NetworkTest, VpnListOverrideAdapterType) { - BasicNetworkManager manager; + PhysicalSocketServer socket_server; + BasicNetworkManager manager(&socket_server); manager.set_vpn_list({NetworkMask(IPFromString("192.168.0.0"), 16)}); manager.StartUpdating();