When doing DisableEquivalentPhases, exclude those AllocationSequences

whose network has ever been removed. It is unlikely the sockets/ports/candidates created from
those AllocationSequences will still be valid.

BUG=

Review URL: https://codereview.webrtc.org/1361183004

Cr-Commit-Position: refs/heads/master@{#10093}

webrtc/base/fakenetwork.h

@@ -34,8 +34,12 @@ class FakeNetworkManager : public NetworkManagerBase,
   typedef std::vector<SocketAddress> IfaceList;
 
   void AddInterface(const SocketAddress& iface) {
-    // ensure a unique name for the interface
-    SocketAddress address("test" + rtc::ToString(next_index_++), 0);
+    // Ensure a unique name for the interface if its name is not given.
+    AddInterface(iface, "test" + rtc::ToString(next_index_++));
+  }
+
+  void AddInterface(const SocketAddress& iface, const std::string& if_name) {
+    SocketAddress address(if_name, 0);
     address.SetResolvedIP(iface.ipaddr());
     ifaces_.push_back(address);
     DoUpdateNetworks();

webrtc/p2p/client/basicportallocator.cc

@@ -298,28 +298,35 @@ void BasicPortAllocatorSession::OnAllocate() {
   allocation_started_ = true;
 }
 
-// For each network, see if we have a sequence that covers it already.  If not,
-// create a new sequence to create the appropriate ports.
-void BasicPortAllocatorSession::DoAllocate() {
-  bool done_signal_needed = false;
-  std::vector<rtc::Network*> networks;
-
+void BasicPortAllocatorSession::GetNetworks(
+    std::vector<rtc::Network*>* networks) {
+  networks->clear();
+  rtc::NetworkManager* network_manager = allocator_->network_manager();
+  ASSERT(network_manager != nullptr);
   // If the network permission state is BLOCKED, we just act as if the flag has
   // been passed in.
-  if (allocator_->network_manager()->enumeration_permission() ==
+  if (network_manager->enumeration_permission() ==
       rtc::NetworkManager::ENUMERATION_BLOCKED) {
     set_flags(flags() | PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION);
   }
-
   // If the adapter enumeration is disabled, we'll just bind to any address
   // instead of specific NIC. This is to ensure the same routing for http
   // traffic by OS is also used here to avoid any local or public IP leakage
   // during stun process.
   if (flags() & PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION) {
-    allocator_->network_manager()->GetAnyAddressNetworks(&networks);
+    network_manager->GetAnyAddressNetworks(networks);
   } else {
-    allocator_->network_manager()->GetNetworks(&networks);
+    network_manager->GetNetworks(networks);
   }
+}
+
+// For each network, see if we have a sequence that covers it already.  If not,
+// create a new sequence to create the appropriate ports.
+void BasicPortAllocatorSession::DoAllocate() {
+  bool done_signal_needed = false;
+  std::vector<rtc::Network*> networks;
+  GetNetworks(&networks);
+
   if (networks.empty()) {
     LOG(LS_WARNING) << "Machine has no networks; no ports will be allocated";
     done_signal_needed = true;
@@ -377,6 +384,18 @@ void BasicPortAllocatorSession::DoAllocate() {
 }
 
 void BasicPortAllocatorSession::OnNetworksChanged() {
+  std::vector<rtc::Network*> networks;
+  GetNetworks(&networks);
+  for (AllocationSequence* sequence : sequences_) {
+    // Remove the network from the allocation sequence if it is not in
+    // |networks|.
+    if (!sequence->network_removed() &&
+        std::find(networks.begin(), networks.end(), sequence->network()) ==
+            networks.end()) {
+      sequence->OnNetworkRemoved();
+    }
+  }
+
   network_manager_started_ = true;
   if (allocation_started_)
     DoAllocate();
@@ -711,12 +730,24 @@ void AllocationSequence::Clear() {
   turn_ports_.clear();
 }
 
+void AllocationSequence::OnNetworkRemoved() {
+  // Stop the allocation sequence if its network is gone.
+  Stop();
+  network_removed_ = true;
+}
+
 AllocationSequence::~AllocationSequence() {
   session_->network_thread()->Clear(this);
 }
 
 void AllocationSequence::DisableEquivalentPhases(rtc::Network* network,
     PortConfiguration* config, uint32* flags) {
+  if (network_removed_) {
+    // If the network of this allocation sequence has ever gone away,
+    // it won't be equivalent to the new network.
+    return;
+  }
+
   if (!((network == network_) && (ip_ == network->GetBestIP()))) {
     // Different network setup; nothing is equivalent.
     return;

webrtc/p2p/client/basicportallocator.h

@@ -182,6 +182,7 @@ class BasicPortAllocatorSession : public PortAllocatorSession,
   void MaybeSignalCandidatesAllocationDone();
   void OnPortAllocationComplete(AllocationSequence* seq);
   PortData* FindPort(Port* port);
+  void GetNetworks(std::vector<rtc::Network*>* networks);
 
   bool CheckCandidateFilter(const Candidate& c);
 
@@ -260,8 +261,11 @@ class AllocationSequence : public rtc::MessageHandler,
   ~AllocationSequence();
   bool Init();
   void Clear();
+  void OnNetworkRemoved();
 
   State state() const { return state_; }
+  const rtc::Network* network() const { return network_; }
+  bool network_removed() const { return network_removed_; }
 
   // Disables the phases for a new sequence that this one already covers for an
   // equivalent network setup.
@@ -311,6 +315,7 @@ class AllocationSequence : public rtc::MessageHandler,
   void OnPortDestroyed(PortInterface* port);
 
   BasicPortAllocatorSession* session_;
+  bool network_removed_ = false;
   rtc::Network* network_;
   rtc::IPAddress ip_;
   PortConfiguration* config_;

webrtc/p2p/client/portallocator_unittest.cc

@@ -111,12 +111,18 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> {
   void AddInterface(const SocketAddress& addr) {
     network_manager_.AddInterface(addr);
   }
+  void AddInterface(const SocketAddress& addr, const std::string& if_name) {
+    network_manager_.AddInterface(addr, if_name);
+  }
   void AddInterfaceAsDefaultRoute(const SocketAddress& addr) {
     AddInterface(addr);
     // When a binding comes from the any address, the |addr| will be used as the
     // srflx address.
     vss_->SetDefaultRoute(addr.ipaddr());
   }
+  void RemoveInterface(const SocketAddress& addr) {
+    network_manager_.RemoveInterface(addr);
+  }
   bool SetPortRange(int min_port, int max_port) {
     return allocator_->SetPortRange(min_port, max_port);
   }
@@ -432,6 +438,61 @@ TEST_F(PortAllocatorTest, TestGetAllPortsWithMinimumStepDelay) {
   EXPECT_TRUE(candidate_allocation_done_);
 }
 
+// Test that when the same network interface is brought down and up, the
+// port allocator session will restart a new allocation sequence if
+// it is not stopped.
+TEST_F(PortAllocatorTest, TestSameNetworkDownAndUpWhenSessionNotStopped) {
+  std::string if_name("test_net0");
+  AddInterface(kClientAddr, if_name);
+  EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
+  session_->StartGettingPorts();
+  ASSERT_EQ_WAIT(7U, candidates_.size(), kDefaultAllocationTimeout);
+  EXPECT_EQ(4U, ports_.size());
+  EXPECT_TRUE(candidate_allocation_done_);
+  candidate_allocation_done_ = false;
+  candidates_.clear();
+  ports_.clear();
+
+  RemoveInterface(kClientAddr);
+  ASSERT_EQ_WAIT(0U, candidates_.size(), kDefaultAllocationTimeout);
+  EXPECT_EQ(0U, ports_.size());
+  EXPECT_FALSE(candidate_allocation_done_);
+
+  // When the same interfaces are added again, new candidates/ports should be
+  // generated.
+  AddInterface(kClientAddr, if_name);
+  ASSERT_EQ_WAIT(7U, candidates_.size(), kDefaultAllocationTimeout);
+  EXPECT_EQ(4U, ports_.size());
+  EXPECT_TRUE(candidate_allocation_done_);
+}
+
+// Test that when the same network interface is brought down and up, the
+// port allocator session will not restart a new allocation sequence if
+// it is stopped.
+TEST_F(PortAllocatorTest, TestSameNetworkDownAndUpWhenSessionStopped) {
+  std::string if_name("test_net0");
+  AddInterface(kClientAddr, if_name);
+  EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
+  session_->StartGettingPorts();
+  ASSERT_EQ_WAIT(7U, candidates_.size(), kDefaultAllocationTimeout);
+  EXPECT_EQ(4U, ports_.size());
+  EXPECT_TRUE(candidate_allocation_done_);
+  session_->StopGettingPorts();
+  candidates_.clear();
+  ports_.clear();
+
+  RemoveInterface(kClientAddr);
+  ASSERT_EQ_WAIT(0U, candidates_.size(), kDefaultAllocationTimeout);
+  EXPECT_EQ(0U, ports_.size());
+
+  // When the same interfaces are added again, new candidates/ports should not
+  // be generated because the session has stopped.
+  AddInterface(kClientAddr, if_name);
+  ASSERT_EQ_WAIT(0U, candidates_.size(), kDefaultAllocationTimeout);
+  EXPECT_EQ(0U, ports_.size());
+  EXPECT_TRUE(candidate_allocation_done_);
+}
+
 // Verify candidates with default step delay of 1sec.
 TEST_F(PortAllocatorTest, TestGetAllPortsWithOneSecondStepDelay) {
   AddInterface(kClientAddr);