Revert "Reland "Pass NetworkMonitorFactory through PeerConnectionFactory.""
This reverts commit 7ded73351870bfb45160fa6b9db71a94fe49397b.
Reason for revert: Found more code calling NetworkMonitorFactory::SetFactory...
Original change's description:
> Reland "Pass NetworkMonitorFactory through PeerConnectionFactory."
>
> This is a reland of 003c9be817817ed0e3aef3f50c78ae5cb31bc0ff
>
> 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: Ibf69a22e8f94226908636c7d50ff9eda65bd4129
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/180720
> Reviewed-by: Taylor <deadbeef@webrtc.org>
> Commit-Queue: Taylor <deadbeef@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#31822}
TBR=deadbeef@webrtc.org,sakal@webrtc.org,hta@webrtc.org
Change-Id: Iae51b94072cec9abc021eed4e51d1fbeee998adc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:9883
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/180721
Reviewed-by: Taylor <deadbeef@webrtc.org>
Commit-Queue: Taylor <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31823}
diff --git a/rtc_base/network_unittest.cc b/rtc_base/network_unittest.cc
index 29a89ba..cd69356 100644
--- a/rtc_base/network_unittest.cc
+++ b/rtc_base/network_unittest.cc
@@ -19,7 +19,6 @@
#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>
@@ -101,27 +100,18 @@
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());
}
@@ -146,7 +136,6 @@
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(),
@@ -258,8 +247,6 @@
class TestBasicNetworkManager : public BasicNetworkManager {
public:
- TestBasicNetworkManager(NetworkMonitorFactory* network_monitor_factory)
- : BasicNetworkManager(network_monitor_factory) {}
using BasicNetworkManager::QueryDefaultLocalAddress;
using BasicNetworkManager::set_default_local_addresses;
};
@@ -281,7 +268,6 @@
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));
}
@@ -292,18 +278,14 @@
24);
Network include_me("include_me", "Include me please!", IPAddress(0x12345600U),
24);
- 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;
+ BasicNetworkManager network_manager;
+ EXPECT_FALSE(IsIgnoredNetwork(network_manager, ignore_me));
+ EXPECT_FALSE(IsIgnoredNetwork(network_manager, include_me));
std::vector<std::string> ignore_list;
ignore_list.push_back("ignore_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));
+ network_manager.set_network_ignore_list(ignore_list);
+ EXPECT_TRUE(IsIgnoredNetwork(network_manager, ignore_me));
+ EXPECT_FALSE(IsIgnoredNetwork(network_manager, include_me));
}
// Test is failing on Windows opt: b/11288214
@@ -667,7 +649,6 @@
// 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);
@@ -676,7 +657,6 @@
TEST_F(NetworkTest, TestIPv6Toggle) {
BasicNetworkManager manager;
- manager.StartUpdating();
bool ipv6_found = false;
NetworkManager::NetworkList list;
list = GetNetworks(manager, true);
@@ -773,7 +753,6 @@
NetworkManager::NetworkList result;
BasicNetworkManager manager;
- manager.StartUpdating();
CallConvertIfAddrs(manager, &list, true, &result);
EXPECT_TRUE(result.empty());
}
@@ -789,7 +768,6 @@
"FFFF:FFFF:FFFF:FFFF::", 0);
NetworkManager::NetworkList result;
BasicNetworkManager manager;
- manager.StartUpdating();
CallConvertIfAddrs(manager, list, true, &result);
EXPECT_EQ(1U, result.size());
bool changed;
@@ -809,35 +787,46 @@
NetworkManager::NetworkList result;
BasicNetworkManager manager;
- manager.StartUpdating();
CallConvertIfAddrs(manager, &list, true, &result);
EXPECT_TRUE(result.empty());
}
-// Tests that the network type can be determined from the network monitor when
-// it would otherwise be unknown.
+// Tests that the network type can be updated after the network monitor is
+// started.
TEST_F(NetworkTest, TestGetAdapterTypeFromNetworkMonitor) {
- char if_name[20] = "wifi0";
- std::string ipv6_address = "1000:2000:3000:4000:0:0:0:1";
+ 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";
std::string ipv6_mask = "FFFF:FFFF:FFFF:FFFF::";
- 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));
+ 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));
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.
- // With the fake network monitor the type should be correctly determined.
- FakeNetworkMonitorFactory factory;
- BasicNetworkManager manager_with_monitor(&factory);
- manager_with_monitor.StartUpdating();
+ // 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();
// Add the same ipv6 address as before but it has the right network type
// detected by the network monitor now.
- addr_list = InstallIpv6Network(if_name, ipv6_address, ipv6_mask,
- manager_with_monitor);
- EXPECT_EQ(ADAPTER_TYPE_WIFI, GetAdapterType(manager_with_monitor));
+ addr_list = InstallIpv6Network(if_name1, ipv6_address1, ipv6_mask, manager);
+ EXPECT_EQ(ADAPTER_TYPE_WIFI, GetAdapterType(manager));
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
@@ -850,7 +839,6 @@
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";
@@ -1034,10 +1022,11 @@
}
TEST_F(NetworkTest, TestNetworkMonitoring) {
- FakeNetworkMonitorFactory factory;
- BasicNetworkManager manager(&factory);
+ BasicNetworkManager manager;
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());
@@ -1054,6 +1043,8 @@
// 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.
@@ -1064,10 +1055,11 @@
#endif
TEST_F(NetworkTest, MAYBE_DefaultLocalAddress) {
IPAddress ip;
- FakeNetworkMonitorFactory factory;
- TestBasicNetworkManager manager(&factory);
+ TestBasicNetworkManager manager;
manager.SignalNetworksChanged.connect(static_cast<NetworkTest*>(this),
&NetworkTest::OnNetworksChanged);
+ FakeNetworkMonitorFactory* factory = new FakeNetworkMonitorFactory();
+ NetworkMonitorFactory::SetFactory(factory);
manager.StartUpdating();
EXPECT_TRUE_WAIT(callback_called_, 1000);
@@ -1078,12 +1070,12 @@
EXPECT_TRUE(!networks.empty());
for (const auto* network : networks) {
if (network->GetBestIP().family() == AF_INET) {
- EXPECT_TRUE(QueryDefaultLocalAddress(manager, AF_INET) != IPAddress());
+ EXPECT_TRUE(manager.QueryDefaultLocalAddress(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(QueryDefaultLocalAddress(manager, AF_INET6) != IPAddress());
+ EXPECT_TRUE(manager.QueryDefaultLocalAddress(AF_INET6) != IPAddress());
}
}