Move injection of PacketSocketFactory from PC to PCF

Injection via PeerConnectionDependecies was broken, in not accepting
ownership of the injected object.

Bug: webrtc:7447, webrtc:14204
Change-Id: Ic53f05d51928b006fc1e46d502633d88471eb518
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/266140
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37270}

api/BUILD.gn

@@ -1100,6 +1100,17 @@ if (rtc_include_tests) {
     ]
   }
 
+  rtc_source_set("mock_packet_socket_factory") {
+    visibility = [ "*" ]
+    testonly = true
+    sources = [ "test/mock_packet_socket_factory.h" ]
+
+    deps = [
+      ":packet_socket_factory",
+      "../test:test_support",
+    ]
+  }
+
   rtc_source_set("mock_peerconnectioninterface") {
     visibility = [ "*" ]
     testonly = true
@@ -1322,6 +1333,7 @@ if (rtc_include_tests) {
       ":mock_frame_decryptor",
       ":mock_frame_encryptor",
       ":mock_media_stream_interface",
+      ":mock_packet_socket_factory",
       ":mock_peer_connection_factory_interface",
       ":mock_peerconnectioninterface",
       ":mock_rtp",

api/peer_connection_interface.h

@@ -1379,10 +1379,9 @@ struct RTC_EXPORT PeerConnectionDependencies final {
   PeerConnectionObserver* observer = nullptr;
   // Optional dependencies
   // TODO(bugs.webrtc.org/7447): remove port allocator once downstream is
-  // updated. For now, you can only set one of allocator and
-  // packet_socket_factory, not both.
+  // updated. The recommended way to inject networking components is to pass a
+  // PacketSocketFactory when creating the PeerConnectionFactory.
   std::unique_ptr<cricket::PortAllocator> allocator;
-  std::unique_ptr<rtc::PacketSocketFactory> packet_socket_factory;
   // Factory for creating resolvers that look up hostnames in DNS
   std::unique_ptr<webrtc::AsyncDnsResolverFactoryInterface>
       async_dns_resolver_factory;
@@ -1422,6 +1421,7 @@ struct RTC_EXPORT PeerConnectionFactoryDependencies final {
   rtc::Thread* worker_thread = nullptr;
   rtc::Thread* signaling_thread = nullptr;
   rtc::SocketFactory* socket_factory = nullptr;
+  std::unique_ptr<rtc::PacketSocketFactory> packet_socket_factory;
   std::unique_ptr<TaskQueueFactory> task_queue_factory;
   std::unique_ptr<cricket::MediaEngineInterface> media_engine;
   std::unique_ptr<CallFactoryInterface> call_factory;

api/test/mock_packet_socket_factory.h

@@ -0,0 +1,49 @@
+/*
+ *  Copyright (c) 2022 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.
+ */
+
+#ifndef API_TEST_MOCK_PACKET_SOCKET_FACTORY_H_
+#define API_TEST_MOCK_PACKET_SOCKET_FACTORY_H_
+
+#include <memory>
+#include <string>
+
+#include "api/packet_socket_factory.h"
+#include "test/gmock.h"
+
+namespace rtc {
+class MockPacketSocketFactory : public PacketSocketFactory {
+ public:
+  MOCK_METHOD(AsyncPacketSocket*,
+              CreateUdpSocket,
+              (const SocketAddress&, uint16_t, uint16_t),
+              (override));
+  MOCK_METHOD(AsyncListenSocket*,
+              CreateServerTcpSocket,
+              (const SocketAddress&, uint16_t, uint16_t, int opts),
+              (override));
+  MOCK_METHOD(AsyncPacketSocket*,
+              CreateClientTcpSocket,
+              (const SocketAddress& local_address,
+               const SocketAddress&,
+               const ProxyInfo&,
+               const std::string&,
+               const PacketSocketTcpOptions&),
+              (override));
+  MOCK_METHOD(std::unique_ptr<webrtc::AsyncDnsResolverInterface>,
+              CreateAsyncDnsResolver,
+              (),
+              (override));
+};
+
+static_assert(!std::is_abstract_v<MockPacketSocketFactory>, "");
+
+}  // namespace rtc
+
+#endif  // API_TEST_MOCK_PACKET_SOCKET_FACTORY_H_

pc/BUILD.gn

@@ -2327,6 +2327,7 @@ if (rtc_include_tests && !build_with_chromium) {
       "../api:make_ref_counted",
       "../api:media_stream_interface",
       "../api:mock_encoder_selector",
+      "../api:mock_packet_socket_factory",
       "../api:mock_video_track",
       "../api:packet_socket_factory",
       "../api:priority",

pc/connection_context.cc

@@ -103,6 +103,7 @@ ConnectionContext::ConnectionContext(
       network_monitor_factory_(
           std::move(dependencies->network_monitor_factory)),
       call_factory_(std::move(dependencies->call_factory)),
+      default_socket_factory_(std::move(dependencies->packet_socket_factory)),
       sctp_factory_(
           MaybeCreateSctpFactory(std::move(dependencies->sctp_factory),
                                  network_thread(),
@@ -146,9 +147,10 @@ ConnectionContext::ConnectionContext(
   default_network_manager_ = std::make_unique<rtc::BasicNetworkManager>(
       network_monitor_factory_.get(), socket_factory, &field_trials());
 
-  default_socket_factory_ =
-      std::make_unique<rtc::BasicPacketSocketFactory>(socket_factory);
-
+  if (!default_socket_factory_) {
+    default_socket_factory_ =
+        std::make_unique<rtc::BasicPacketSocketFactory>(socket_factory);
+  }
   // Set warning levels on the threads, to give warnings when response
   // may be slower than is expected of the thread.
   // Since some of the threads may be the same, start with the least

pc/connection_context.h

@@ -92,7 +92,7 @@ class ConnectionContext final
     RTC_DCHECK_RUN_ON(signaling_thread_);
     return default_network_manager_.get();
   }
-  rtc::BasicPacketSocketFactory* default_socket_factory() {
+  rtc::PacketSocketFactory* default_socket_factory() {
     RTC_DCHECK_RUN_ON(signaling_thread_);
     return default_socket_factory_.get();
   }
@@ -141,7 +141,7 @@ class ConnectionContext final
   std::unique_ptr<webrtc::CallFactoryInterface> const call_factory_
       RTC_GUARDED_BY(worker_thread());
 
-  std::unique_ptr<rtc::BasicPacketSocketFactory> default_socket_factory_
+  std::unique_ptr<rtc::PacketSocketFactory> default_socket_factory_
       RTC_GUARDED_BY(signaling_thread_);
   std::unique_ptr<SctpTransportFactoryInterface> const sctp_factory_;
 };

pc/peer_connection_factory.cc

@@ -204,9 +204,6 @@ PeerConnectionFactory::CreatePeerConnectionOrError(
     const PeerConnectionInterface::RTCConfiguration& configuration,
     PeerConnectionDependencies dependencies) {
   RTC_DCHECK_RUN_ON(signaling_thread());
-  RTC_DCHECK(!(dependencies.allocator && dependencies.packet_socket_factory))
-      << "You can't set both allocator and packet_socket_factory; "
-         "the former is going away (see bugs.webrtc.org/7447";
 
   // Set internal defaults if optional dependencies are not set.
   if (!dependencies.cert_generator) {
@@ -215,14 +212,8 @@ PeerConnectionFactory::CreatePeerConnectionOrError(
                                                        network_thread());
   }
   if (!dependencies.allocator) {
-    rtc::PacketSocketFactory* packet_socket_factory;
-    if (dependencies.packet_socket_factory)
-      packet_socket_factory = dependencies.packet_socket_factory.get();
-    else
-      packet_socket_factory = context_->default_socket_factory();
-
     dependencies.allocator = std::make_unique<cricket::BasicPortAllocator>(
-        context_->default_network_manager(), packet_socket_factory,
+        context_->default_network_manager(), context_->default_socket_factory(),
         configuration.turn_customizer);
     dependencies.allocator->SetPortRange(
         configuration.port_allocator_config.min_port,

pc/peer_connection_factory_unittest.cc

@@ -20,6 +20,7 @@
 #include "api/data_channel_interface.h"
 #include "api/jsep.h"
 #include "api/media_stream_interface.h"
+#include "api/test/mock_packet_socket_factory.h"
 #include "api/video_codecs/builtin_video_decoder_factory.h"
 #include "api/video_codecs/builtin_video_encoder_factory.h"
 #include "media/base/fake_frame_source.h"
@@ -31,6 +32,8 @@
 #include "p2p/base/port_interface.h"
 #include "pc/test/fake_audio_capture_module.h"
 #include "pc/test/fake_video_track_source.h"
+#include "pc/test/mock_peer_connection_observers.h"
+#include "rtc_base/gunit.h"
 #include "rtc_base/rtc_certificate_generator.h"
 #include "rtc_base/socket_address.h"
 #include "rtc_base/time_utils.h"
@@ -51,6 +54,10 @@ using webrtc::PeerConnectionObserver;
 using webrtc::VideoTrackInterface;
 using webrtc::VideoTrackSourceInterface;
 
+using ::testing::_;
+using ::testing::InvokeWithoutArgs;
+using ::testing::Return;
+
 namespace {
 
 static const char kStunIceServer[] = "stun:stun.l.google.com:19302";
@@ -492,3 +499,38 @@ TEST_F(PeerConnectionFactoryTest, LocalRendering) {
   EXPECT_EQ(3, local_renderer.num_rendered_frames());
   EXPECT_FALSE(local_renderer.black_frame());
 }
+
+#ifdef WEBRTC_ANDROID
+// TODO(bugs.webrtc.org/14204): Enable on android. Fails on Nexus 5X android
+// test bot, logging "Machine has no networks; no ports will be allocated"
+#define MAYBE_UsesPacketSocketFactory DISABLED_UsesPacketSocketFactory
+#else
+#define MAYBE_UsesPacketSocketFactory UsesPacketSocketFactory
+#endif
+TEST(PeerConnectionFactoryDependenciesTest, MAYBE_UsesPacketSocketFactory) {
+  constexpr int64_t kWaitTimeoutMs = 10000;
+  auto mock_socket_factory = std::make_unique<rtc::MockPacketSocketFactory>();
+
+  rtc::Event called;
+  EXPECT_CALL(*mock_socket_factory, CreateUdpSocket(_, _, _))
+      .WillOnce(InvokeWithoutArgs([&] {
+        called.Set();
+        return nullptr;
+      }))
+      .WillRepeatedly(Return(nullptr));
+
+  webrtc::PeerConnectionFactoryDependencies pcf_dependencies;
+  pcf_dependencies.packet_socket_factory = std::move(mock_socket_factory);
+
+  rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface> pcf =
+      CreateModularPeerConnectionFactory(std::move(pcf_dependencies));
+
+  PeerConnectionInterface::RTCConfiguration config;
+  config.ice_candidate_pool_size = 2;
+  NullPeerConnectionObserver observer;
+  auto pc = pcf->CreatePeerConnectionOrError(
+      config, webrtc::PeerConnectionDependencies(&observer));
+  ASSERT_TRUE(pc.ok());
+
+  called.Wait(kWaitTimeoutMs);
+}