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}

webrtc/base/network.cc

@@ -317,10 +317,11 @@ void NetworkManagerBase::MergeNetworkList(const NetworkList& new_networks,
     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

webrtc/base/network.h

@@ -265,6 +265,8 @@ class Network {
           AdapterType type);
   ~Network();
 
+  sigslot::signal1<const Network*> SignalInactive;
+
   const DefaultLocalAddressProvider* default_local_address_provider() {
     return default_local_address_provider_;
   }
@@ -342,7 +344,15 @@ class Network {
   // 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;

webrtc/base/network_unittest.cc

@@ -58,6 +58,16 @@ class NetworkTest : public testing::Test, public sigslot::has_slots<>  {
     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 @@ class NetworkTest : public testing::Test, public sigslot::has_slots<>  {
 
  protected:
   bool callback_called_;
+  // Number of networks that become inactive.
+  int num_networks_inactive_ = 0;
 };
 
 class TestBasicNetworkManager : public BasicNetworkManager {
@@ -280,6 +292,7 @@ TEST_F(NetworkTest, TestBasicMergeNetworkList) {
   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 @@ TEST_F(NetworkTest, TestBasicMergeNetworkList) {
   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 @@ TEST_F(NetworkTest, TestBasicMergeNetworkList) {
   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 @@ TEST_F(NetworkTest, TestBasicMergeNetworkList) {
   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.

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 @@ void P2PTransportChannel::OnPortReady(PortAllocatorSession *session,
   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 @@ void P2PTransportChannel::OnPortDestroyed(PortInterface* port) {
             << 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,

webrtc/p2p/base/p2ptransportchannel.h

@@ -227,6 +227,7 @@ class P2PTransportChannel : public TransportChannelImpl,
                         const std::string& remote_username,
                         bool port_muxed);
   void OnPortDestroyed(PortInterface* port);
+  void OnPortNetworkInactive(PortInterface* port);
   void OnRoleConflict(PortInterface* port);
 
   void OnConnectionStateChange(Connection* connection);

webrtc/p2p/base/p2ptransportchannel_unittest.cc

@@ -1770,6 +1770,37 @@ TEST_F(P2PTransportChannelMultihomedTest, TestGetState) {
                  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

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 @@ void Port::Construct() {
     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::OnMessage(rtc::Message *pmsg) {
   }
 }
 
+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();
 }
 

webrtc/p2p/base/port.h

@@ -358,6 +358,8 @@ class Port : public PortInterface, public rtc::MessageHandler,
     return ice_role_ == ICEROLE_CONTROLLED && connections_.empty();
   }
 
+  void OnNetworkInactive(const rtc::Network* network);
+
   rtc::Thread* thread_;
   rtc::PacketSocketFactory* factory_;
   std::string type_;

webrtc/p2p/base/portinterface.h

@@ -104,6 +104,9 @@ class PortInterface {
   // 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;