Fixing the behavior of the candidate filter with pooled candidates.

webrtc/p2p/client/basicportallocator_unittest.cc

Index: webrtc/p2p/client/basicportallocator_unittest.cc
diff --git a/webrtc/p2p/client/basicportallocator_unittest.cc b/webrtc/p2p/client/basicportallocator_unittest.cc
index 83d904f181480deca88bc1701af4e7e451e6208c..4557b1664a4fd28b3813e1a1b6132263f15ddbf3 100644
--- a/webrtc/p2p/client/basicportallocator_unittest.cc
+++ b/webrtc/p2p/client/basicportallocator_unittest.cc
@@ -965,6 +965,21 @@ TEST_F(BasicPortAllocatorTest, TestGetAllPortsRestarts) {
   // TODO(deadbeef): Extend this to verify ICE restart.
 }
 
+// Test that the allocator session uses the candidate filter it's created with,
+// rather than the filter of its parent allocator.
+// The filter of the allocator should only affect the next gathering phase,
+// according to JSEP, which means the *next* allocator session returned.
+TEST_F(BasicPortAllocatorTest, TestSessionUsesOwnCandidateFilter) {
+  AddInterface(kClientAddr);
+  EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
+  // Set candidate filter *after* creating the session. Should have no effect.
+  allocator().set_candidate_filter(cricket::CF_RELAY);
+  session_->StartGettingPorts();
+  EXPECT_EQ_WAIT(7U, candidates_.size(), kDefaultAllocationTimeout);
+  EXPECT_EQ(4U, ports_.size());

[pthatcher1, 2016/05/20 01:23:54]

Can you make it clear that 7 candidates on 4 ports means !CF_RELAY?

[Taylor Brandstetter, 2016/05/20 21:03:37]

Done.

+  EXPECT_TRUE(candidate_allocation_done_);
+}
+
 // Test ICE candidate filter mechanism with options Relay/Host/Reflexive.
 // This test also verifies that when the allocator is only allowed to use
 // relay (i.e. IceTransportsType is relay), the raddr is an empty
@@ -1498,3 +1513,37 @@ TEST_F(BasicPortAllocatorTest, TestTransportInformationUpdated) {
     EXPECT_EQ(kIcePwd0, candidate.password());
   }
 }
+
+// Test that a new candidate filter takes effect even on already-gathered
+// candidates.
+TEST_F(BasicPortAllocatorTest, TestSetCandidateFilterAfterCandidatesGathered) {
+  AddInterface(kClientAddr);
+  int pool_size = 1;
+  allocator_->SetConfiguration(allocator_->stun_servers(),
+                               allocator_->turn_servers(), pool_size);
+  const cricket::PortAllocatorSession* peeked_session =
+      allocator_->GetPooledSession();
+  ASSERT_NE(nullptr, peeked_session);
+  EXPECT_EQ_WAIT(true, peeked_session->CandidatesAllocationDone(),
+                 kDefaultAllocationTimeout);
+  size_t initial_candidates_size = peeked_session->ReadyCandidates().size();
+  size_t initial_ports_size = peeked_session->ReadyPorts().size();
+  allocator_->set_candidate_filter(cricket::CF_RELAY);
+  // Assume that when TakePooledSession is called, the candidate filter will be
+  // applied to the pooled session. This is tested by PortAllocatorTest.
+  session_ =
+      allocator_->TakePooledSession(kContentName, 1, kIceUfrag0, kIcePwd0);
+  ASSERT_NE(nullptr, session_.get());
+  auto candidates = session_->ReadyCandidates();
+  // Check that the number of candidates and ports decreased.
+  EXPECT_GT(initial_candidates_size, candidates.size());
+  EXPECT_GT(initial_ports_size, session_->ReadyPorts().size());
+  for (size_t i = 0; i < candidates.size(); ++i) {

[pthatcher1, 2016/05/20 01:23:54]

c++11 for-loop?

[Taylor Brandstetter, 2016/05/20 21:03:37]

Done.

+    // Expect only relay candidates now that the filter is applied.
+    EXPECT_EQ(std::string(cricket::RELAY_PORT_TYPE), candidates[i].type());
+    // Expect that the raddr is emptied due to the CF_RELAY filter.
+    EXPECT_EQ(
+        candidates[0].related_address(),
+        rtc::EmptySocketAddressWithFamily(candidates[0].address().family()));
+  }
+}