From b1ec8133394fd711bc7734fdaf85d51a536d78b3 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Tue, 4 Feb 2025 17:50:44 +0100 Subject: [PATCH] Expose direct access to PeerConnection in PeerConnectionWrapper helper Multiple derived classes duplcated that code, and one more fixture can benefit from the same direct access to avoid saving reference to port allocator Cleaned includes and build dependencies on the way, in particular left single build target that contains peer_connection_wrapper Bug: webrtc:42232556 Change-Id: Ieb3d5449f3a0285230847716e33fb3b2d1b47882 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/376300 Reviewed-by: Per Kjellander Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#43847} --- pc/BUILD.gn | 9 +++------ pc/peer_connection_bundle_unittest.cc | 9 --------- pc/peer_connection_data_channel_unittest.cc | 18 +++--------------- pc/peer_connection_histogram_unittest.cc | 9 +-------- pc/peer_connection_ice_unittest.cc | 17 ++++++++--------- pc/peer_connection_signaling_unittest.cc | 15 ++++----------- pc/peer_connection_wrapper.cc | 9 +++++++++ pc/peer_connection_wrapper.h | 3 +++ 8 files changed, 31 insertions(+), 58 deletions(-) diff --git a/pc/BUILD.gn b/pc/BUILD.gn index d46ff16d72..c73261c6df 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -2241,6 +2241,8 @@ if (rtc_include_tests && !build_with_chromium) { ] deps = [ ":pc_test_utils", + ":peer_connection", + ":peer_connection_proxy", ":sdp_utils", "../api:function_view", "../api:libjingle_peerconnection_api", @@ -2250,14 +2252,11 @@ if (rtc_include_tests && !build_with_chromium) { "../api:rtc_error_matchers", "../api:rtc_stats_api", "../api:rtp_parameters", - "../api:rtp_sender_interface", "../api:scoped_refptr", "../rtc_base:checks", - "../rtc_base:gunit_helpers", "../rtc_base:logging", "../test:test_support", "../test:wait_until", - "//testing/gmock", ] } @@ -2323,8 +2322,6 @@ if (rtc_include_tests && !build_with_chromium) { "peer_connection_signaling_unittest.cc", "peer_connection_simulcast_unittest.cc", "peer_connection_svc_integrationtest.cc", - "peer_connection_wrapper.cc", - "peer_connection_wrapper.h", "proxy_unittest.cc", "rtc_stats_collector_unittest.cc", "rtc_stats_integrationtest.cc", @@ -2366,6 +2363,7 @@ if (rtc_include_tests && !build_with_chromium) { ":peer_connection_factory", ":peer_connection_internal", ":peer_connection_proxy", + ":peerconnection_wrapper", ":proxy", ":rtc_stats_collector", ":rtc_stats_traversal", @@ -2403,7 +2401,6 @@ if (rtc_include_tests && !build_with_chromium) { "../api:fake_frame_encryptor", "../api:field_trials", "../api:field_trials_view", - "../api:function_view", "../api:ice_transport_interface", "../api:libjingle_logging_api", "../api:libjingle_peerconnection_api", diff --git a/pc/peer_connection_bundle_unittest.cc b/pc/peer_connection_bundle_unittest.cc index db828b49c3..e5f7bd1f56 100644 --- a/pc/peer_connection_bundle_unittest.cc +++ b/pc/peer_connection_bundle_unittest.cc @@ -17,7 +17,6 @@ #include #include -#include "api/audio/audio_device.h" #include "api/candidate.h" #include "api/enable_media_with_defaults.h" #include "api/jsep.h" @@ -46,7 +45,6 @@ #include "p2p/base/transport_info.h" #include "pc/channel.h" #include "pc/peer_connection.h" -#include "pc/peer_connection_proxy.h" #include "pc/peer_connection_wrapper.h" #include "pc/rtp_transceiver.h" #include "pc/rtp_transport_internal.h" @@ -151,13 +149,6 @@ class PeerConnectionWrapperForBundleTest : public PeerConnectionWrapper { return nullptr; } - PeerConnection* GetInternalPeerConnection() { - auto* pci = - static_cast*>( - pc()); - return static_cast(pci->internal()); - } - // Returns true if the stats indicate that an ICE connection is either in // progress or established with the given remote address. bool HasConnectionWithRemoteAddress(const SocketAddress& address) { diff --git a/pc/peer_connection_data_channel_unittest.cc b/pc/peer_connection_data_channel_unittest.cc index 6b773a8722..9f7a21bd9c 100644 --- a/pc/peer_connection_data_channel_unittest.cc +++ b/pc/peer_connection_data_channel_unittest.cc @@ -11,7 +11,6 @@ #include #include #include -#include #include #include @@ -21,13 +20,9 @@ #include "api/scoped_refptr.h" #include "api/sctp_transport_interface.h" #include "api/task_queue/default_task_queue_factory.h" -#include "api/task_queue/task_queue_factory.h" -#include "api/transport/sctp_transport_factory_interface.h" #include "p2p/base/p2p_constants.h" -#include "p2p/base/port_allocator.h" #include "pc/media_session.h" #include "pc/peer_connection.h" -#include "pc/peer_connection_proxy.h" #include "pc/peer_connection_wrapper.h" #include "pc/sctp_transport.h" #include "pc/sdp_utils.h" @@ -36,15 +31,15 @@ #include "pc/test/mock_peer_connection_observers.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" -#include "rtc_base/rtc_certificate_generator.h" #include "rtc_base/thread.h" +#include "rtc_base/virtual_socket_server.h" #include "test/gmock.h" #include "test/gtest.h" +#include "test/pc/sctp/fake_sctp_transport.h" + #ifdef WEBRTC_ANDROID #include "pc/test/android_test_initializer.h" #endif -#include "rtc_base/virtual_socket_server.h" -#include "test/pc/sctp/fake_sctp_transport.h" namespace webrtc { @@ -90,13 +85,6 @@ class PeerConnectionWrapperForDataChannelTest : public PeerConnectionWrapper { return GetInternalPeerConnection()->sctp_transport_name(); } - PeerConnection* GetInternalPeerConnection() { - auto* pci = - static_cast*>( - pc()); - return static_cast(pci->internal()); - } - private: FakeSctpTransportFactory* sctp_transport_factory_ = nullptr; }; diff --git a/pc/peer_connection_histogram_unittest.cc b/pc/peer_connection_histogram_unittest.cc index 914124670a..a2847dc4e8 100644 --- a/pc/peer_connection_histogram_unittest.cc +++ b/pc/peer_connection_histogram_unittest.cc @@ -24,11 +24,11 @@ #include "api/task_queue/default_task_queue_factory.h" #include "api/test/mock_async_dns_resolver.h" #include "api/test/rtc_error_matchers.h" +#include "api/units/time_delta.h" #include "p2p/base/port_allocator.h" #include "p2p/client/basic_port_allocator.h" #include "pc/peer_connection.h" #include "pc/peer_connection_factory.h" -#include "pc/peer_connection_proxy.h" #include "pc/peer_connection_wrapper.h" #include "pc/sdp_utils.h" #include "pc/test/enable_fake_media.h" @@ -125,13 +125,6 @@ class PeerConnectionWrapperForUsageHistogramTest public: using PeerConnectionWrapper::PeerConnectionWrapper; - PeerConnection* GetInternalPeerConnection() { - auto* pci = - static_cast*>( - pc()); - return static_cast(pci->internal()); - } - // Override with different return type ObserverForUsageHistogramTest* observer() { return static_cast( diff --git a/pc/peer_connection_ice_unittest.cc b/pc/peer_connection_ice_unittest.cc index 1f1db87654..bb2531dbbf 100644 --- a/pc/peer_connection_ice_unittest.cc +++ b/pc/peer_connection_ice_unittest.cc @@ -136,9 +136,6 @@ class PeerConnectionWrapperForIceTest : public PeerConnectionWrapper { void set_network(rtc::FakeNetworkManager* network) { network_ = network; } - // The port allocator used by this PC. - cricket::PortAllocator* port_allocator_; - private: rtc::FakeNetworkManager* network_; }; @@ -182,7 +179,6 @@ class PeerConnectionIceBaseTest : public ::testing::Test { RTCConfiguration modified_config = config; modified_config.sdp_semantics = sdp_semantics_; auto observer = std::make_unique(); - auto port_allocator_copy = port_allocator.get(); PeerConnectionDependencies pc_dependencies(observer.get()); pc_dependencies.allocator = std::move(port_allocator); auto result = pc_factory_->CreatePeerConnectionOrError( @@ -195,7 +191,6 @@ class PeerConnectionIceBaseTest : public ::testing::Test { auto wrapper = std::make_unique( pc_factory_, result.MoveValue(), std::move(observer)); wrapper->set_network(fake_network); - wrapper->port_allocator_ = port_allocator_copy; return wrapper; } @@ -1566,9 +1561,11 @@ TEST_P(PeerConnectionIceTest, IceCredentialsCreateOffer) { config.sdp_semantics = SdpSemantics::kUnifiedPlan; config.ice_candidate_pool_size = 1; auto pc = CreatePeerConnectionWithAudioVideo(config); - ASSERT_NE(pc->port_allocator_, nullptr); + ASSERT_NE(pc->GetInternalPeerConnection()->port_allocator(), nullptr); auto offer = pc->CreateOffer(); - auto credentials = pc->port_allocator_->GetPooledIceCredentials(); + auto credentials = pc->GetInternalPeerConnection() + ->port_allocator() + ->GetPooledIceCredentials(); ASSERT_EQ(1u, credentials.size()); auto* desc = offer->description(); @@ -1584,12 +1581,14 @@ TEST_P(PeerConnectionIceTest, IceCredentialsCreateAnswer) { config.sdp_semantics = SdpSemantics::kUnifiedPlan; config.ice_candidate_pool_size = 1; auto pc = CreatePeerConnectionWithAudioVideo(config); - ASSERT_NE(pc->port_allocator_, nullptr); + ASSERT_NE(pc->GetInternalPeerConnection()->port_allocator(), nullptr); auto offer = pc->CreateOffer(); ASSERT_TRUE(pc->SetRemoteDescription(std::move(offer))); auto answer = pc->CreateAnswer(); - auto credentials = pc->port_allocator_->GetPooledIceCredentials(); + auto credentials = pc->GetInternalPeerConnection() + ->port_allocator() + ->GetPooledIceCredentials(); ASSERT_EQ(1u, credentials.size()); auto* desc = answer->description(); diff --git a/pc/peer_connection_signaling_unittest.cc b/pc/peer_connection_signaling_unittest.cc index c8ff14b16d..221455740b 100644 --- a/pc/peer_connection_signaling_unittest.cc +++ b/pc/peer_connection_signaling_unittest.cc @@ -51,23 +51,23 @@ #include "api/video_codecs/video_encoder_factory_template_open_h264_adapter.h" #include "media/base/codec.h" #include "pc/peer_connection.h" -#include "pc/peer_connection_proxy.h" #include "pc/peer_connection_wrapper.h" #include "pc/sdp_utils.h" #include "pc/session_description.h" +#include "pc/test/fake_audio_capture_module.h" +#include "pc/test/fake_rtc_certificate_generator.h" #include "pc/test/mock_peer_connection_observers.h" #include "rtc_base/checks.h" #include "rtc_base/string_encode.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 "pc/test/fake_rtc_certificate_generator.h" -#include "rtc_base/virtual_socket_server.h" namespace webrtc { @@ -90,13 +90,6 @@ class PeerConnectionWrapperForSignalingTest : public PeerConnectionWrapper { bool initial_offerer() { return GetInternalPeerConnection()->initial_offerer(); } - - PeerConnection* GetInternalPeerConnection() { - auto* pci = - static_cast*>( - pc()); - return static_cast(pci->internal()); - } }; class ExecuteFunctionOnCreateSessionDescriptionObserver diff --git a/pc/peer_connection_wrapper.cc b/pc/peer_connection_wrapper.cc index 2f63ce64a4..030c4de3f6 100644 --- a/pc/peer_connection_wrapper.cc +++ b/pc/peer_connection_wrapper.cc @@ -32,6 +32,8 @@ #include "api/scoped_refptr.h" #include "api/stats/rtc_stats_report.h" #include "api/test/rtc_error_matchers.h" +#include "pc/peer_connection.h" +#include "pc/peer_connection_proxy.h" #include "pc/sdp_utils.h" #include "pc/test/fake_video_track_source.h" #include "pc/test/mock_peer_connection_observers.h" @@ -76,6 +78,13 @@ MockPeerConnectionObserver* PeerConnectionWrapper::observer() { return observer_.get(); } +PeerConnection* PeerConnectionWrapper::GetInternalPeerConnection() { + auto* pci = + static_cast*>( + pc()); + return static_cast(pci->internal()); +} + std::unique_ptr PeerConnectionWrapper::CreateOffer() { return CreateOffer(RTCOfferAnswerOptions()); diff --git a/pc/peer_connection_wrapper.h b/pc/peer_connection_wrapper.h index 8055c7b16f..d87324f44c 100644 --- a/pc/peer_connection_wrapper.h +++ b/pc/peer_connection_wrapper.h @@ -28,6 +28,7 @@ #include "api/rtp_transceiver_interface.h" #include "api/scoped_refptr.h" #include "api/stats/rtc_stats_report.h" +#include "pc/peer_connection.h" #include "pc/test/mock_peer_connection_observers.h" namespace webrtc { @@ -62,6 +63,8 @@ class PeerConnectionWrapper { PeerConnectionInterface* pc(); MockPeerConnectionObserver* observer(); + PeerConnection* GetInternalPeerConnection(); + // Calls the underlying PeerConnection's CreateOffer method and returns the // resulting SessionDescription once it is available. If the method call // failed, null is returned.