Fix two problems in network.cc:
1. It signals network changed events whenever there are more than one IP address in a network.
2. It does not signal network changed events if a network disconnects and connects again.
Also changed DumpNetworks for better debugging.
BUG=webrtc:5096
Review URL: https://codereview.webrtc.org/1421433003
Cr-Commit-Position: refs/heads/master@{#11107}
diff --git a/webrtc/base/network.cc b/webrtc/base/network.cc
index 678541d..2dcfa17 100644
--- a/webrtc/base/network.cc
+++ b/webrtc/base/network.cc
@@ -242,20 +242,12 @@
void NetworkManagerBase::MergeNetworkList(const NetworkList& new_networks,
bool* changed,
NetworkManager::Stats* stats) {
+ *changed = false;
// AddressList in this map will track IP addresses for all Networks
// with the same key.
std::map<std::string, AddressList> consolidated_address_list;
NetworkList list(new_networks);
-
- // Result of Network merge. Element in this list should have unique key.
- NetworkList merged_list;
std::sort(list.begin(), list.end(), CompareNetworks);
-
- *changed = false;
-
- if (networks_.size() != list.size())
- *changed = true;
-
// First, build a set of network-keys to the ipaddresses.
for (Network* network : list) {
bool might_add_to_merged_list = false;
@@ -287,6 +279,8 @@
}
// Next, look for existing network objects to re-use.
+ // Result of Network merge. Element in this list should have unique key.
+ NetworkList merged_list;
for (const auto& kv : consolidated_address_list) {
const std::string& key = kv.first;
Network* net = kv.second.net;
@@ -301,17 +295,36 @@
*changed = true;
} else {
// This network exists in the map already. Reset its IP addresses.
- *changed = existing->second->SetIPs(kv.second.ips, *changed);
- merged_list.push_back(existing->second);
- if (existing->second != net) {
+ Network* existing_net = existing->second;
+ *changed = existing_net->SetIPs(kv.second.ips, *changed);
+ merged_list.push_back(existing_net);
+ // If the existing network was not active, networks have changed.
+ if (!existing_net->active()) {
+ *changed = true;
+ }
+ ASSERT(net->active());
+ if (existing_net != net) {
delete net;
}
}
}
- networks_ = merged_list;
+ // It may still happen that the merged list is a subset of |networks_|.
+ // To detect this change, we compare their sizes.
+ if (merged_list.size() != networks_.size()) {
+ *changed = true;
+ }
- // If the network lists changes, we resort it.
+ // If the network list changes, we re-assign |networks_| to the merged list
+ // and re-sort it.
if (*changed) {
+ networks_ = merged_list;
+ // Reset the active states of all networks.
+ for (const auto& kv : networks_map_) {
+ kv.second->set_active(false);
+ }
+ for (Network* network : networks_) {
+ network->set_active(true);
+ }
std::sort(networks_.begin(), networks_.end(), SortNetworks);
// Now network interfaces are sorted, we should set the preference value
// for each of the interfaces we are planning to use.
@@ -450,6 +463,7 @@
network->AddIP(ip);
network->set_ignored(IsIgnoredNetwork(*network));
if (include_ignored || !network->ignored()) {
+ current_networks[key] = network.get();
networks->push_back(network.release());
}
} else {
@@ -604,6 +618,7 @@
bool ignored = IsIgnoredNetwork(*network);
network->set_ignored(ignored);
if (include_ignored || !network->ignored()) {
+ current_networks[key] = network.get();
networks->push_back(network.release());
}
} else {
@@ -801,21 +816,14 @@
thread_->PostDelayed(kNetworksUpdateIntervalMs, this, kUpdateNetworksMessage);
}
-void BasicNetworkManager::DumpNetworks(bool include_ignored) {
+void BasicNetworkManager::DumpNetworks() {
NetworkList list;
- CreateNetworks(include_ignored, &list);
+ GetNetworks(&list);
LOG(LS_INFO) << "NetworkManager detected " << list.size() << " networks:";
for (const Network* network : list) {
- if (!network->ignored() || include_ignored) {
- LOG(LS_INFO) << network->ToString() << ": "
- << network->description()
- << ((network->ignored()) ? ", Ignored" : "");
- }
- }
- // Release the network list created previously.
- // Do this in a seperated for loop for better readability.
- for (Network* network : list) {
- delete network;
+ LOG(LS_INFO) << network->ToString() << ": " << network->description()
+ << ", active ? " << network->active()
+ << ((network->ignored()) ? ", Ignored" : "");
}
}
diff --git a/webrtc/base/network.h b/webrtc/base/network.h
index 8980b5d..d2e013e 100644
--- a/webrtc/base/network.h
+++ b/webrtc/base/network.h
@@ -111,11 +111,10 @@
// TODO(guoweis): remove this body when chromium implements this.
virtual void GetAnyAddressNetworks(NetworkList* networks) {}
+ // Dumps the current list of networks in the network manager.
+ virtual void DumpNetworks() {}
bool GetDefaultLocalAddress(int family, IPAddress* ipaddr) const override;
- // Dumps a list of networks available to LS_INFO.
- virtual void DumpNetworks(bool include_ignored) {}
-
struct Stats {
int ipv4_network_count;
int ipv6_network_count;
@@ -195,8 +194,7 @@
void StartUpdating() override;
void StopUpdating() override;
- // Logs the available networks.
- void DumpNetworks(bool include_ignored) override;
+ void DumpNetworks() override;
// MessageHandler interface.
void OnMessage(Message* msg) override;
@@ -359,6 +357,12 @@
int preference() const { return preference_; }
void set_preference(int preference) { preference_ = preference; }
+ // When we enumerate networks and find a previously-seen network is missing,
+ // we do not remove it (because it may be used elsewhere). Instead, we mark
+ // it inactive, so that we can detect network changes properly.
+ bool active() const { return active_; }
+ void set_active(bool active) { active_ = active; }
+
// Debugging description of this network
std::string ToString() const;
@@ -374,6 +378,7 @@
bool ignored_;
AdapterType type_;
int preference_;
+ bool active_ = true;
friend class NetworkManager;
};
diff --git a/webrtc/base/network_unittest.cc b/webrtc/base/network_unittest.cc
index cfba62b..5ae7589 100644
--- a/webrtc/base/network_unittest.cc
+++ b/webrtc/base/network_unittest.cc
@@ -91,6 +91,46 @@
NetworkManager::NetworkList* networks) {
network_manager.ConvertIfAddrs(interfaces, include_ignored, networks);
}
+
+ struct sockaddr_in6* CreateIpv6Addr(const std::string& ip_string,
+ uint32_t scope_id) {
+ struct sockaddr_in6* ipv6_addr = new struct sockaddr_in6;
+ memset(ipv6_addr, 0, sizeof(struct sockaddr_in6));
+ ipv6_addr->sin6_family = AF_INET6;
+ ipv6_addr->sin6_scope_id = scope_id;
+ IPAddress ip;
+ IPFromString(ip_string, &ip);
+ ipv6_addr->sin6_addr = ip.ipv6_address();
+ return ipv6_addr;
+ }
+
+ // Pointers created here need to be released via ReleaseIfAddrs.
+ struct ifaddrs* AddIpv6Address(struct ifaddrs* list,
+ char* if_name,
+ const std::string& ipv6_address,
+ const std::string& ipv6_netmask,
+ uint32_t scope_id) {
+ struct ifaddrs* if_addr = new struct ifaddrs;
+ memset(if_addr, 0, sizeof(struct ifaddrs));
+ if_addr->ifa_name = if_name;
+ if_addr->ifa_addr = reinterpret_cast<struct sockaddr*>(
+ CreateIpv6Addr(ipv6_address, scope_id));
+ if_addr->ifa_netmask =
+ reinterpret_cast<struct sockaddr*>(CreateIpv6Addr(ipv6_netmask, 0));
+ if_addr->ifa_next = list;
+ return if_addr;
+ }
+
+ void ReleaseIfAddrs(struct ifaddrs* list) {
+ struct ifaddrs* if_addr = list;
+ while (if_addr != nullptr) {
+ struct ifaddrs* next_addr = if_addr->ifa_next;
+ delete if_addr->ifa_addr;
+ delete if_addr->ifa_netmask;
+ delete if_addr;
+ if_addr = next_addr;
+ }
+ }
#endif // defined(WEBRTC_POSIX)
protected:
@@ -590,10 +630,13 @@
}
}
-// Test that DumpNetworks works.
-TEST_F(NetworkTest, TestDumpNetworks) {
+// Test that DumpNetworks does not crash.
+TEST_F(NetworkTest, TestCreateAndDumpNetworks) {
BasicNetworkManager manager;
- manager.DumpNetworks(true);
+ NetworkManager::NetworkList list = GetNetworks(manager, true);
+ bool changed;
+ MergeNetworkList(manager, list, &changed);
+ manager.DumpNetworks();
}
// Test that we can toggle IPv6 on and off.
@@ -700,6 +743,25 @@
CallConvertIfAddrs(manager, &list, true, &result);
EXPECT_TRUE(result.empty());
}
+
+// Verify that if there are two addresses on one interface, only one network
+// is generated.
+TEST_F(NetworkTest, TestConvertIfAddrsMultiAddressesOnOneInterface) {
+ char if_name[20] = "rmnet0";
+ ifaddrs* list = nullptr;
+ list = AddIpv6Address(list, if_name, "1000:2000:3000:4000:0:0:0:1",
+ "FFFF:FFFF:FFFF:FFFF::", 0);
+ list = AddIpv6Address(list, if_name, "1000:2000:3000:4000:0:0:0:2",
+ "FFFF:FFFF:FFFF:FFFF::", 0);
+ NetworkManager::NetworkList result;
+ BasicNetworkManager manager;
+ CallConvertIfAddrs(manager, list, true, &result);
+ EXPECT_EQ(1U, result.size());
+ bool changed;
+ // This ensures we release the objects created in CallConvertIfAddrs.
+ MergeNetworkList(manager, result, &changed);
+ ReleaseIfAddrs(list);
+}
#endif // defined(WEBRTC_POSIX)
#if defined(WEBRTC_LINUX) && !defined(WEBRTC_ANDROID)
@@ -783,6 +845,49 @@
EXPECT_EQ(list2[0]->GetIPs()[1], ip2);
}
+// Test that MergeNetworkList successfully detects the change if
+// a network becomes inactive and then active again.
+TEST_F(NetworkTest, TestMergeNetworkListWithInactiveNetworks) {
+ BasicNetworkManager manager;
+ Network network1("test_wifi", "Test Network Adapter 1",
+ IPAddress(0x12345600U), 24);
+ Network network2("test_eth0", "Test Network Adapter 2",
+ IPAddress(0x00010000U), 16);
+ network1.AddIP(IPAddress(0x12345678));
+ network2.AddIP(IPAddress(0x00010004));
+ NetworkManager::NetworkList list;
+ Network* net1 = new Network(network1);
+ list.push_back(net1);
+ bool changed;
+ MergeNetworkList(manager, list, &changed);
+ EXPECT_TRUE(changed);
+ list.clear();
+ manager.GetNetworks(&list);
+ ASSERT_EQ(1U, list.size());
+ EXPECT_EQ(net1, list[0]);
+
+ list.clear();
+ Network* net2 = new Network(network2);
+ list.push_back(net2);
+ MergeNetworkList(manager, list, &changed);
+ EXPECT_TRUE(changed);
+ list.clear();
+ manager.GetNetworks(&list);
+ ASSERT_EQ(1U, list.size());
+ EXPECT_EQ(net2, list[0]);
+
+ // Now network1 is inactive. Try to merge it again.
+ list.clear();
+ list.push_back(new Network(network1));
+ MergeNetworkList(manager, list, &changed);
+ EXPECT_TRUE(changed);
+ list.clear();
+ manager.GetNetworks(&list);
+ ASSERT_EQ(1U, list.size());
+ EXPECT_TRUE(list[0]->active());
+ EXPECT_EQ(net1, list[0]);
+}
+
// Test that the filtering logic follows the defined ruleset in network.h.
TEST_F(NetworkTest, TestIPv6Selection) {
InterfaceAddress ip;