commit ecd3054b560a5a644ed7b5952466707463c9ef01
author Qingsi Wang <qingsi@webrtc.org> Wed May 22 21:34:56 2019
committer Commit Bot <commit-bot@chromium.org> Wed May 22 22:58:58 2019
tree 2f09854e5d898aeee81280e1ea5c7c38916974e0
parent 7e7c5c3c2599541f36e43cac4a786a8c34b9c2c0

Replace a broken assumption in candidate gathering for mDNS candidates.

The gathering of host candidates with mDNS names is asynchronous and its
completion can happen after a srflx candidate is gathered by the same
underlying socket. We have a broken check in UDPPort::CreateConnection()
that assumes the gathering of host and srflx candidates is sequential.

This CL also does minor refactoring and clean-up.

Bug: chromium:944577
Change-Id: Ic28136a9515081f40b232a22fcbf4209814ed33a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/138043
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Jeroen de Borst <jeroendb@webrtc.org>
Reviewed-by: Amit Hilbuch <amithi@webrtc.org>
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28030}

pc/peer_connection_histogram_unittest.cc

diff --git a/pc/peer_connection_histogram_unittest.cc b/pc/peer_connection_histogram_unittest.cc
index 97555a9..54c251a 100644
--- a/pc/peer_connection_histogram_unittest.cc
+++ b/pc/peer_connection_histogram_unittest.cc
@@ -33,6 +33,7 @@
 #include "pc/test/mock_peer_connection_observers.h"
 #include "rtc_base/arraysize.h"
 #include "rtc_base/checks.h"
+#include "rtc_base/fake_mdns_responder.h"
 #include "rtc_base/fake_network.h"
 #include "rtc_base/gunit.h"
 #include "rtc_base/ref_counted_object.h"
@@ -245,7 +246,8 @@
     webrtc::PeerConnectionDependencies deps(nullptr /* observer_in */);
 
     auto fake_network = NewFakeNetwork();
-    fake_network->CreateMdnsResponder(rtc::Thread::Current());
+    fake_network->set_mdns_responder(
+        absl::make_unique<webrtc::FakeMdnsResponder>(rtc::Thread::Current()));
     fake_network->AddInterface(NextLocalAddress());
 
     std::unique_ptr<cricket::BasicPortAllocator> port_allocator(

pc/peer_connection_integrationtest.cc

diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index 3a39971..0fc2e15 100644
--- a/pc/peer_connection_integrationtest.cc
+++ b/pc/peer_connection_integrationtest.cc
@@ -61,6 +61,7 @@
 #include "pc/test/fake_video_track_renderer.h"
 #include "pc/test/mock_peer_connection_observers.h"
 #include "rtc_base/fake_clock.h"
+#include "rtc_base/fake_mdns_responder.h"
 #include "rtc_base/fake_network.h"
 #include "rtc_base/firewall_socket_server.h"
 #include "rtc_base/gunit.h"
@@ -552,7 +553,7 @@
     }
   }
 
-  rtc::FakeNetworkManager* network() const {
+  rtc::FakeNetworkManager* network_manager() const {
     return fake_network_manager_.get();
   }
   cricket::PortAllocator* port_allocator() const { return port_allocator_; }
@@ -565,6 +566,15 @@
     return last_candidate_gathered_;
   }
 
+  // Sets the mDNS responder for the owned fake network manager and keeps a
+  // reference to the responder.
+  void SetMdnsResponder(
+      std::unique_ptr<webrtc::FakeMdnsResponder> mdns_responder) {
+    RTC_DCHECK(mdns_responder != nullptr);
+    mdns_responder_ = mdns_responder.get();
+    network_manager()->set_mdns_responder(std::move(mdns_responder));
+  }
+
  private:
   explicit PeerConnectionWrapper(const std::string& debug_name)
       : debug_name_(debug_name) {}
@@ -926,11 +936,10 @@
 
     if (remote_async_resolver_) {
       const auto& local_candidate = candidate->candidate();
-      const auto& mdns_responder = network()->GetMdnsResponderForTesting();
       if (local_candidate.address().IsUnresolvedIP()) {
         RTC_DCHECK(local_candidate.type() == cricket::LOCAL_PORT_TYPE);
         rtc::SocketAddress resolved_addr(local_candidate.address());
-        const auto resolved_ip = mdns_responder->GetMappedAddressForName(
+        const auto resolved_ip = mdns_responder_->GetMappedAddressForName(
             local_candidate.address().hostname());
         RTC_DCHECK(!resolved_ip.IsNil());
         resolved_addr.SetResolvedIP(resolved_ip);
@@ -959,6 +968,8 @@
   std::string debug_name_;
 
   std::unique_ptr<rtc::FakeNetworkManager> fake_network_manager_;
+  // Reference to the mDNS responder owned by |fake_network_manager_| after set.
+  webrtc::FakeMdnsResponder* mdns_responder_ = nullptr;
 
   rtc::scoped_refptr<webrtc::PeerConnectionInterface> peer_connection_;
   rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface>
@@ -3825,8 +3836,10 @@
   callee()->SetRemoteAsyncResolver(&caller_async_resolver);
 
   // Enable hostname candidates with mDNS names.
-  caller()->network()->CreateMdnsResponder(network_thread());
-  callee()->network()->CreateMdnsResponder(network_thread());
+  caller()->SetMdnsResponder(
+      absl::make_unique<webrtc::FakeMdnsResponder>(network_thread()));
+  callee()->SetMdnsResponder(
+      absl::make_unique<webrtc::FakeMdnsResponder>(network_thread()));
 
   SetPortAllocatorFlags(kOnlyLocalPorts, kOnlyLocalPorts);
 
@@ -3891,15 +3904,15 @@
 
   void SetUpNetworkInterfaces() {
     // Remove the default interfaces added by the test infrastructure.
-    caller()->network()->RemoveInterface(kDefaultLocalAddress);
-    callee()->network()->RemoveInterface(kDefaultLocalAddress);
+    caller()->network_manager()->RemoveInterface(kDefaultLocalAddress);
+    callee()->network_manager()->RemoveInterface(kDefaultLocalAddress);
 
     // Add network addresses for test.
     for (const auto& caller_address : CallerAddresses()) {
-      caller()->network()->AddInterface(caller_address);
+      caller()->network_manager()->AddInterface(caller_address);
     }
     for (const auto& callee_address : CalleeAddresses()) {
-      callee()->network()->AddInterface(callee_address);
+      callee()->network_manager()->AddInterface(callee_address);
     }
   }