When doing DisableEquivalentPhases, exclude those AllocationSequences

webrtc/p2p/client/basicportallocator.cc

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 280 matching lines @@
   network_thread_->Post(this, MSG_ALLOCATE);
 }
 
 void BasicPortAllocatorSession::OnAllocate() {
   if (network_manager_started_)
     DoAllocate();
 
   allocation_started_ = true;
 }
 
+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 (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) {
+    network_manager->GetAnyAddressNetworks(networks);
+  } else {
+    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 the network permission state is BLOCKED, we just act as if the flag has
-  // been passed in.
-  if (allocator_->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);
-  } else {
-    allocator_->network_manager()->GetNetworks(&networks);
-  }
   if (networks.empty()) {
     LOG(LS_WARNING) << "Machine has no networks; no ports will be allocated";
     done_signal_needed = true;
   } else {
     for (uint32 i = 0; i < networks.size(); ++i) {
       PortConfiguration* config = NULL;
       if (configs_.size() > 0)
         config = configs_.back();
 
       uint32 sequence_flags = flags();
@@ skipping 37 matching lines @@
         sequence->Start();
       sequences_.push_back(sequence);
     }
   }
   if (done_signal_needed) {
     network_thread_->Post(this, MSG_SEQUENCEOBJECTS_CREATED);
   }
 }
 
 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() != nullptr &&
+        std::find(networks.begin(), networks.end(), sequence->network()) ==
+            networks.end()) {
+      sequence->RemoveNetwork();
+    }

[pthatcher1, 2015/09/24 21:41:43]

Wouldn't this be the same as:

if (sequence->state() != kStopped && 
    std::find(networks.begin(), networks.end(), sequence->network()) ==
              networks.end()) {
  sequence->Stop();
}

?

[honghaiz3, 2015/09/24 22:41:28]

Not exactly as we discussed. 

+  }
+
   network_manager_started_ = true;
   if (allocation_started_)
     DoAllocate();
 }
 
 void BasicPortAllocatorSession::DisableEquivalentPhases(
     rtc::Network* network, PortConfiguration* config, uint32* flags) {
   for (uint32 i = 0; i < sequences_.size() &&
       (*flags & DISABLE_ALL_PHASES) != DISABLE_ALL_PHASES; ++i) {
     sequences_[i]->DisableEquivalentPhases(network, config, flags);
@@ skipping 314 matching lines @@
     // are next available options to setup a communication channel.
   }
   return true;
 }
 
 void AllocationSequence::Clear() {
   udp_port_ = NULL;
   turn_ports_.clear();
 }
 
+void AllocationSequence::RemoveNetwork() {
+  // Stop the allocation sequence if its network is gone.
+  Stop();
+  network_ = nullptr;

[pthatcher1, 2015/09/24 21:41:43]

There's lot of code in this file that expects network_ not not be null.   Will
none of that ever run?  If not, what's the point of setting network_ = null?

[honghaiz3, 2015/09/24 22:41:28]

Changed to use a binary variable. 

+}
+
 AllocationSequence::~AllocationSequence() {
   session_->network_thread()->Clear(this);
 }
 
 void AllocationSequence::DisableEquivalentPhases(rtc::Network* network,
     PortConfiguration* config, uint32* flags) {
   if (!((network == network_) && (ip_ == network->GetBestIP()))) {
     // Different network setup; nothing is equivalent.
     return;
   }
@@ skipping 35 matching lines @@
 
 void AllocationSequence::OnMessage(rtc::Message* msg) {
   ASSERT(rtc::Thread::Current() == session_->network_thread());
   ASSERT(msg->message_id == MSG_ALLOCATION_PHASE);
 
   const char* const PHASE_NAMES[kNumPhases] = {
     "Udp", "Relay", "Tcp", "SslTcp"
   };
 
   // Perform all of the phases in the current step.
+  ASSERT(network_ != nullptr);
   LOG_J(LS_INFO, network_) << "Allocation Phase="
                            << PHASE_NAMES[phase_];
 
   switch (phase_) {
     case PHASE_UDP:
       CreateUDPPorts();
       CreateStunPorts();
       EnableProtocol(PROTO_UDP);
       break;
 
@@ skipping 361 matching lines @@
   ServerAddresses servers;
   for (size_t i = 0; i < relays.size(); ++i) {
     if (relays[i].type == turn_type && SupportsProtocol(relays[i], type)) {
       servers.insert(relays[i].ports.front().address);
     }
   }
   return servers;
 }
 
 }  // namespace cricket