Migrate PCLF not to create BasicPortAllocator itself

Instead rely on PeerConnectionFactory to create it

Bug: webrtc:42232556
Change-Id: I5710f979e0a030057b16c20fbf088ea2303be760
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/376882
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43881}

api/test/network_emulation_manager.h

@@ -26,7 +26,6 @@
 #include "api/packet_socket_factory.h"
 #include "api/test/network_emulation/cross_traffic.h"
 #include "api/test/network_emulation/network_emulation_interfaces.h"
-#include "api/test/peer_network_dependencies.h"
 #include "api/test/simulated_network.h"
 #include "api/test/time_controller.h"
 #include "api/units/data_rate.h"
@@ -141,29 +140,24 @@ class EmulatedNetworkManagerInterface {
   // EmulatedNetworkManagerInterface implementation.
   // Deprecated in favor of injecting NetworkManager into PeerConnectionFactory
   // instead of creating and injecting BasicPortAllocator into PeerConnection.
-  // TODO: bugs.webrtc.org/42232556 - Cleanup usages of this accessor in WebRTC,
+  [[deprecated("bugs.webrtc.org/42232556")]]  //
-  // and then deprecate or remove it.
+  virtual absl::Nonnull<rtc::NetworkManager*>
-  virtual absl::Nonnull<rtc::NetworkManager*> network_manager() = 0;
+  network_manager() = 0;
 
   // Returns packet socket factory that have to be injected
   // into WebRTC to properly setup network emulation. Returned factory is owned
   // by EmulatedNetworkManagerInterface implementation.
   // Deprecated in favor of injecting SocketFactory into PeerConnectionFactory
   // instead of creating and injecting BasicPortAllocator into PeerConnection.
-  // TODO: bugs.webrtc.org/42232556 - Cleanup usages of this accessor in WebRTC,
+  [[deprecated("bugs.webrtc.org/42232556")]]  //
-  // and then deprecate or remove it.
+  virtual absl::Nonnull<rtc::PacketSocketFactory*>
-  virtual absl::Nonnull<rtc::PacketSocketFactory*> packet_socket_factory() = 0;
+  packet_socket_factory() = 0;
 
   // Returns objects to pass to PeerConnectionFactoryDependencies.
   virtual absl::Nonnull<rtc::SocketFactory*> socket_factory() = 0;
   virtual absl::Nonnull<std::unique_ptr<rtc::NetworkManager>>
   ReleaseNetworkManager() = 0;
 
-  [[deprecated("bugs.webrtc.org/42232556")]]  //
-  webrtc::webrtc_pc_e2e::PeerNetworkDependencies
-  network_dependencies() {
-    return {network_thread(), network_manager(), packet_socket_factory()};
-  }
   // Returns list of endpoints that are associated with this instance. Pointers
   // are guaranteed to be non-null and are owned by NetworkEmulationManager.
   virtual std::vector<EmulatedEndpoint*> endpoints() const = 0;

api/test/pclf/BUILD.gn

@@ -44,7 +44,6 @@ rtc_library("media_quality_test_params") {
     "../..:field_trials_view",
     "../..:ice_transport_interface",
     "../..:libjingle_peerconnection_api",
-    "../..:packet_socket_factory",
     "../..:scoped_refptr",
     "../../../p2p:port_allocator",
     "../../../rtc_base:checks",
@@ -82,8 +81,8 @@ rtc_library("peer_configurer") {
     "../..:ice_transport_interface",
     "../..:libjingle_peerconnection_api",
     "../..:network_emulation_manager_api",
-    "../..:peer_network_dependencies",
     "../..:scoped_refptr",
+    "../../../p2p:port_allocator",
     "../../../rtc_base:checks",
     "../../../rtc_base:rtc_certificate_generator",
     "../../../rtc_base:ssl",

api/test/pclf/media_quality_test_params.h

@@ -15,6 +15,7 @@
 #include <memory>
 #include <optional>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "api/async_dns_resolver.h"
@@ -26,7 +27,6 @@
 #include "api/field_trials_view.h"
 #include "api/ice_transport_interface.h"
 #include "api/neteq/neteq_factory.h"
-#include "api/packet_socket_factory.h"
 #include "api/peer_connection_interface.h"
 #include "api/rtc_event_log/rtc_event_log_factory_interface.h"
 #include "api/scoped_refptr.h"
@@ -56,6 +56,8 @@ namespace webrtc_pc_e2e {
 // can override only some parts of media engine like video encoder/decoder
 // factories.
 struct PeerConnectionFactoryComponents {
+  std::unique_ptr<rtc::NetworkManager> network_manager;
+  rtc::SocketFactory* socket_factory = nullptr;
   std::unique_ptr<RtcEventLogFactoryInterface> event_log_factory;
   std::unique_ptr<FecControllerFactoryInterface> fec_controller_factory;
   std::unique_ptr<NetworkControllerFactoryInterface> network_controller_factory;
@@ -79,18 +81,8 @@ struct PeerConnectionFactoryComponents {
 // Separate class was introduced to clarify which components can be
 // overridden. For example observer, which is required to
 // PeerConnectionDependencies, will be provided by fixture implementation,
-// so client can't inject its own. Also only network manager can be overridden
+// so client can't inject its own.
-// inside port allocator.
 struct PeerConnectionComponents {
-  PeerConnectionComponents(rtc::NetworkManager* network_manager,
-                           rtc::PacketSocketFactory* packet_socket_factory)
-      : network_manager(network_manager),
-        packet_socket_factory(packet_socket_factory) {
-    RTC_CHECK(network_manager);
-  }
-
-  rtc::NetworkManager* const network_manager;
-  rtc::PacketSocketFactory* const packet_socket_factory;
   std::unique_ptr<webrtc::AsyncDnsResolverFactoryInterface>
       async_dns_resolver_factory;
   std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator;
@@ -102,15 +94,15 @@ struct PeerConnectionComponents {
 // has a network thread, that will be used to communicate with another peers.
 struct InjectableComponents {
   InjectableComponents(rtc::Thread* network_thread,
-                       rtc::NetworkManager* network_manager,
+                       std::unique_ptr<rtc::NetworkManager> network_manager,
-                       rtc::PacketSocketFactory* packet_socket_factory)
+                       rtc::SocketFactory* socket_factory)
       : network_thread(network_thread),
         worker_thread(nullptr),
         pcf_dependencies(std::make_unique<PeerConnectionFactoryComponents>()),
-        pc_dependencies(
+        pc_dependencies(std::make_unique<PeerConnectionComponents>()) {
-            std::make_unique<PeerConnectionComponents>(network_manager,
-                                                       packet_socket_factory)) {
     RTC_CHECK(network_thread);
+    pcf_dependencies->network_manager = std::move(network_manager);
+    pcf_dependencies->socket_factory = socket_factory;
   }
 
   rtc::Thread* const network_thread;
@@ -128,14 +120,12 @@ struct Params {
   std::optional<std::string> name;
   // If `audio_config` is set audio stream will be configured
   std::optional<AudioConfig> audio_config;
-  // Flags to set on `cricket::PortAllocator`. If not set,
+  // Flags to override `rtc_configuration.port_allocator_config.flags`
-  // cricket::kDefaultPortAllocatorFlags will be used and
-  // cricket::PORTALLOCATOR_DISABLE_TCP will be disabled.
   //
   // IMPORTANT: if you use WebRTC Network Emulation
   // (api/test/network_emulation_manager.h) and set this field, remember to set
-  // cricket::PORTALLOCATOR_DISABLE_TCP to 0.
+  // cricket::PORTALLOCATOR_DISABLE_TCP.
-  std::optional<uint32_t> port_allocator_flags = std::nullopt;
+  uint32_t port_allocator_flags = cricket::PORTALLOCATOR_DISABLE_TCP;
   // If `rtc_event_log_path` is set, an RTCEventLog will be saved in that
   // location and it will be available for further analysis.
   std::optional<std::string> rtc_event_log_path;

api/test/pclf/peer_configurer.cc

@@ -35,11 +35,11 @@
 #include "api/test/network_emulation_manager.h"
 #include "api/test/pclf/media_configuration.h"
 #include "api/test/pclf/media_quality_test_params.h"
-#include "api/test/peer_network_dependencies.h"
 #include "api/transport/bitrate_settings.h"
 #include "api/transport/network_control.h"
 #include "api/video_codecs/video_decoder_factory.h"
 #include "api/video_codecs/video_encoder_factory.h"
+#include "p2p/base/port_allocator.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/rtc_certificate_generator.h"
 #include "rtc_base/ssl_certificate.h"
@@ -47,20 +47,11 @@
 namespace webrtc {
 namespace webrtc_pc_e2e {
 
-PeerConfigurer::PeerConfigurer(
-    const PeerNetworkDependencies& network_dependencies)
-    : components_(std::make_unique<InjectableComponents>(
-          network_dependencies.network_thread,
-          network_dependencies.network_manager,
-          network_dependencies.packet_socket_factory)),
-      params_(std::make_unique<Params>()),
-      configurable_params_(std::make_unique<ConfigurableParams>()) {}
-
 PeerConfigurer::PeerConfigurer(EmulatedNetworkManagerInterface& network)
     : components_(std::make_unique<InjectableComponents>(
           network.network_thread(),
-          network.network_manager(),
+          network.ReleaseNetworkManager(),
-          network.packet_socket_factory())),
+          network.socket_factory())),
       params_(std::make_unique<Params>()),
       configurable_params_(std::make_unique<ConfigurableParams>()) {}
 

api/test/pclf/peer_configurer.h

@@ -33,7 +33,6 @@
 #include "api/test/network_emulation_manager.h"
 #include "api/test/pclf/media_configuration.h"
 #include "api/test/pclf/media_quality_test_params.h"
-#include "api/test/peer_network_dependencies.h"
 #include "api/transport/bitrate_settings.h"
 #include "api/transport/network_control.h"
 #include "api/video_codecs/video_decoder_factory.h"
@@ -51,8 +50,6 @@ class PeerConfigurer {
       absl::variant<std::unique_ptr<test::FrameGeneratorInterface>,
                     CapturingDeviceIndex>;
 
-  [[deprecated("bugs.webrtc.org/42232556")]] explicit PeerConfigurer(
-      const PeerNetworkDependencies& network_dependencies);
   explicit PeerConfigurer(EmulatedNetworkManagerInterface& network);
 
   // Sets peer name that will be used to report metrics related to this peer.

test/pc/e2e/BUILD.gn

@@ -121,8 +121,6 @@ if (!build_with_chromium) {
         "../../../api/video_codecs:video_codecs_api",
         "../../../modules/audio_device:test_audio_device_module",
         "../../../modules/audio_processing/aec_dump",
-        "../../../p2p:basic_port_allocator",
-        "../../../p2p:port_allocator",
         "../../../pc:pc_test_utils",
         "../../../rtc_base:checks",
         "../../../rtc_base:threading",

test/pc/e2e/test_peer_factory.cc

@@ -34,8 +34,6 @@
 #include "api/video_codecs/video_decoder_factory.h"
 #include "api/video_codecs/video_encoder_factory.h"
 #include "modules/audio_device/include/test_audio_device.h"
-#include "p2p/base/port_allocator.h"
-#include "p2p/client/basic_port_allocator.h"
 #include "pc/test/mock_peer_connection_observers.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/system/file_wrapper.h"
@@ -206,6 +204,8 @@ PeerConnectionFactoryDependencies CreatePCFDependencies(
   pcf_deps.signaling_thread = signaling_thread;
   pcf_deps.worker_thread = worker_thread;
   pcf_deps.network_thread = network_thread;
+  pcf_deps.socket_factory = pcf_dependencies->socket_factory;
+  pcf_deps.network_manager = std::move(pcf_dependencies->network_manager);
 
   pcf_deps.event_log_factory = std::move(pcf_dependencies->event_log_factory);
   pcf_deps.task_queue_factory = time_controller.CreateTaskQueueFactory();
@@ -247,22 +247,10 @@ PeerConnectionFactoryDependencies CreatePCFDependencies(
 // from InjectableComponents::PeerConnectionComponents.
 PeerConnectionDependencies CreatePCDependencies(
     MockPeerConnectionObserver* observer,
-    std::optional<uint32_t> port_allocator_flags,
     std::unique_ptr<PeerConnectionComponents> pc_dependencies) {
   PeerConnectionDependencies pc_deps(observer);
 
-  auto port_allocator = std::make_unique<cricket::BasicPortAllocator>(
-      pc_dependencies->network_manager, pc_dependencies->packet_socket_factory);
 
-  // This test does not support TCP by default.
-  int flags =
-      cricket::kDefaultPortAllocatorFlags | cricket::PORTALLOCATOR_DISABLE_TCP;
-  if (port_allocator_flags.has_value()) {
-    flags = *port_allocator_flags;
-  }
-  port_allocator->set_flags(port_allocator->flags() | flags);
-
-  pc_deps.allocator = std::move(port_allocator);
 
   if (pc_dependencies->async_dns_resolver_factory != nullptr) {
     pc_deps.async_dns_resolver_factory =
@@ -346,9 +334,11 @@ std::unique_ptr<TestPeer> TestPeerFactory::CreateTestPeer(
   }
 
   // Create peer connection.
-  PeerConnectionDependencies pc_deps =
+  PeerConnectionDependencies pc_deps = CreatePCDependencies(
-      CreatePCDependencies(observer.get(), params->port_allocator_flags,
+      observer.get(), std::move(components->pc_dependencies));
-                           std::move(components->pc_dependencies));
+
+  params->rtc_configuration.port_allocator_config.flags =
+      params->port_allocator_flags;
   rtc::scoped_refptr<PeerConnectionInterface> peer_connection =
       peer_connection_factory
           ->CreatePeerConnectionOrError(params->rtc_configuration,