Signal to remove remote candidates if ports are pruned.

webrtc/p2p/client/basicportallocator_unittest.cc

 /*
  *  Copyright 2009 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 225 matching lines @@
       const std::string& ice_ufrag,
       const std::string& ice_pwd) {
     std::unique_ptr<PortAllocatorSession> session =
         allocator_->CreateSession(content_name, component, ice_ufrag, ice_pwd);
     session->SignalPortReady.connect(this,
                                      &BasicPortAllocatorTest::OnPortReady);
     session->SignalPortsPruned.connect(this,
                                        &BasicPortAllocatorTest::OnPortsPruned);
     session->SignalCandidatesReady.connect(
         this, &BasicPortAllocatorTest::OnCandidatesReady);
+    session->SignalCandidatesRemoved.connect(
+        this, &BasicPortAllocatorTest::OnCandidatesRemoved);
     session->SignalCandidatesAllocationDone.connect(
         this, &BasicPortAllocatorTest::OnCandidatesAllocationDone);
     return session;
   }
 
   // Return true if the addresses are the same, or the port is 0 in |pattern|
   // (acting as a wildcard) and the IPs are the same.
   // Even with a wildcard port, the port of the address should be nonzero if
   // the IP is nonzero.
   static bool AddressMatch(const SocketAddress& address,
@@ skipping 141 matching lines @@
       EXPECT_PRED5(HasCandidateWithRelatedAddr, candidates_, "relay", "udp",
                    rtc::SocketAddress(relay_candidate_tcp_transport_addr, 0),
                    rtc::SocketAddress(stun_candidate_addr, 0));
       ++total_candidates;
     }
 
     EXPECT_EQ(total_candidates, candidates_.size());
     EXPECT_EQ(total_ports, ports_.size());
   }
 
+  rtc::VirtualSocketServer* virtual_socket_server() { return vss_.get(); }
+
  protected:
   BasicPortAllocator& allocator() { return *allocator_; }
 
   void OnPortReady(PortAllocatorSession* ses, PortInterface* port) {
     LOG(LS_INFO) << "OnPortReady: " << port->ToString();
     ports_.push_back(port);
     // Make sure the new port is added to ReadyPorts.
     auto ready_ports = ses->ReadyPorts();
     EXPECT_NE(ready_ports.end(),
               std::find(ready_ports.begin(), ready_ports.end(), port));
@@ skipping 22 matching lines @@
     }
     // Make sure the new candidates are added to Candidates.
     auto ses_candidates = ses->ReadyCandidates();
     for (const Candidate& candidate : candidates) {
       EXPECT_NE(
           ses_candidates.end(),
           std::find(ses_candidates.begin(), ses_candidates.end(), candidate));
     }
   }
 
+  void OnCandidatesRemoved(PortAllocatorSession* session,
+                           const std::vector<Candidate>& removed_candidates) {
+    auto new_end = std::remove_if(
+        candidates_.begin(), candidates_.end(),
+        [removed_candidates](Candidate& candidate) {
+          for (const Candidate& removed_candidate : removed_candidates) {
+            if (candidate.MatchesForRemoval(removed_candidate)) {
+              return true;
+            }
+          }
+          return false;
+        });
+    candidates_.erase(new_end, candidates_.end());
+  }
+
   bool HasRelayAddress(const ProtocolAddress& proto_addr) {
     for (size_t i = 0; i < allocator_->turn_servers().size(); ++i) {
       RelayServerConfig server_config = allocator_->turn_servers()[i];
       PortList::const_iterator relay_port;
       for (relay_port = server_config.ports.begin();
            relay_port != server_config.ports.end(); ++relay_port) {
         if (proto_addr.address == relay_port->address &&
             proto_addr.proto == relay_port->proto)
           return true;
       }
@@ skipping 748 matching lines @@
                rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
   EXPECT_PRED4(HasCandidate, candidates_, "relay", "udp",
                rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
   EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
   EXPECT_EQ(3U, candidates_.size());
 }
 
 // Test that if prune_turn_ports is set, TCP TurnPort will not
 // be used if UDP TurnPort is used.
 TEST_F(BasicPortAllocatorTest, TestUdpTurnPortPrunesTcpTurnPorts) {
+  // Using a large standard deviation for the transit delay so that TCP Turn
+  // ports may become ready sometimes before the UDP Turn port does,
+  // and sometimes after that.
+  // TODO(honghaiz): Change the virtual socket to tell if it is UDP or TCP, so
+  // that we can control the TCP delay independently of the UDP delay.

[Taylor Brandstetter, 2016/09/15 00:43:17]

Since IPv4 vs. IPv6 is already handled in this CL, can UDP vs. TCP be handled
too? I think we should avoid non-deterministic tests.

[honghaiz3, 2016/09/16 01:41:30]

Done.

+  virtual_socket_server()->set_delay_mean(50);
+  virtual_socket_server()->set_delay_stddev(200);
+  virtual_socket_server()->UpdateDelayDistribution();
+
   turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP);
   AddInterface(kClientAddr);
   allocator_.reset(new BasicPortAllocator(&network_manager_));
   allocator_->SetConfiguration(allocator_->stun_servers(),
                                allocator_->turn_servers(), 0, true);
   AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr);
   allocator_->set_step_delay(kMinimumStepDelay);
   allocator_->set_flags(allocator().flags() |
                         PORTALLOCATOR_ENABLE_SHARED_SOCKET |
                         PORTALLOCATOR_DISABLE_TCP);
 
   EXPECT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
   session_->StartGettingPorts();
-  EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
+  EXPECT_TRUE_WAIT(candidate_allocation_done_, 10000);

[Taylor Brandstetter, 2016/09/15 00:43:17]

nit: May want to use a constant for this timeout. Could even just increase the
default timeout to 10 seconds (I've done that in other places; it reduces
msan/tsan/etc flakes).

[honghaiz3, 2016/09/16 01:41:30]

Done. Now that there is no randomness, I don't think it needs 10 seconds. But
still make it larger to be 3 seconds. If we still see flakiness, we can increase
to 10seconds. 

   // Only 2 ports (one STUN and one TURN) are actually being used.
   EXPECT_EQ(2U, session_->ReadyPorts().size());
   // We have verified that each port, when it is added to |ports_|, it is found
   // in |ready_ports|, and when it is pruned, it is not found in |ready_ports|,
   // so we only need to verify the content in one of them.
   EXPECT_EQ(2U, ports_.size());
   EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientAddr));
   EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_UDP, kClientAddr));
   EXPECT_EQ(0, CountPorts(ports_, "relay", PROTO_TCP, kClientAddr));
 
-  // We don't remove candidates, so the size of |candidates_| will depend on
-  // when the TCP TURN port becomes ready. If it is ready after the UDP TURN
-  // port becomes ready, its candidates will be used there will be 3 candidates.
-  // Otherwise there will be only 2 candidates.
-  EXPECT_LE(2U, candidates_.size());
+  // Now that we remove candidates when a Turn port is pruned, |candidates_|
+  // should only contains two candidates regardless whether the TCP Turn port is
+  // created before or after the UDP turn port.

[Taylor Brandstetter, 2016/09/15 00:43:17]

nit: Capitalize "TURN"

[honghaiz3, 2016/09/16 01:41:30]

Done.

+  EXPECT_EQ(2U, candidates_.size());
   // There will only be 2 candidates in |ready_candidates| because it only
   // includes the candidates in the ready ports.
   const std::vector<Candidate>& ready_candidates = session_->ReadyCandidates();
   EXPECT_EQ(2U, ready_candidates.size());
   EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientAddr);
   EXPECT_PRED4(HasCandidate, ready_candidates, "relay", "udp",
                rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
 }
 
 // Tests that if prune_turn_ports is set, IPv4 TurnPort will not
 // be used if IPv6 TurnPort is used.
 TEST_F(BasicPortAllocatorTest, TestIPv6TurnPortPrunesIPv4TurnPorts) {
+  // Make the mean IPv6 delay longer than the mean IPv4 delay so that it is
+  // more likely that the IPv4 candidates are initially generated and then
+  // removed.
+  virtual_socket_server()->set_delay_stddev(100);

[Taylor Brandstetter, 2016/09/15 00:43:17]

Since you can control the delay for IPv6 vs. IPv4 directly, why is variation
needed?

[honghaiz3, 2016/09/16 01:41:30]

I was hoping to use a random value to test behaviors under different realistic
scenarios. 
vlad made a good argument on using a deterministic value and you also want a
deterministic value. So removed it now with the new approach of setting delays

+  virtual_socket_server()->set_delay_mean(100);
+  virtual_socket_server()->UpdateDelayDistribution();
+  virtual_socket_server()->set_delay_mean(200);
+  virtual_socket_server()->UpdateDelayDistributionForIPv6();
+
   turn_server_.AddInternalSocket(kTurnUdpIntIPv6Addr, PROTO_UDP);
   // Add two IP addresses on the same interface.
   AddInterface(kClientAddr, "net1");
   AddInterface(kClientIPv6Addr, "net1");
   allocator_.reset(new BasicPortAllocator(&network_manager_));
   allocator_->SetConfiguration(allocator_->stun_servers(),
                                allocator_->turn_servers(), 0, true);
   AddTurnServers(kTurnUdpIntIPv6Addr, rtc::SocketAddress());
+  AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
 
   allocator_->set_step_delay(kMinimumStepDelay);
   allocator_->set_flags(allocator().flags() |
                         PORTALLOCATOR_ENABLE_SHARED_SOCKET |
                         PORTALLOCATOR_ENABLE_IPV6 | PORTALLOCATOR_DISABLE_TCP);
 
   EXPECT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
   session_->StartGettingPorts();
-  EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
+  EXPECT_TRUE_WAIT(candidate_allocation_done_, 10000);
   // Three ports (one IPv4 STUN, one IPv6 STUN and one TURN) will be ready.
   EXPECT_EQ(3U, session_->ReadyPorts().size());
   EXPECT_EQ(3U, ports_.size());
   EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientAddr));
   EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientIPv6Addr));
   EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_UDP, kClientIPv6Addr));
   EXPECT_EQ(0, CountPorts(ports_, "relay", PROTO_UDP, kClientAddr));
 
-  // We don't remove candidates, so there may be more than 3 elemenets in
-  // |candidates_|, although |ready_candidates| only includes the candidates
-  // in |ready_ports|.
-  EXPECT_LE(3U, candidates_.size());
+  // Now that we remove candidates when a Turn port is pruned, there will be
+  // exactly 3 candidates in both |candidates_| and |ready_candidates|.
+  EXPECT_EQ(3U, candidates_.size());
   const std::vector<Candidate>& ready_candidates = session_->ReadyCandidates();
   EXPECT_EQ(3U, ready_candidates.size());
   EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientAddr);
   EXPECT_PRED4(HasCandidate, ready_candidates, "relay", "udp",
                rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
 }
 
 // Tests that if prune_turn_ports is set, each network interface
 // will has its own set of TurnPorts based on their priorities.
 TEST_F(BasicPortAllocatorTest, TestEachInterfaceHasItsOwnTurnPorts) {
+  // Make IPv6 sockets have longer delay than IPv4 sockets so that it has
+  // higher chance that IPv4 candidates are generated and then removed.
+  virtual_socket_server()->set_delay_stddev(100);
+  virtual_socket_server()->set_delay_mean(100);
+  virtual_socket_server()->UpdateDelayDistribution();
+  virtual_socket_server()->set_delay_mean(200);
+  virtual_socket_server()->UpdateDelayDistributionForIPv6();
+
   turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP);
   turn_server_.AddInternalSocket(kTurnUdpIntIPv6Addr, PROTO_UDP);
   turn_server_.AddInternalSocket(kTurnTcpIntIPv6Addr, PROTO_TCP);
   // Add two interfaces both having IPv4 and IPv6 addresses.
   AddInterface(kClientAddr, "net1", rtc::ADAPTER_TYPE_WIFI);
   AddInterface(kClientIPv6Addr, "net1", rtc::ADAPTER_TYPE_WIFI);
   AddInterface(kClientAddr2, "net2", rtc::ADAPTER_TYPE_CELLULAR);
   AddInterface(kClientIPv6Addr2, "net2", rtc::ADAPTER_TYPE_CELLULAR);
   allocator_.reset(new BasicPortAllocator(&network_manager_));
   allocator_->SetConfiguration(allocator_->stun_servers(),
                                allocator_->turn_servers(), 0, true);
   // Have both UDP/TCP and IPv4/IPv6 TURN ports.
   AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr);
   AddTurnServers(kTurnUdpIntIPv6Addr, kTurnTcpIntIPv6Addr);
 
   allocator_->set_step_delay(kMinimumStepDelay);
   allocator_->set_flags(allocator().flags() |
                         PORTALLOCATOR_ENABLE_SHARED_SOCKET |
                         PORTALLOCATOR_ENABLE_IPV6);
   EXPECT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
   session_->StartGettingPorts();
-  EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
+  EXPECT_TRUE_WAIT(candidate_allocation_done_, 10000);
   // 10 ports (4 STUN and 1 TURN ports on each interface) will be ready to use.
   EXPECT_EQ(10U, session_->ReadyPorts().size());
   EXPECT_EQ(10U, ports_.size());
   EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientAddr));
   EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientAddr2));
   EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientIPv6Addr));
   EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientIPv6Addr2));
   EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kClientAddr));
   EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kClientAddr2));
   EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kClientIPv6Addr));
   EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kClientIPv6Addr2));
   EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_UDP, kClientIPv6Addr));
   EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_UDP, kClientIPv6Addr2));
 
-  // We don't remove candidates, so there may be more than 10 candidates
-  // in |candidates_|.
-  EXPECT_LE(10U, candidates_.size());
+  // Now that we remove candidates when Turn ports are pruned, there will be
+  // exactly 10 candidates in |candidates_|.
+  EXPECT_EQ(10U, candidates_.size());
   const std::vector<Candidate>& ready_candidates = session_->ReadyCandidates();
   EXPECT_EQ(10U, ready_candidates.size());
   EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientAddr);
   EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientAddr2);
   EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientIPv6Addr);
   EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp",
                kClientIPv6Addr2);
   EXPECT_PRED4(HasCandidate, ready_candidates, "local", "tcp", kClientAddr);
   EXPECT_PRED4(HasCandidate, ready_candidates, "local", "tcp", kClientAddr2);
   EXPECT_PRED4(HasCandidate, ready_candidates, "local", "tcp", kClientIPv6Addr);
@@ skipping 361 matching lines @@
   for (const Candidate& candidate : candidates) {
     // Expect only relay candidates now that the filter is applied.
     EXPECT_EQ(std::string(RELAY_PORT_TYPE), candidate.type());
     // Expect that the raddr is emptied due to the CF_RELAY filter.
     EXPECT_EQ(candidate.related_address(),
               rtc::EmptySocketAddressWithFamily(candidate.address().family()));
   }
 }
 
 }  // namespace cricket