When doing continual gathering, remove the local ports when a corresponding network is dropped.
BUG=
Review URL: https://codereview.webrtc.org/1696933003
Cr-Commit-Position: refs/heads/master@{#11660}
diff --git a/webrtc/base/network.cc b/webrtc/base/network.cc
index 6b04f89..4f3b919 100644
--- a/webrtc/base/network.cc
+++ b/webrtc/base/network.cc
@@ -317,10 +317,11 @@
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);
+ Network* network = kv.second;
+ // If |network| is in the newly generated |networks_|, it is active.
+ bool found = std::find(networks_.begin(), networks_.end(), network) !=
+ networks_.end();
+ network->set_active(found);
}
std::sort(networks_.begin(), networks_.end(), SortNetworks);
// Now network interfaces are sorted, we should set the preference value
diff --git a/webrtc/base/network.h b/webrtc/base/network.h
index 10ce6f0..a81bcab 100644
--- a/webrtc/base/network.h
+++ b/webrtc/base/network.h
@@ -265,6 +265,8 @@
AdapterType type);
~Network();
+ sigslot::signal1<const Network*> SignalInactive;
+
const DefaultLocalAddressProvider* default_local_address_provider() {
return default_local_address_provider_;
}
@@ -342,7 +344,15 @@
// 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; }
+ void set_active(bool active) {
+ if (active_ == active) {
+ return;
+ }
+ active_ = active;
+ if (!active) {
+ SignalInactive(this);
+ }
+ }
// Debugging description of this network
std::string ToString() const;
diff --git a/webrtc/base/network_unittest.cc b/webrtc/base/network_unittest.cc
index 0f7d6db..7ad45a3 100644
--- a/webrtc/base/network_unittest.cc
+++ b/webrtc/base/network_unittest.cc
@@ -58,6 +58,16 @@
callback_called_ = true;
}
+ void listenToNetworkInactive(BasicNetworkManager& network_manager) {
+ BasicNetworkManager::NetworkList networks;
+ network_manager.GetNetworks(&networks);
+ for (Network* network : networks) {
+ network->SignalInactive.connect(this, &NetworkTest::OnNetworkInactive);
+ }
+ }
+
+ void OnNetworkInactive(const Network* network) { num_networks_inactive_++; }
+
NetworkManager::Stats MergeNetworkList(
BasicNetworkManager& network_manager,
const NetworkManager::NetworkList& list,
@@ -147,6 +157,8 @@
protected:
bool callback_called_;
+ // Number of networks that become inactive.
+ int num_networks_inactive_ = 0;
};
class TestBasicNetworkManager : public BasicNetworkManager {
@@ -280,6 +292,7 @@
EXPECT_TRUE(changed);
EXPECT_EQ(stats.ipv6_network_count, 0);
EXPECT_EQ(stats.ipv4_network_count, 1);
+ listenToNetworkInactive(manager);
list.clear();
manager.GetNetworks(&list);
@@ -294,7 +307,9 @@
EXPECT_TRUE(changed);
EXPECT_EQ(stats.ipv6_network_count, 0);
EXPECT_EQ(stats.ipv4_network_count, 1);
+ EXPECT_EQ(1, num_networks_inactive_);
list.clear();
+ num_networks_inactive_ = 0;
manager.GetNetworks(&list);
EXPECT_EQ(1U, list.size());
@@ -309,6 +324,7 @@
EXPECT_TRUE(changed);
EXPECT_EQ(stats.ipv6_network_count, 0);
EXPECT_EQ(stats.ipv4_network_count, 2);
+ EXPECT_EQ(0, num_networks_inactive_);
list.clear();
// Verify that we get previous instances of Network objects.
@@ -326,6 +342,7 @@
EXPECT_FALSE(changed);
EXPECT_EQ(stats.ipv6_network_count, 0);
EXPECT_EQ(stats.ipv4_network_count, 2);
+ EXPECT_EQ(0, num_networks_inactive_);
list.clear();
// Verify that we get previous instances of Network objects.
diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc
index 87a50bc..74f1392 100644
--- a/webrtc/p2p/base/p2ptransportchannel.cc
+++ b/webrtc/p2p/base/p2ptransportchannel.cc
@@ -12,13 +12,15 @@
#include <algorithm>
#include <set>
-#include "webrtc/p2p/base/common.h"
-#include "webrtc/p2p/base/relayport.h" // For RELAY_PORT_TYPE.
-#include "webrtc/p2p/base/stunport.h" // For STUN_PORT_TYPE.
+
#include "webrtc/base/common.h"
#include "webrtc/base/crc32.h"
#include "webrtc/base/logging.h"
#include "webrtc/base/stringencode.h"
+#include "webrtc/p2p/base/candidate.h"
+#include "webrtc/p2p/base/common.h"
+#include "webrtc/p2p/base/relayport.h" // For RELAY_PORT_TYPE.
+#include "webrtc/p2p/base/stunport.h" // For STUN_PORT_TYPE.
#include "webrtc/system_wrappers/include/field_trial.h"
namespace {
@@ -464,6 +466,8 @@
port->SignalUnknownAddress.connect(
this, &P2PTransportChannel::OnUnknownAddress);
port->SignalDestroyed.connect(this, &P2PTransportChannel::OnPortDestroyed);
+ port->SignalNetworkInactive.connect(
+ this, &P2PTransportChannel::OnPortNetworkInactive);
port->SignalRoleConflict.connect(
this, &P2PTransportChannel::OnRoleConflict);
port->SignalSentPacket.connect(this, &P2PTransportChannel::OnSentPacket);
@@ -1415,6 +1419,23 @@
<< static_cast<int>(ports_.size()) << " remaining";
}
+void P2PTransportChannel::OnPortNetworkInactive(PortInterface* port) {
+ // If it does not gather continually, the port will be removed from the list
+ // when ICE restarts.
+ if (!gather_continually_) {
+ return;
+ }
+ auto it = std::find(ports_.begin(), ports_.end(), port);
+ // Don't need to do anything if the port has been deleted from the port list.
+ if (it == ports_.end()) {
+ return;
+ }
+ ports_.erase(it);
+ LOG(INFO) << "Removed port due to inactive networks: " << ports_.size()
+ << " remaining";
+ // TODO(honghaiz): Signal candidate removals to the remote side.
+}
+
// We data is available, let listeners know
void P2PTransportChannel::OnReadPacket(Connection* connection,
const char* data,
diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h
index f2e9315..fde0026 100644
--- a/webrtc/p2p/base/p2ptransportchannel.h
+++ b/webrtc/p2p/base/p2ptransportchannel.h
@@ -227,6 +227,7 @@
const std::string& remote_username,
bool port_muxed);
void OnPortDestroyed(PortInterface* port);
+ void OnPortNetworkInactive(PortInterface* port);
void OnRoleConflict(PortInterface* port);
void OnConnectionStateChange(Connection* connection);
diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
index ab17cfb..38f12fa 100644
--- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc
+++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
@@ -1770,6 +1770,37 @@
ep2_ch1()->GetState(), 1000);
}
+// Tests that when a network interface becomes inactive, the ports associated
+// with that network will be removed from the port list of the channel if
+// and only if Continual Gathering is enabled.
+TEST_F(P2PTransportChannelMultihomedTest, TestNetworkBecomesInactive) {
+ AddAddress(0, kPublicAddrs[0]);
+ AddAddress(1, kPublicAddrs[1]);
+ // Create channels and let them go writable, as usual.
+ CreateChannels(1);
+ ep1_ch1()->SetIceConfig(CreateIceConfig(2000, true));
+ ep2_ch1()->SetIceConfig(CreateIceConfig(2000, false));
+
+ SetAllocatorFlags(0, kOnlyLocalPorts);
+ SetAllocatorFlags(1, kOnlyLocalPorts);
+ EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+ ep2_ch1()->receiving() && ep2_ch1()->writable(),
+ 1000, 1000);
+ // More than one port has been created.
+ EXPECT_LE(1U, ep1_ch1()->ports().size());
+ // Endpoint 1 enabled continual gathering; the port will be removed
+ // when the interface is removed.
+ RemoveAddress(0, kPublicAddrs[0]);
+ EXPECT_EQ(0U, ep1_ch1()->ports().size());
+
+ size_t num_ports = ep2_ch1()->ports().size();
+ EXPECT_LE(1U, num_ports);
+ // Endpoint 2 did not enable continual gathering; the port will not be removed
+ // when the interface is removed.
+ RemoveAddress(1, kPublicAddrs[1]);
+ EXPECT_EQ(num_ports, ep2_ch1()->ports().size());
+}
+
/*
TODO(pthatcher): Once have a way to handle network interfaces changes
diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc
index 4d95279..ed26fe4 100644
--- a/webrtc/p2p/base/port.cc
+++ b/webrtc/p2p/base/port.cc
@@ -20,6 +20,7 @@
#include "webrtc/base/helpers.h"
#include "webrtc/base/logging.h"
#include "webrtc/base/messagedigest.h"
+#include "webrtc/base/network.h"
#include "webrtc/base/scoped_ptr.h"
#include "webrtc/base/stringencode.h"
#include "webrtc/base/stringutils.h"
@@ -195,6 +196,7 @@
ice_username_fragment_ = rtc::CreateRandomString(ICE_UFRAG_LENGTH);
password_ = rtc::CreateRandomString(ICE_PWD_LENGTH);
}
+ network_->SignalInactive.connect(this, &Port::OnNetworkInactive);
// TODO(honghaiz): Make it configurable from user setting.
network_cost_ =
(network_->type() == rtc::ADAPTER_TYPE_CELLULAR) ? kMaxNetworkCost : 0;
@@ -633,11 +635,16 @@
}
}
+void Port::OnNetworkInactive(const rtc::Network* network) {
+ ASSERT(network == network_);
+ SignalNetworkInactive(this);
+}
+
std::string Port::ToString() const {
std::stringstream ss;
- ss << "Port[" << content_name_ << ":" << component_
- << ":" << generation_ << ":" << type_
- << ":" << network_->ToString() << "]";
+ ss << "Port[" << std::hex << this << std::dec << ":" << content_name_ << ":"
+ << component_ << ":" << generation_ << ":" << type_ << ":"
+ << network_->ToString() << "]";
return ss.str();
}
diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h
index 95abe86..75b786f 100644
--- a/webrtc/p2p/base/port.h
+++ b/webrtc/p2p/base/port.h
@@ -358,6 +358,8 @@
return ice_role_ == ICEROLE_CONTROLLED && connections_.empty();
}
+ void OnNetworkInactive(const rtc::Network* network);
+
rtc::Thread* thread_;
rtc::PacketSocketFactory* factory_;
std::string type_;
diff --git a/webrtc/p2p/base/portinterface.h b/webrtc/p2p/base/portinterface.h
index e83879f..e738619 100644
--- a/webrtc/p2p/base/portinterface.h
+++ b/webrtc/p2p/base/portinterface.h
@@ -104,6 +104,9 @@
// any usefulness.
sigslot::signal1<PortInterface*> SignalDestroyed;
+ // Signaled when the network used by this port becomes inactive.
+ sigslot::signal1<PortInterface*> SignalNetworkInactive;
+
// Signaled when Port discovers ice role conflict with the peer.
sigslot::signal1<PortInterface*> SignalRoleConflict;
