[Nearby] Reuse IpcNetworkManager for peer connections

Previously a new IpcNetworkManager was created for each PeerConnection.
IpcNetworkManager calls StartNetworkNotifications on the
p2p_socket_manager_ which is not re-created for each PeerConnection. The
second call failed and ended up blocking ice candidate gathering because
it was waiting for the network list to come through which it never did.
The simple solution here is to just re-used the IpcNetworkManager for
each new peer connection in the same way the socket_factory_ is reused.
See the bug for more details.

Fixed: 1142717
Change-Id: I033e986361a7c3a7acce3a7dea12e1f51c3b3b75
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2503889Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Richard Knoll <knollr@chromium.org>
Commit-Queue: James Vecore <vecore@google.com>
Cr-Commit-Position: refs/heads/master@{#821512}

chrome/services/sharing/nearby/platform/webrtc.cc

@@ -233,16 +233,28 @@ void WebRtcMedium::OnIceServersFetched(
         p2p_socket_manager_, kTrafficAnnotation);
   }
 
-  auto network_manager = std::make_unique<sharing::IpcNetworkManager>(
-      p2p_socket_manager_,
-      std::make_unique<sharing::MdnsResponderAdapter>(mdns_responder_));
+  if (!network_manager_) {
+    // NOTE: |network_manager_| is only created once and shared for every peer
+    // connection due to the use of the shared |p2p_socket_manager_|. See
+    // https://crbug.com/1142717 for more details.
+    network_manager_ = std::make_unique<sharing::IpcNetworkManager>(
+        p2p_socket_manager_,
+        std::make_unique<sharing::MdnsResponderAdapter>(mdns_responder_));
+
+    // NOTE: IpcNetworkManager::Initialize() does not override the empty default
+    // implementation so this doesn't actually do anything right now. However
+    // the contract of rtc::NetworkManagerBase states that it should be called
+    // before using and explicitly on the network thread (which right now is the
+    // current thread). Previously this was handled by P2PPortAllocator.
+    network_manager_->Initialize();
+  }
 
   webrtc::PeerConnectionDependencies dependencies(observer);
   sharing::P2PPortAllocator::Config port_config;
   port_config.enable_multiple_routes = true;
   port_config.enable_nonproxied_udp = true;
   dependencies.allocator = std::make_unique<sharing::P2PPortAllocator>(
-      std::move(network_manager), socket_factory_.get(), port_config);
+      network_manager_.get(), socket_factory_.get(), port_config);
   dependencies.async_resolver_factory =
       std::make_unique<ProxyAsyncResolverFactory>(socket_factory_.get());
 

chrome/services/sharing/nearby/platform/webrtc.h

@@ -63,6 +63,7 @@ class WebRtcMedium : public api::WebRtcMedium {
       webrtc_signaling_messenger_;
 
   std::unique_ptr<sharing::IpcPacketSocketFactory> socket_factory_;
+  std::unique_ptr<rtc::NetworkManager> network_manager_;
 
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 

chrome/services/sharing/webrtc/p2p_port_allocator.cc

@@ -11,14 +11,12 @@
 
 namespace sharing {
 
-P2PPortAllocator::P2PPortAllocator(
-    std::unique_ptr<rtc::NetworkManager> network_manager,
-    rtc::PacketSocketFactory* socket_factory,
-    const Config& config)
-    : cricket::BasicPortAllocator(network_manager.get(), socket_factory),
-      network_manager_(std::move(network_manager)),
+P2PPortAllocator::P2PPortAllocator(rtc::NetworkManager* network_manager,
+                                   rtc::PacketSocketFactory* socket_factory,
+                                   const Config& config)
+    : cricket::BasicPortAllocator(network_manager, socket_factory),
       config_(config) {
-  DCHECK(network_manager_);
+  DCHECK(network_manager);
   DCHECK(socket_factory);
   uint32_t flags = 0;
   if (!config_.enable_multiple_routes) {
@@ -38,9 +36,4 @@ P2PPortAllocator::P2PPortAllocator(
 
 P2PPortAllocator::~P2PPortAllocator() = default;
 
-void P2PPortAllocator::Initialize() {
-  BasicPortAllocator::Initialize();
-  network_manager_->Initialize();
-}
-
 }  // namespace sharing

chrome/services/sharing/webrtc/p2p_port_allocator.h

@@ -33,18 +33,15 @@ class P2PPortAllocator : public cricket::BasicPortAllocator {
     bool enable_default_local_candidate = true;
   };
 
-  P2PPortAllocator(std::unique_ptr<rtc::NetworkManager> network_manager,
+  // NOTE: The network_manager passed must have had Initialize() called.
+  P2PPortAllocator(rtc::NetworkManager* network_manager,
                    rtc::PacketSocketFactory* socket_factory,
                    const Config& config);
   P2PPortAllocator(const P2PPortAllocator&) = delete;
   P2PPortAllocator& operator=(const P2PPortAllocator&) = delete;
   ~P2PPortAllocator() override;
 
-  // Will also initialize the network manager passed into the constructor.
-  void Initialize() override;
-
  private:
-  std::unique_ptr<rtc::NetworkManager> network_manager_;
   Config config_;
 };