Relanding: Implement RTCConfiguration.iceCandidatePoolSize.

webrtc/p2p/base/p2ptransportchannel_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.
  */
 
+#include <algorithm>
 #include <memory>
 
+#include "webrtc/p2p/base/fakeportallocator.h"
 #include "webrtc/p2p/base/p2ptransportchannel.h"
 #include "webrtc/p2p/base/testrelayserver.h"
 #include "webrtc/p2p/base/teststunserver.h"
 #include "webrtc/p2p/base/testturnserver.h"
 #include "webrtc/p2p/client/basicportallocator.h"
-#include "webrtc/p2p/client/fakeportallocator.h"
 #include "webrtc/base/dscp.h"
 #include "webrtc/base/fakenetwork.h"
 #include "webrtc/base/firewallsocketserver.h"
 #include "webrtc/base/gunit.h"
 #include "webrtc/base/helpers.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/natserver.h"
 #include "webrtc/base/natsocketfactory.h"
 #include "webrtc/base/physicalsocketserver.h"
 #include "webrtc/base/proxyserver.h"
@@ skipping 251 matching lines @@
     std::string ice_ufrag_ep2_cd1_ch = kIceUfrag[1];
     std::string ice_pwd_ep2_cd1_ch = kIcePwd[1];
     ep1_.cd1_.ch_.reset(CreateChannel(
         0, cricket::ICE_CANDIDATE_COMPONENT_DEFAULT,
         ice_ufrag_ep1_cd1_ch, ice_pwd_ep1_cd1_ch,
         ice_ufrag_ep2_cd1_ch, ice_pwd_ep2_cd1_ch));
     ep2_.cd1_.ch_.reset(CreateChannel(
         1, cricket::ICE_CANDIDATE_COMPONENT_DEFAULT,
         ice_ufrag_ep2_cd1_ch, ice_pwd_ep2_cd1_ch,
         ice_ufrag_ep1_cd1_ch, ice_pwd_ep1_cd1_ch));
+    ep1_.cd1_.ch_->MaybeStartGathering();
+    ep2_.cd1_.ch_->MaybeStartGathering();
     if (num == 2) {
       std::string ice_ufrag_ep1_cd2_ch = kIceUfrag[2];
       std::string ice_pwd_ep1_cd2_ch = kIcePwd[2];
       std::string ice_ufrag_ep2_cd2_ch = kIceUfrag[3];
       std::string ice_pwd_ep2_cd2_ch = kIcePwd[3];
       ep1_.cd2_.ch_.reset(CreateChannel(
           0, cricket::ICE_CANDIDATE_COMPONENT_DEFAULT,
           ice_ufrag_ep1_cd2_ch, ice_pwd_ep1_cd2_ch,
           ice_ufrag_ep2_cd2_ch, ice_pwd_ep2_cd2_ch));
       ep2_.cd2_.ch_.reset(CreateChannel(
           1, cricket::ICE_CANDIDATE_COMPONENT_DEFAULT,
           ice_ufrag_ep2_cd2_ch, ice_pwd_ep2_cd2_ch,
           ice_ufrag_ep1_cd2_ch, ice_pwd_ep1_cd2_ch));
+      ep1_.cd2_.ch_->MaybeStartGathering();
+      ep2_.cd2_.ch_->MaybeStartGathering();
     }
   }
   cricket::P2PTransportChannel* CreateChannel(
       int endpoint,
       int component,
       const std::string& local_ice_ufrag,
       const std::string& local_ice_pwd,
       const std::string& remote_ice_ufrag,
       const std::string& remote_ice_pwd) {
     cricket::P2PTransportChannel* channel = new cricket::P2PTransportChannel(
         "test content name", component, GetAllocator(endpoint));
     channel->SignalCandidateGathered.connect(
         this, &P2PTransportChannelTestBase::OnCandidateGathered);
     channel->SignalCandidatesRemoved.connect(
         this, &P2PTransportChannelTestBase::OnCandidatesRemoved);
     channel->SignalReadPacket.connect(
         this, &P2PTransportChannelTestBase::OnReadPacket);
     channel->SignalRoleConflict.connect(
         this, &P2PTransportChannelTestBase::OnRoleConflict);
     channel->SetIceCredentials(local_ice_ufrag, local_ice_pwd);
     if (clear_remote_candidates_ufrag_pwd_) {
       // This only needs to be set if we're clearing them from the
       // candidates.  Some unit tests rely on this not being set.
       channel->SetRemoteIceCredentials(remote_ice_ufrag, remote_ice_pwd);
     }
     channel->SetIceRole(GetEndpoint(endpoint)->ice_role());
     channel->SetIceTiebreaker(GetEndpoint(endpoint)->GetIceTiebreaker());
     channel->Connect();
-    channel->MaybeStartGathering();

[Taylor Brandstetter, 2016/05/05 19:56:23]

This needed to be moved because when using a pooled session, the
P2PTransportChannel can signal candidates synchronously inside of
MaybeStartGathering.

[pthatcher1, 2016/05/05 21:51:25]

Should we change that by having MaybeStartGathering do a thread post?

[Taylor Brandstetter, 2016/05/06 03:53:34]

No; the TransportController already posts candidates asynchronously, so there's
no issue with candidates being emitted before the SLD success callback. This is
only an issue with unit test implementation.

     return channel;
   }
   void DestroyChannels() {
     ep1_.cd1_.ch_.reset();
     ep2_.cd1_.ch_.reset();
     ep1_.cd2_.ch_.reset();
     ep2_.cd2_.ch_.reset();
   }
   cricket::P2PTransportChannel* ep1_ch1() { return ep1_.cd1_.ch_.get(); }
   cricket::P2PTransportChannel* ep1_ch2() { return ep1_.cd2_.ch_.get(); }
@@ skipping 1198 matching lines @@
        1000);
   EXPECT_EQ(cricket::IceGatheringState::kIceGatheringGathering,
             ep1_ch1()->gathering_state());
   // By now, ep2 should have completed gathering.
   EXPECT_EQ(cricket::IceGatheringState::kIceGatheringComplete,
             ep2_ch1()->gathering_state());
 
   DestroyChannels();
 }
 
+// Test that a connection succeeds when the P2PTransportChannel uses a pooled
+// PortAllocatorSession that has not yet finished gathering candidates.
+TEST_F(P2PTransportChannelTest, TestUsingPooledSessionBeforeDoneGathering) {
+  ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags,
+                     kDefaultPortAllocatorFlags);
+  // First create a pooled session for each endpoint.
+  auto& allocator_1 = GetEndpoint(0)->allocator_;
+  auto& allocator_2 = GetEndpoint(1)->allocator_;
+  allocator_1->SetConfiguration(allocator_1->stun_servers(),
+                                allocator_1->turn_servers(), 1);
+  allocator_2->SetConfiguration(allocator_2->stun_servers(),
+                                allocator_2->turn_servers(), 1);
+  const cricket::PortAllocatorSession* pooled_session_1 =
+      allocator_1->PeekPooledSession();
+  const cricket::PortAllocatorSession* pooled_session_2 =
+      allocator_2->PeekPooledSession();
+  ASSERT_NE(nullptr, pooled_session_1);
+  ASSERT_NE(nullptr, pooled_session_2);
+  // Sanity check that pooled sessions haven't gathered anything yet.
+  EXPECT_TRUE(pooled_session_1->ReadyPorts().empty());
+  EXPECT_TRUE(pooled_session_1->ReadyCandidates().empty());
+  EXPECT_TRUE(pooled_session_2->ReadyPorts().empty());
+  EXPECT_TRUE(pooled_session_2->ReadyCandidates().empty());
+  // Now let the endpoints connect and try exchanging some data.
+  CreateChannels(1);
+  EXPECT_TRUE_WAIT_MARGIN(ep1_ch1() != NULL && ep2_ch1() != NULL &&
+                              ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+                              ep2_ch1()->receiving() && ep2_ch1()->writable(),
+                          1000, 1000);
+  TestSendRecv(1);
+  // Make sure the P2PTransportChannels are actually using ports from the
+  // pooled sessions.
+  auto pooled_ports_1 = pooled_session_1->ReadyPorts();
+  auto pooled_ports_2 = pooled_session_2->ReadyPorts();
+  EXPECT_NE(pooled_ports_1.end(),
+            std::find(pooled_ports_1.begin(), pooled_ports_1.end(),
+                      ep1_ch1()->best_connection()->port()));
+  EXPECT_NE(pooled_ports_2.end(),
+            std::find(pooled_ports_2.begin(), pooled_ports_2.end(),
+                      ep2_ch1()->best_connection()->port()));
+}
+
+// Test that a connection succeeds when the P2PTransportChannel uses a pooled
+// PortAllocatorSession that already finished gathering candidates.
+TEST_F(P2PTransportChannelTest, TestUsingPooledSessionAfterDoneGathering) {
+  ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags,
+                     kDefaultPortAllocatorFlags);
+  // First create a pooled session for each endpoint.
+  auto& allocator_1 = GetEndpoint(0)->allocator_;
+  auto& allocator_2 = GetEndpoint(1)->allocator_;
+  allocator_1->SetConfiguration(allocator_1->stun_servers(),
+                                allocator_1->turn_servers(), 1);
+  allocator_2->SetConfiguration(allocator_2->stun_servers(),
+                                allocator_2->turn_servers(), 1);

[pthatcher1, 2016/05/05 21:51:25]

Can you give this value of 1 a name to make it more clear what it is?

[Taylor Brandstetter, 2016/05/06 03:53:34]

Done.

+  const cricket::PortAllocatorSession* pooled_session_1 =
+      allocator_1->PeekPooledSession();
+  const cricket::PortAllocatorSession* pooled_session_2 =
+      allocator_2->PeekPooledSession();
+  ASSERT_NE(nullptr, pooled_session_1);
+  ASSERT_NE(nullptr, pooled_session_2);
+  // Wait for the pooled sessions to finish gathering before the
+  // P2PTransportChannels try to use them.
+  EXPECT_TRUE_WAIT(pooled_session_1->CandidatesAllocationDone() &&
+                       pooled_session_2->CandidatesAllocationDone(),
+                   kDefaultTimeout);
+  // Now let the endpoints connect and try exchanging some data.
+  CreateChannels(1);
+  EXPECT_TRUE_WAIT_MARGIN(ep1_ch1() != NULL && ep2_ch1() != NULL &&
+                              ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+                              ep2_ch1()->receiving() && ep2_ch1()->writable(),
+                          1000, 1000);
+  TestSendRecv(1);
+  // Make sure the P2PTransportChannels are actually using ports from the
+  // pooled sessions.
+  auto pooled_ports_1 = pooled_session_1->ReadyPorts();
+  auto pooled_ports_2 = pooled_session_2->ReadyPorts();
+  EXPECT_NE(pooled_ports_1.end(),
+            std::find(pooled_ports_1.begin(), pooled_ports_1.end(),
+                      ep1_ch1()->best_connection()->port()));
+  EXPECT_NE(pooled_ports_2.end(),
+            std::find(pooled_ports_2.begin(), pooled_ports_2.end(),
+                      ep2_ch1()->best_connection()->port()));
+}
+
 // Test what happens when we have 2 users behind the same NAT. This can lead
 // to interesting behavior because the STUN server will only give out the
 // address of the outermost NAT.
 class P2PTransportChannelSameNatTest : public P2PTransportChannelTestBase {
  protected:
   void ConfigureEndpoints(Config nat_type, Config config1, Config config2) {
     ASSERT(nat_type >= NAT_FULL_CONE && nat_type <= NAT_SYMMETRIC);
     rtc::NATSocketServer::Translator* outer_nat =
         nat()->AddTranslator(kPublicAddrs[0], kNatAddrs[0],
             static_cast<rtc::NATType>(nat_type - NAT_FULL_CONE));
@@ skipping 1237 matching lines @@
 
   // TCP Relay/Relay is the next.
   VerifyNextPingableConnection(cricket::RELAY_PORT_TYPE,
                                cricket::RELAY_PORT_TYPE,
                                cricket::TCP_PROTOCOL_NAME);
 
   // Finally, Local/Relay will be pinged.
   VerifyNextPingableConnection(cricket::LOCAL_PORT_TYPE,
                                cricket::RELAY_PORT_TYPE);
 }