Restructure PeerConnectionBundleTest helper not to create PortAllocator

Instead rely on PeerConnectionFactory to create it.

Bug: webrtc:42232556
Change-Id: If3de8a2e311fcdca4371cca03d10bd383fbd3e01
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375922
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43835}

pc/BUILD.gn

@@ -2349,7 +2349,6 @@ if (rtc_include_tests && !build_with_chromium) {
       ":audio_track",
       ":channel",
       ":channel_interface",
-      ":data_channel_controller_unittest",
       ":dtls_srtp_transport",
       ":dtls_transport",
       ":dtmf_sender",
@@ -2362,6 +2361,7 @@ if (rtc_include_tests && !build_with_chromium) {
       ":media_protocol_names",
       ":media_session",
       ":media_stream",
+      ":pc_test_utils",
       ":peer_connection",
       ":peer_connection_factory",
       ":peer_connection_internal",
@@ -2397,7 +2397,6 @@ if (rtc_include_tests && !build_with_chromium) {
       "../api:candidate",
       "../api:create_peerconnection_factory",
       "../api:dtls_transport_interface",
-      "../api:dtmf_sender_interface",
       "../api:enable_media",
       "../api:enable_media_with_defaults",
       "../api:fake_frame_decryptor",
@@ -2419,6 +2418,8 @@ if (rtc_include_tests && !build_with_chromium) {
       "../api:ref_count",
       "../api:rtc_error",
       "../api:rtc_error_matchers",
+      "../api:rtc_stats_api",
+      "../api:rtp_parameters",
       "../api:rtp_sender_interface",
       "../api:rtp_transceiver_direction",
       "../api:scoped_refptr",
@@ -2428,6 +2429,13 @@ if (rtc_include_tests && !build_with_chromium) {
       "../api/audio:audio_mixer_api",
       "../api/audio:audio_processing",
       "../api/audio:audio_processing_statistics",
+      "../api/audio_codecs:audio_codecs_api",
+      "../api/audio_codecs:builtin_audio_decoder_factory",
+      "../api/audio_codecs:builtin_audio_encoder_factory",
+      "../api/audio_codecs:opus_audio_decoder_factory",
+      "../api/audio_codecs:opus_audio_encoder_factory",
+      "../api/audio_codecs/L16:audio_decoder_L16",
+      "../api/audio_codecs/L16:audio_encoder_L16",
       "../api/crypto:frame_decryptor_interface",
       "../api/crypto:frame_encryptor_interface",
       "../api/crypto:options",
@@ -2450,12 +2458,25 @@ if (rtc_include_tests && !build_with_chromium) {
       "../api/video:builtin_video_bitrate_allocator_factory",
       "../api/video:encoded_image",
       "../api/video:recordable_encoded_frame",
-      "../api/video:resolution",
       "../api/video:video_bitrate_allocator_factory",
       "../api/video:video_codec_constants",
       "../api/video:video_frame",
       "../api/video:video_rtp_headers",
+      "../api/video_codecs:builtin_video_decoder_factory",
+      "../api/video_codecs:builtin_video_encoder_factory",
       "../api/video_codecs:scalability_mode",
+      "../api/video_codecs:video_codecs_api",
+      "../api/video_codecs:video_decoder_factory_template",
+      "../api/video_codecs:video_decoder_factory_template_dav1d_adapter",
+      "../api/video_codecs:video_decoder_factory_template_libvpx_vp8_adapter",
+      "../api/video_codecs:video_decoder_factory_template_libvpx_vp9_adapter",
+      "../api/video_codecs:video_decoder_factory_template_open_h264_adapter",
+      "../api/video_codecs:video_encoder_factory_template",
+      "../api/video_codecs:video_encoder_factory_template_libaom_av1_adapter",
+      "../api/video_codecs:video_encoder_factory_template_libvpx_vp8_adapter",
+      "../api/video_codecs:video_encoder_factory_template_libvpx_vp9_adapter",
+      "../api/video_codecs:video_encoder_factory_template_open_h264_adapter",
+      "../call:call_interfaces",
       "../call/adaptation:resource_adaptation_test_utilities",
       "../common_video",
       "../logging:fake_rtc_event_log",
@@ -2464,20 +2485,21 @@ if (rtc_include_tests && !build_with_chromium) {
       "../media:media_constants",
       "../media:media_engine",
       "../media:rid_description",
+      "../media:rtc_audio_video",
       "../media:rtc_data_sctp_transport_internal",
       "../media:rtc_media_config",
+      "../media:rtc_media_tests_utils",
       "../media:stream_params",
       "../modules/audio_processing:mocks",
       "../modules/rtp_rtcp:rtp_rtcp_format",
       "../p2p:basic_packet_socket_factory",
       "../p2p:basic_port_allocator",
-      "../p2p:connection",
       "../p2p:connection_info",
       "../p2p:dtls_transport_internal",
       "../p2p:fake_port_allocator",
       "../p2p:ice_transport_internal",
       "../p2p:p2p_constants",
-      "../p2p:p2p_server_utils",
+      "../p2p:p2p_test_utils",
       "../p2p:port",
       "../p2p:port_allocator",
       "../p2p:port_interface",
@@ -2493,7 +2515,6 @@ if (rtc_include_tests && !build_with_chromium) {
       "../rtc_base:ip_address",
       "../rtc_base:logging",
       "../rtc_base:macromagic",
-      "../rtc_base:mdns_responder_interface",
       "../rtc_base:net_helper",
       "../rtc_base:network",
       "../rtc_base:network_constants",
@@ -2503,6 +2524,7 @@ if (rtc_include_tests && !build_with_chromium) {
       "../rtc_base:rtc_base_tests_utils",
       "../rtc_base:rtc_certificate_generator",
       "../rtc_base:rtc_json",
+      "../rtc_base:safe_conversions",
       "../rtc_base:socket_address",
       "../rtc_base:socket_server",
       "../rtc_base:ssl",
@@ -2517,16 +2539,23 @@ if (rtc_include_tests && !build_with_chromium) {
       "../rtc_base/third_party/base64",
       "../rtc_base/third_party/sigslot",
       "../system_wrappers:metrics",
+      "../test:audio_codec_mocks",
       "../test:rtc_expect_death",
       "../test:run_loop",
-      "../test:scoped_key_value_config",
+      "../test:test_support",
       "../test:wait_until",
       "../test/pc/sctp:fake_sctp_transport",
-      "//testing/gmock",
-      "//testing/gtest",
       "//third_party/abseil-cpp/absl/algorithm:container",
       "//third_party/abseil-cpp/absl/memory",
       "//third_party/abseil-cpp/absl/strings",
+      "//third_party/abseil-cpp/absl/strings:string_view",
+    ]
+
+    # These deps are kept separately because they can't be automatically
+    # regenerated by gn_check_autofix tool
+    deps += [
+      ":data_channel_controller_unittest",
+      "../test:test_main",
     ]
 
     if (is_android) {
@@ -2540,44 +2569,6 @@ if (rtc_include_tests && !build_with_chromium) {
       ]
       shard_timeout = 900
     }
-
-    deps += [
-      ":libjingle_peerconnection",
-      ":pc_test_utils",
-      ":rtc_pc",
-      "../api:rtc_event_log_output_file",
-      "../api:rtc_stats_api",
-      "../api:rtp_parameters",
-      "../api/audio_codecs:audio_codecs_api",
-      "../api/audio_codecs:builtin_audio_decoder_factory",
-      "../api/audio_codecs:builtin_audio_encoder_factory",
-      "../api/audio_codecs:opus_audio_decoder_factory",
-      "../api/audio_codecs:opus_audio_encoder_factory",
-      "../api/audio_codecs/L16:audio_decoder_L16",
-      "../api/audio_codecs/L16:audio_encoder_L16",
-      "../api/video_codecs:builtin_video_decoder_factory",
-      "../api/video_codecs:builtin_video_encoder_factory",
-      "../api/video_codecs:video_codecs_api",
-      "../api/video_codecs:video_decoder_factory_template",
-      "../api/video_codecs:video_decoder_factory_template_dav1d_adapter",
-      "../api/video_codecs:video_decoder_factory_template_libvpx_vp8_adapter",
-      "../api/video_codecs:video_decoder_factory_template_libvpx_vp9_adapter",
-      "../api/video_codecs:video_decoder_factory_template_open_h264_adapter",
-      "../api/video_codecs:video_encoder_factory_template",
-      "../api/video_codecs:video_encoder_factory_template_libaom_av1_adapter",
-      "../api/video_codecs:video_encoder_factory_template_libvpx_vp8_adapter",
-      "../api/video_codecs:video_encoder_factory_template_libvpx_vp9_adapter",
-      "../api/video_codecs:video_encoder_factory_template_open_h264_adapter",
-      "../call:call_interfaces",
-      "../media:rtc_audio_video",
-      "../media:rtc_media_tests_utils",
-      "../modules/audio_processing",
-      "../p2p:p2p_test_utils",
-      "../rtc_base:safe_conversions",
-      "../test:audio_codec_mocks",
-      "../test:test_main",
-      "../test:test_support",
-    ]
   }
 
   rtc_library("data_channel_controller_unittest") {

pc/peer_connection_bundle_unittest.cc

@@ -18,10 +18,8 @@
 #include <vector>
 
 #include "api/audio/audio_device.h"
-#include "api/audio_codecs/builtin_audio_decoder_factory.h"
-#include "api/audio_codecs/builtin_audio_encoder_factory.h"
 #include "api/candidate.h"
-#include "api/create_peerconnection_factory.h"
+#include "api/enable_media_with_defaults.h"
 #include "api/jsep.h"
 #include "api/media_types.h"
 #include "api/peer_connection_interface.h"
@@ -43,11 +41,9 @@
 #include "api/video_codecs/video_encoder_factory_template_libvpx_vp9_adapter.h"
 #include "api/video_codecs/video_encoder_factory_template_open_h264_adapter.h"
 #include "media/base/stream_params.h"
-#include "p2p/base/basic_packet_socket_factory.h"
 #include "p2p/base/p2p_constants.h"
 #include "p2p/base/port_allocator.h"
 #include "p2p/base/transport_info.h"
-#include "p2p/client/basic_port_allocator.h"
 #include "pc/channel.h"
 #include "pc/peer_connection.h"
 #include "pc/peer_connection_proxy.h"
@@ -56,23 +52,24 @@
 #include "pc/rtp_transport_internal.h"
 #include "pc/sdp_utils.h"
 #include "pc/session_description.h"
+#include "pc/test/fake_audio_capture_module.h"
 #include "pc/test/integration_test_helpers.h"
 #include "pc/test/mock_peer_connection_observers.h"
 #include "rtc_base/checks.h"
+#include "rtc_base/fake_network.h"
 #include "rtc_base/logging.h"
 #include "rtc_base/net_helper.h"
 #include "rtc_base/network.h"
 #include "rtc_base/socket_address.h"
 #include "rtc_base/thread.h"
+#include "rtc_base/virtual_socket_server.h"
+#include "test/gmock.h"
 #include "test/gtest.h"
 #include "test/wait_until.h"
+
 #ifdef WEBRTC_ANDROID
 #include "pc/test/android_test_initializer.h"
 #endif
-#include "pc/test/fake_audio_capture_module.h"
-#include "rtc_base/fake_network.h"
-#include "rtc_base/virtual_socket_server.h"
-#include "test/gmock.h"
 
 namespace webrtc {
 
@@ -205,24 +202,10 @@ class PeerConnectionBundleBaseTest : public ::testing::Test {
   typedef std::unique_ptr<PeerConnectionWrapperForBundleTest> WrapperPtr;
 
   explicit PeerConnectionBundleBaseTest(SdpSemantics sdp_semantics)
-      : vss_(new rtc::VirtualSocketServer()),
+      : main_(&vss_), sdp_semantics_(sdp_semantics) {
-        socket_factory_(new rtc::BasicPacketSocketFactory(vss_.get())),
-        main_(vss_.get()),
-        sdp_semantics_(sdp_semantics) {
 #ifdef WEBRTC_ANDROID
     InitializeAndroidObjects();
 #endif
-    pc_factory_ = CreatePeerConnectionFactory(
-        rtc::Thread::Current(), rtc::Thread::Current(), rtc::Thread::Current(),
-        rtc::scoped_refptr<AudioDeviceModule>(FakeAudioCaptureModule::Create()),
-        CreateBuiltinAudioEncoderFactory(), CreateBuiltinAudioDecoderFactory(),
-        std::make_unique<VideoEncoderFactoryTemplate<
-            LibvpxVp8EncoderTemplateAdapter, LibvpxVp9EncoderTemplateAdapter,
-            OpenH264EncoderTemplateAdapter, LibaomAv1EncoderTemplateAdapter>>(),
-        std::make_unique<VideoDecoderFactoryTemplate<
-            LibvpxVp8DecoderTemplateAdapter, LibvpxVp9DecoderTemplateAdapter,
-            OpenH264DecoderTemplateAdapter, Dav1dDecoderTemplateAdapter>>(),
-        nullptr /* audio_mixer */, nullptr /* audio_processing */);
   }
 
   WrapperPtr CreatePeerConnection() {
@@ -230,25 +213,46 @@ class PeerConnectionBundleBaseTest : public ::testing::Test {
   }
 
   WrapperPtr CreatePeerConnection(const RTCConfiguration& config) {
-    auto* fake_network = NewFakeNetwork();
+    // Each PeerConnection has its own `NetworkManager` which is injected into
-    auto port_allocator = std::make_unique<cricket::BasicPortAllocator>(
+    // `PeerConnectionFactoryDependencies`, thus each PeerConnection in these
-        fake_network, socket_factory_.get());
+    // tests is created with own PeerConnectionFactory.
-    port_allocator->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP |
+    PeerConnectionFactoryDependencies pcf_deps;
-                              cricket::PORTALLOCATOR_DISABLE_RELAY);
+    pcf_deps.network_thread = rtc::Thread::Current();
-    port_allocator->set_step_delay(cricket::kMinimumStepDelay);
+    pcf_deps.worker_thread = rtc::Thread::Current();
+    pcf_deps.signaling_thread = rtc::Thread::Current();
+    pcf_deps.socket_factory = &vss_;
+    auto network_manager =
+        std::make_unique<FakeNetworkManagerWithNoAnyNetwork>();
+    auto* fake_network = network_manager.get();
+    pcf_deps.network_manager = std::move(network_manager);
+    pcf_deps.adm = FakeAudioCaptureModule::Create();
+    pcf_deps.video_encoder_factory =
+        std::make_unique<VideoEncoderFactoryTemplate<
+            LibvpxVp8EncoderTemplateAdapter, LibvpxVp9EncoderTemplateAdapter,
+            OpenH264EncoderTemplateAdapter, LibaomAv1EncoderTemplateAdapter>>();
+    pcf_deps.video_decoder_factory =
+        std::make_unique<VideoDecoderFactoryTemplate<
+            LibvpxVp8DecoderTemplateAdapter, LibvpxVp9DecoderTemplateAdapter,
+            OpenH264DecoderTemplateAdapter, Dav1dDecoderTemplateAdapter>>();
+    EnableMediaWithDefaults(pcf_deps);
+
+    scoped_refptr<PeerConnectionFactoryInterface> pc_factory =
+        CreateModularPeerConnectionFactory(std::move(pcf_deps));
+
     auto observer = std::make_unique<MockPeerConnectionObserver>();
     RTCConfiguration modified_config = config;
+    modified_config.set_port_allocator_flags(
+        cricket::PORTALLOCATOR_DISABLE_TCP |
+        cricket::PORTALLOCATOR_DISABLE_RELAY);
     modified_config.sdp_semantics = sdp_semantics_;
-    PeerConnectionDependencies pc_dependencies(observer.get());
+    auto result = pc_factory->CreatePeerConnectionOrError(
-    pc_dependencies.allocator = std::move(port_allocator);
+        modified_config, PeerConnectionDependencies(observer.get()));
-    auto result = pc_factory_->CreatePeerConnectionOrError(
-        modified_config, std::move(pc_dependencies));
     if (!result.ok()) {
       return nullptr;
     }
 
     auto wrapper = std::make_unique<PeerConnectionWrapperForBundleTest>(
-        pc_factory_, result.MoveValue(), std::move(observer));
+        std::move(pc_factory), result.MoveValue(), std::move(observer));
     wrapper->set_network(fake_network);
     return wrapper;
   }
@@ -275,24 +279,8 @@ class PeerConnectionBundleBaseTest : public ::testing::Test {
     return candidate;
   }
 
-  rtc::FakeNetworkManager* NewFakeNetwork() {
+  rtc::VirtualSocketServer vss_;
-    // The PeerConnection's port allocator is tied to the PeerConnection's
-    // lifetime and expects the underlying NetworkManager to outlive it. If
-    // PeerConnectionWrapper owned the NetworkManager, it would be destroyed
-    // before the PeerConnection (since subclass members are destroyed before
-    // base class members). Therefore, the test fixture will own all the fake
-    // networks even though tests should access the fake network through the
-    // PeerConnectionWrapper.
-    auto* fake_network = new FakeNetworkManagerWithNoAnyNetwork();
-    fake_networks_.emplace_back(fake_network);
-    return fake_network;
-  }
-
-  std::unique_ptr<rtc::VirtualSocketServer> vss_;
-  std::unique_ptr<rtc::BasicPacketSocketFactory> socket_factory_;
   rtc::AutoSocketServerThread main_;
-  rtc::scoped_refptr<PeerConnectionFactoryInterface> pc_factory_;
-  std::vector<std::unique_ptr<rtc::FakeNetworkManager>> fake_networks_;
   const SdpSemantics sdp_semantics_;
 };