fix a bug in the logic when new Networks are merged. This happens when
we have 2 networks with the same key
BUG=410554 in chromium
http://code.google.com/p/chromium/issues/detail?id=410554
Corresponding change in chromium is
https://codereview.chromium.org/536133003/
R=jiayl@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/19249005
git-svn-id: http://webrtc.googlecode.com/svn/trunk@7117 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/base/network.cc b/webrtc/base/network.cc
index d94c69e..686de97 100644
--- a/webrtc/base/network.cc
+++ b/webrtc/base/network.cc
@@ -65,6 +65,11 @@
const int kHighestNetworkPreference = 127;
+typedef struct {
+ Network* net;
+ std::vector<IPAddress> ips;
+} AddressList;
+
bool CompareNetworks(const Network* a, const Network* b) {
if (a->prefix_length() == b->prefix_length()) {
if (a->name() == b->name()) {
@@ -144,10 +149,12 @@
void NetworkManagerBase::MergeNetworkList(const NetworkList& new_networks,
bool* changed) {
- // Sort the list so that we can detect when it changes.
- typedef std::pair<Network*, std::vector<IPAddress> > address_list;
- std::map<std::string, address_list> address_map;
+ // 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);
@@ -162,16 +169,19 @@
std::string key = MakeNetworkKey(list[i]->name(),
list[i]->prefix(),
list[i]->prefix_length());
- if (address_map.find(key) == address_map.end()) {
- address_map[key] = address_list(list[i], std::vector<IPAddress>());
+ if (consolidated_address_list.find(key) ==
+ consolidated_address_list.end()) {
+ AddressList addrlist;
+ addrlist.net = list[i];
+ consolidated_address_list[key] = addrlist;
might_add_to_merged_list = true;
}
const std::vector<IPAddress>& addresses = list[i]->GetIPs();
- address_list& current_list = address_map[key];
+ AddressList& current_list = consolidated_address_list[key];
for (std::vector<IPAddress>::const_iterator it = addresses.begin();
it != addresses.end();
++it) {
- current_list.second.push_back(*it);
+ current_list.ips.push_back(*it);
}
if (!might_add_to_merged_list) {
delete list[i];
@@ -179,20 +189,24 @@
}
// Next, look for existing network objects to re-use.
- for (std::map<std::string, address_list >::iterator it = address_map.begin();
- it != address_map.end();
+ for (std::map<std::string, AddressList>::iterator it =
+ consolidated_address_list.begin();
+ it != consolidated_address_list.end();
++it) {
const std::string& key = it->first;
- Network* net = it->second.first;
+ Network* net = it->second.net;
NetworkMap::iterator existing = networks_map_.find(key);
if (existing == networks_map_.end()) {
// This network is new. Place it in the network map.
merged_list.push_back(net);
networks_map_[key] = net;
+ // Also, we might have accumulated IPAddresses from the first
+ // step, set it here.
+ net->SetIPs(it->second.ips, true);
*changed = true;
} else {
// This network exists in the map already. Reset its IP addresses.
- *changed = existing->second->SetIPs(it->second.second, *changed);
+ *changed = existing->second->SetIPs(it->second.ips, *changed);
merged_list.push_back(existing->second);
if (existing->second != net) {
delete net;
diff --git a/webrtc/base/network_unittest.cc b/webrtc/base/network_unittest.cc
index 431f8b4..8123f8b 100644
--- a/webrtc/base/network_unittest.cc
+++ b/webrtc/base/network_unittest.cc
@@ -614,4 +614,40 @@
}
#endif
+// Test MergeNetworkList successfully combines all IPs for the same
+// prefix/length into a single Network.
+TEST_F(NetworkTest, TestMergeNetworkList) {
+ BasicNetworkManager manager;
+ NetworkManager::NetworkList list;
+
+ // Create 2 IPAddress classes with only last digit different.
+ IPAddress ip1, ip2;
+ EXPECT_TRUE(IPFromString("2400:4030:1:2c00:be30:0:0:1", &ip1));
+ EXPECT_TRUE(IPFromString("2400:4030:1:2c00:be30:0:0:2", &ip2));
+
+ // Create 2 networks with the same prefix and length.
+ Network* net1 = new Network("em1", "em1", TruncateIP(ip1, 64), 64);
+ Network* net2 = new Network("em1", "em1", TruncateIP(ip1, 64), 64);
+
+ // Add different IP into each.
+ net1->AddIP(ip1);
+ net2->AddIP(ip2);
+
+ list.push_back(net1);
+ list.push_back(net2);
+ bool changed;
+ MergeNetworkList(manager, list, &changed);
+ EXPECT_TRUE(changed);
+
+ NetworkManager::NetworkList list2;
+ manager.GetNetworks(&list2);
+
+ // Make sure the resulted networklist has only 1 element and 2
+ // IPAddresses.
+ EXPECT_EQ(list2.size(), 1uL);
+ EXPECT_EQ(list2[0]->GetIPs().size(), 2uL);
+ EXPECT_EQ(list2[0]->GetIPs()[0], ip1);
+ EXPECT_EQ(list2[0]->GetIPs()[1], ip2);
+}
+
} // namespace rtc
