Bug 4865: Enable connectivity when the remote peer is on public internet.

webrtc/p2p/client/portallocator_unittest.cc

Index: webrtc/p2p/client/portallocator_unittest.cc
diff --git a/webrtc/p2p/client/portallocator_unittest.cc b/webrtc/p2p/client/portallocator_unittest.cc
index 7dc25140cccf91a91194af75af8f8b4c18f33993..8735eb0c5ae83c543184e1f9027fa46a6779ad58 100644
--- a/webrtc/p2p/client/portallocator_unittest.cc
+++ b/webrtc/p2p/client/portallocator_unittest.cc
@@ -110,9 +110,20 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> {
   void AddInterface(const SocketAddress& addr) {
     network_manager_.AddInterface(addr);
   }
+  void AddInterfaceAsDefault(const SocketAddress& addr) {

[pthatcher1, 2015/08/06 01:06:19]

Would SetDefaultRoute make more sense?

[guoweis_webrtc, 2015/08/06 08:50:07]

I renamed it as AddInterfaceAsDefaultRoute, since it's doing the AddInterface
part.

+    AddInterface(addr);
+    // When a binding comes from the any address, the |addr| will be used as the
+    // srflx address.
+    vss_->set_default_interface(addr.ipaddr());
+  }
   bool SetPortRange(int min_port, int max_port) {
     return allocator_->SetPortRange(min_port, max_port);
   }
+  // No STUN/TURN configuration.
+  void ResetWithNoServers() {
+    allocator_.reset(new cricket::BasicPortAllocator(&network_manager_));
+    allocator_->set_step_delay(cricket::kMinimumStepDelay);
+  }
   void ResetWithNatServer(const rtc::SocketAddress& stun_server) {
     nat_server_.reset(new rtc::NATServer(
         rtc::NAT_OPEN_CONE, vss_.get(), kNatUdpAddr, kNatTcpAddr, vss_.get(),
@@ -232,7 +243,7 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> {
     }
   }
 
-  void CheckDisableAdapterEnumeration() {
+  void CheckDisableAdapterEnumerationNonSharedSocket() {

[juberti1, 2015/08/06 00:35:26]

can we just fix this to use shared socket? non-shared socket is going to
disappear very soon.

[guoweis_webrtc, 2015/08/06 08:50:07]

Done.

     EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
     session_->set_flags(cricket::PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION);
     session_->StartGettingPorts();
@@ -461,7 +472,7 @@ TEST_F(PortAllocatorTest, TestDisableAdapterEnumeration) {
   ResetWithNatServer(kStunAddr);
   AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
 
-  CheckDisableAdapterEnumeration();
+  CheckDisableAdapterEnumerationNonSharedSocket();
 }
 
 // Test that even with multiple interfaces, the result should be only 1 Stun
@@ -472,7 +483,7 @@ TEST_F(PortAllocatorTest, TestDisableAdapterEnumerationMultipleInterfaces) {
   ResetWithNatServer(kStunAddr);
   AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
 
-  CheckDisableAdapterEnumeration();
+  CheckDisableAdapterEnumerationNonSharedSocket();
 }
 
 // Disable for asan, see
@@ -1161,6 +1172,45 @@ TEST_F(PortAllocatorTest, TestDestroyPortsNonSharedSockets) {
   }
 }
 
+// Test that when adapter enumeration is disabled, STUN is the only candidate

[juberti1, 2015/08/06 00:35:26]

Isn't this already tested by the tests above?

[pthatcher1, 2015/08/06 01:06:19]

I think this one is different because it is shared socket and
PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION. The other is not shared socket.  But
we should probably just delete the non-shared socket test.

[guoweis_webrtc, 2015/08/06 08:50:07]

this one is different because it doesn't have a NAT. It's testing the endpoint
sitting on the public internet.

+// for endpoints not behind a NAT.
+TEST_F(PortAllocatorTest, TestAdapterEnumerationDisabledWithoutNat) {

[juberti1, 2015/08/06 00:35:26]

move this next to other adapter-enum disabled tests?

[guoweis_webrtc, 2015/08/06 08:50:07]

Done.

+  AddInterfaceAsDefault(kClientAddr);
+  allocator_->set_flags(allocator().flags() |
+                        cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG |
+                        cricket::PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION |
+                        cricket::PORTALLOCATOR_DISABLE_RELAY |
+                        cricket::PORTALLOCATOR_DISABLE_TCP |
+                        cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET);
+  EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
+  session_->StartGettingPorts();
+  EXPECT_TRUE_WAIT(1U == ports_.size(), kDefaultAllocationTimeout);
+  ASSERT_EQ_WAIT(1U, candidates_.size(), kDefaultAllocationTimeout);
+  EXPECT_PRED5(CheckCandidate, candidates_[0],
+               cricket::ICE_CANDIDATE_COMPONENT_RTP, "stun", "udp",
+               kClientAddr);
+  EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
+}
+
+// Test that when adapter enumeration is disabled, for endpoints without
+// STUN/TURN specified, no candidate is generated.
+TEST_F(PortAllocatorTest,
+       TestAdapterEnumerationDisabledWithoutNat_NoCandidate) {
+  AddInterfaceAsDefault(kClientAddr);
+  ResetWithNoServers();
+  allocator_->set_flags(allocator().flags() |
+                        cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG |
+                        cricket::PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION |
+                        cricket::PORTALLOCATOR_DISABLE_RELAY |
+                        cricket::PORTALLOCATOR_DISABLE_TCP |
+                        cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET);
+  EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
+  session_->StartGettingPorts();
+  EXPECT_TRUE_WAIT(1U == ports_.size(), kDefaultAllocationTimeout);
+  EXPECT_EQ(candidates_.size(), 0u);
+  EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
+}
+
 class AllocationSequenceForTest : public cricket::AllocationSequence {
  public:
   AllocationSequenceForTest(cricket::BasicPortAllocatorSession* session,