diff --git a/api/BUILD.gn b/api/BUILD.gn index c270aadd28..6b82e9076c 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -897,11 +897,13 @@ rtc_source_set("network_emulation_manager_api") { "../rtc_base:network", "../rtc_base:network_constants", "../rtc_base:socket_address", + "../rtc_base:socket_factory", "../rtc_base:threading", "test/network_emulation", "units:data_rate", "units:data_size", "units:timestamp", + "//third_party/abseil-cpp/absl/base:nullability", "//third_party/abseil-cpp/absl/strings:string_view", ] } diff --git a/api/test/DEPS b/api/test/DEPS index 4a51584c86..02a677eadd 100644 --- a/api/test/DEPS +++ b/api/test/DEPS @@ -20,6 +20,7 @@ specific_include_rules = { "+rtc_base/network_constants.h", "+rtc_base/ip_address.h", "+rtc_base/socket_address.h", + "+rtc_base/socket_factory.h", ], "peer_network_dependencies\.h": [ "+rtc_base/network.h", diff --git a/api/test/network_emulation_manager.h b/api/test/network_emulation_manager.h index 338e16744d..8db1099f40 100644 --- a/api/test/network_emulation_manager.h +++ b/api/test/network_emulation_manager.h @@ -19,6 +19,7 @@ #include #include +#include "absl/base/nullability.h" #include "absl/strings/string_view.h" #include "api/array_view.h" #include "api/field_trials_view.h" @@ -33,6 +34,7 @@ #include "rtc_base/network.h" #include "rtc_base/network_constants.h" #include "rtc_base/socket_address.h" +#include "rtc_base/socket_factory.h" #include "rtc_base/thread.h" namespace webrtc { @@ -129,18 +131,36 @@ class EmulatedNetworkManagerInterface { public: virtual ~EmulatedNetworkManagerInterface() = default; - // Returns non-null pointer to thread that have to be used as network thread + // Returns thread that have to be used as network thread // for WebRTC to properly setup network emulation. Returned thread is owned // by EmulatedNetworkManagerInterface implementation. - virtual rtc::Thread* network_thread() = 0; - // Returns non-null pointer to network manager that have to be injected into + virtual absl::Nonnull network_thread() = 0; + + // Returns network manager that have to be injected into // WebRTC to properly setup network emulation. Returned manager is owned by // EmulatedNetworkManagerInterface implementation. - virtual rtc::NetworkManager* network_manager() = 0; - // Returns non-null pointer to packet socket factory that have to be injected + // 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, + // and then deprecate or remove it. + virtual absl::Nonnull 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. - virtual rtc::PacketSocketFactory* packet_socket_factory() = 0; + // 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, + // and then deprecate or remove it. + virtual absl::Nonnull packet_socket_factory() = 0; + + // Returns objects to pass to PeerConnectionFactoryDependencies. + virtual absl::Nonnull socket_factory() = 0; + virtual absl::Nonnull> + ReleaseNetworkManager() = 0; + + // TODO: bugs.webrtc.org/42232556 - Cleanup usages of this accessor in WebRTC, + // and then deprecate or remove it. webrtc::webrtc_pc_e2e::PeerNetworkDependencies network_dependencies() { return {network_thread(), network_manager(), packet_socket_factory()}; } @@ -349,11 +369,11 @@ class NetworkEmulationManager { virtual void StopCrossTraffic(CrossTrafficGenerator* generator) = 0; // Creates EmulatedNetworkManagerInterface which can be used then to inject - // network emulation layer into PeerConnection. `endpoints` - are available - // network interfaces for PeerConnection. If endpoint is enabled, it will be - // immediately available for PeerConnection, otherwise user will be able to - // enable endpoint later to make it available for PeerConnection. - virtual EmulatedNetworkManagerInterface* + // network emulation layer into PeerConnectionFactory. `endpoints` are + // available network interfaces for PeerConnection. If endpoint is enabled, it + // will be immediately available for PeerConnection, otherwise user will be + // able to enable endpoint later to make it available for PeerConnection. + virtual absl::Nonnull CreateEmulatedNetworkManagerInterface( const std::vector& endpoints) = 0; diff --git a/test/DEPS b/test/DEPS index 72f4c55e69..2ebbfcde24 100644 --- a/test/DEPS +++ b/test/DEPS @@ -66,14 +66,10 @@ specific_include_rules = { "+pc", "+p2p", ], - ".*peer_connection_quality_test_params\.h": [ - "+p2p/base/port_allocator.h", - ], ".*network_emulation_pc_unittest\.cc": [ "+p2p/base/port_allocator.h", "+pc/peer_connection_wrapper.h", "+pc/test/mock_peer_connection_observers.h", - "+p2p/client/basic_port_allocator.h", ], ".*peer_connection_quality_test\.(h|cc)": [ "+pc", diff --git a/test/network/BUILD.gn b/test/network/BUILD.gn index 511c02e164..8624a3a8d2 100644 --- a/test/network/BUILD.gn +++ b/test/network/BUILD.gn @@ -82,12 +82,12 @@ rtc_library("emulated_network") { "../../rtc_base/synchronization:mutex", "../../rtc_base/system:no_unique_address", "../../rtc_base/task_utils:repeating_task", - "../../rtc_base/third_party/sigslot", "../../system_wrappers", "../scenario:column_printer", "../time_controller", "//third_party/abseil-cpp/absl/algorithm:container", "//third_party/abseil-cpp/absl/base:nullability", + "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings:string_view", ] } @@ -106,7 +106,6 @@ if (rtc_include_tests && !build_with_chromium) { "../../api:libjingle_peerconnection_api", "../../api:media_stream_interface", "../../api:network_emulation_manager_api", - "../../api:packet_socket_factory", "../../api:rtc_error_matchers", "../../api:scoped_refptr", "../../api:simulated_network_api", @@ -115,7 +114,6 @@ if (rtc_include_tests && !build_with_chromium) { "../../api/test/network_emulation", "../../api/transport:field_trial_based_config", "../../modules/audio_device:test_audio_device_module", - "../../p2p:basic_port_allocator", "../../p2p:port_allocator", "../../pc:pc_test_utils", "../../pc:peerconnection_wrapper", diff --git a/test/network/emulated_network_manager.cc b/test/network/emulated_network_manager.cc index 1561f8e4a6..ce4a75b53d 100644 --- a/test/network/emulated_network_manager.cc +++ b/test/network/emulated_network_manager.cc @@ -15,35 +15,79 @@ #include #include +#include "absl/base/nullability.h" +#include "absl/memory/memory.h" #include "api/sequence_checker.h" +#include "api/task_queue/task_queue_base.h" #include "api/test/network_emulation/network_emulation_interfaces.h" #include "api/test/time_controller.h" #include "p2p/base/basic_packet_socket_factory.h" #include "rtc_base/checks.h" #include "rtc_base/network.h" -#include "rtc_base/task_queue_for_test.h" +#include "rtc_base/thread_annotations.h" #include "test/network/fake_network_socket_server.h" #include "test/network/network_emulation.h" namespace webrtc { namespace test { +// Framework assumes that rtc::NetworkManager is called from network thread. +class EmulatedNetworkManager::NetworkManagerImpl + : public rtc::NetworkManagerBase { + public: + explicit NetworkManagerImpl( + absl::Nonnull network_thread, + absl::Nonnull endpoints_container) + : network_thread_(network_thread), + endpoints_container_(endpoints_container) {} + + void StartUpdating() override; + void StopUpdating() override; + + void UpdateNetworksOnce(); + void MaybeSignalNetworksChanged(); + + // We don't support any address interfaces in the network emulation framework. + std::vector GetAnyAddressNetworks() override { + return {}; + } + + private: + const absl::Nonnull network_thread_; + const absl::Nonnull endpoints_container_; + bool sent_first_update_ RTC_GUARDED_BY(network_thread_) = false; + int start_count_ RTC_GUARDED_BY(network_thread_) = 0; +}; + EmulatedNetworkManager::EmulatedNetworkManager( TimeController* time_controller, - TaskQueueForTest* task_queue, + TaskQueueBase* task_queue, EndpointsContainer* endpoints_container) : task_queue_(task_queue), endpoints_container_(endpoints_container), - sent_first_update_(false), - start_count_(0) { - auto socket_server = - std::make_unique(endpoints_container); - packet_socket_factory_ = - std::make_unique(socket_server.get()); - // Since we pass ownership of the socket server to `network_thread_`, we must - // arrange that it outlives `packet_socket_factory_` which refers to it. - network_thread_ = - time_controller->CreateThread("net_thread", std::move(socket_server)); + socket_server_(new FakeNetworkSocketServer(endpoints_container)), + network_thread_( + time_controller->CreateThread("net_thread", + absl::WrapUnique(socket_server_))), + packet_socket_factory_(socket_server_), + network_manager_( + std::make_unique(network_thread_.get(), + endpoints_container)), + network_manager_ptr_(network_manager_.get()) {} + +EmulatedNetworkManager::~EmulatedNetworkManager() = default; + +rtc::NetworkManager* EmulatedNetworkManager::network_manager() { + RTC_CHECK(network_manager_ != nullptr) + << "network_manager() can't be used together with ReleaseNetworkManager."; + return network_manager_.get(); +} + +absl::Nonnull> +EmulatedNetworkManager::ReleaseNetworkManager() { + RTC_CHECK(network_manager_ != nullptr) + << "ReleaseNetworkManager can be called at most once."; + return std::move(network_manager_); } void EmulatedNetworkManager::EnableEndpoint(EmulatedEndpointImpl* endpoint) { @@ -51,7 +95,7 @@ void EmulatedNetworkManager::EnableEndpoint(EmulatedEndpointImpl* endpoint) { << "No such interface: " << endpoint->GetPeerLocalAddress().ToString(); network_thread_->PostTask([this, endpoint]() { endpoint->Enable(); - UpdateNetworksOnce(); + network_manager_ptr_->UpdateNetworksOnce(); }); } @@ -60,16 +104,14 @@ void EmulatedNetworkManager::DisableEndpoint(EmulatedEndpointImpl* endpoint) { << "No such interface: " << endpoint->GetPeerLocalAddress().ToString(); network_thread_->PostTask([this, endpoint]() { endpoint->Disable(); - UpdateNetworksOnce(); + network_manager_ptr_->UpdateNetworksOnce(); }); } -// Network manager interface. All these methods are supposed to be called from -// the same thread. -void EmulatedNetworkManager::StartUpdating() { - RTC_DCHECK_RUN_ON(network_thread_.get()); +void EmulatedNetworkManager::NetworkManagerImpl::StartUpdating() { + RTC_DCHECK_RUN_ON(network_thread_); - if (start_count_) { + if (start_count_ > 0) { // If network interfaces are already discovered and signal is sent, // we should trigger network signal immediately for the new clients // to start allocating ports. @@ -81,13 +123,13 @@ void EmulatedNetworkManager::StartUpdating() { ++start_count_; } -void EmulatedNetworkManager::StopUpdating() { - RTC_DCHECK_RUN_ON(network_thread_.get()); - if (!start_count_) +void EmulatedNetworkManager::NetworkManagerImpl::StopUpdating() { + RTC_DCHECK_RUN_ON(network_thread_); + if (start_count_ == 0) return; --start_count_; - if (!start_count_) { + if (start_count_ == 0) { sent_first_update_ = false; } } @@ -99,8 +141,8 @@ void EmulatedNetworkManager::GetStats( }); } -void EmulatedNetworkManager::UpdateNetworksOnce() { - RTC_DCHECK_RUN_ON(network_thread_.get()); +void EmulatedNetworkManager::NetworkManagerImpl::UpdateNetworksOnce() { + RTC_DCHECK_RUN_ON(network_thread_); std::vector> networks; for (std::unique_ptr& net : @@ -117,8 +159,8 @@ void EmulatedNetworkManager::UpdateNetworksOnce() { } } -void EmulatedNetworkManager::MaybeSignalNetworksChanged() { - RTC_DCHECK_RUN_ON(network_thread_.get()); +void EmulatedNetworkManager::NetworkManagerImpl::MaybeSignalNetworksChanged() { + RTC_DCHECK_RUN_ON(network_thread_); // If manager is stopped we don't need to signal anything. if (start_count_ == 0) { return; diff --git a/test/network/emulated_network_manager.h b/test/network/emulated_network_manager.h index 3ba12ee38c..0f507d528e 100644 --- a/test/network/emulated_network_manager.h +++ b/test/network/emulated_network_manager.h @@ -15,49 +15,45 @@ #include #include +#include "absl/base/nullability.h" #include "api/packet_socket_factory.h" +#include "api/task_queue/task_queue_base.h" #include "api/test/network_emulation/network_emulation_interfaces.h" #include "api/test/network_emulation_manager.h" #include "api/test/time_controller.h" +#include "p2p/base/basic_packet_socket_factory.h" #include "rtc_base/network.h" #include "rtc_base/socket_server.h" -#include "rtc_base/task_queue_for_test.h" -#include "rtc_base/third_party/sigslot/sigslot.h" #include "rtc_base/thread.h" -#include "rtc_base/thread_annotations.h" #include "test/network/network_emulation.h" namespace webrtc { namespace test { -// Framework assumes that rtc::NetworkManager is called from network thread. -class EmulatedNetworkManager : public rtc::NetworkManagerBase, - public sigslot::has_slots<>, - public EmulatedNetworkManagerInterface { +class EmulatedNetworkManager : public EmulatedNetworkManagerInterface { public: - EmulatedNetworkManager(TimeController* time_controller, - TaskQueueForTest* task_queue, - EndpointsContainer* endpoints_container); + EmulatedNetworkManager( + absl::Nonnull time_controller, + absl::Nonnull task_queue, + absl::Nonnull endpoints_container); + ~EmulatedNetworkManager() override; - void EnableEndpoint(EmulatedEndpointImpl* endpoint); - void DisableEndpoint(EmulatedEndpointImpl* endpoint); + void EnableEndpoint(absl::Nonnull endpoint); + void DisableEndpoint(absl::Nonnull endpoint); - // NetworkManager interface. All these methods are supposed to be called from - // the same thread. - void StartUpdating() override; - void StopUpdating() override; - - // We don't support any address interfaces in the network emulation framework. - std::vector GetAnyAddressNetworks() override { - return {}; + absl::Nonnull network_thread() override { + return network_thread_.get(); } - - // EmulatedNetworkManagerInterface API - rtc::Thread* network_thread() override { return network_thread_.get(); } - rtc::NetworkManager* network_manager() override { return this; } - rtc::PacketSocketFactory* packet_socket_factory() override { - return packet_socket_factory_.get(); + absl::Nonnull network_manager() override; + absl::Nonnull packet_socket_factory() override { + return &packet_socket_factory_; } + absl::Nonnull socket_factory() override { + return socket_server_; + } + absl::Nonnull> ReleaseNetworkManager() + override; + std::vector endpoints() const override { return endpoints_container_->GetEndpoints(); } @@ -65,19 +61,24 @@ class EmulatedNetworkManager : public rtc::NetworkManagerBase, std::function stats_callback) const override; private: - void UpdateNetworksOnce(); - void MaybeSignalNetworksChanged(); + class NetworkManagerImpl; + + const absl::Nonnull task_queue_; + const absl::Nonnull endpoints_container_; + + // Socket server is owned by the `network_thread_' + const absl::Nonnull socket_server_; - TaskQueueForTest* const task_queue_; - const EndpointsContainer* const endpoints_container_; // The `network_thread_` must outlive `packet_socket_factory_`, because they - // both refer to a socket server that is owned by `network_thread_`. Both - // pointers are assigned only in the constructor, but the way they are - // initialized unfortunately doesn't work with const std::unique_ptr<...>. - std::unique_ptr network_thread_; - std::unique_ptr packet_socket_factory_; - bool sent_first_update_ RTC_GUARDED_BY(network_thread_); - int start_count_ RTC_GUARDED_BY(network_thread_); + // both refer to the `socket_server_` that is owned by `network_thread_`. + const absl::Nonnull> network_thread_; + rtc::BasicPacketSocketFactory packet_socket_factory_; + absl::Nullable> network_manager_; + + // Keep pointer to the network manager when it is extracted to be injected + // into PeerConnectionFactory. That is brittle and may crash if a test would + // try to use emulated network after related PeerConnectionFactory is deleted. + const absl::Nonnull network_manager_ptr_; }; } // namespace test diff --git a/test/network/network_emulation_manager.cc b/test/network/network_emulation_manager.cc index 35127f67a2..f520310db9 100644 --- a/test/network/network_emulation_manager.cc +++ b/test/network/network_emulation_manager.cc @@ -19,6 +19,7 @@ #include #include +#include "absl/base/nullability.h" #include "api/array_view.h" #include "api/field_trials_view.h" #include "api/task_queue/task_queue_factory.h" @@ -31,6 +32,7 @@ #include "rtc_base/checks.h" #include "rtc_base/ip_address.h" #include "rtc_base/strings/string_builder.h" +#include "rtc_base/task_queue_for_test.h" #include "rtc_base/task_utils/repeating_task.h" #include "test/network/cross_traffic.h" #include "test/network/emulated_network_manager.h" @@ -298,7 +300,7 @@ void NetworkEmulationManagerImpl::StopCrossTraffic( }); } -EmulatedNetworkManagerInterface* +absl::Nonnull NetworkEmulationManagerImpl::CreateEmulatedNetworkManagerInterface( const std::vector& endpoints) { std::vector endpoint_impls; @@ -309,7 +311,7 @@ NetworkEmulationManagerImpl::CreateEmulatedNetworkManagerInterface( auto endpoints_container = std::make_unique( endpoint_impls, stats_gathering_mode_); auto network_manager = std::make_unique( - time_controller_.get(), &task_queue_, endpoints_container.get()); + time_controller_.get(), task_queue_.Get(), endpoints_container.get()); for (auto* endpoint : endpoints) { // Associate endpoint with network manager. bool insertion_result = diff --git a/test/network/network_emulation_manager.h b/test/network/network_emulation_manager.h index bba9094428..850d9e66fb 100644 --- a/test/network/network_emulation_manager.h +++ b/test/network/network_emulation_manager.h @@ -21,6 +21,7 @@ #include #include +#include "absl/base/nullability.h" #include "api/array_view.h" #include "api/test/network_emulation/cross_traffic.h" #include "api/test/network_emulation/network_emulation_interfaces.h" @@ -80,7 +81,8 @@ class NetworkEmulationManagerImpl : public NetworkEmulationManager { std::unique_ptr generator) override; void StopCrossTraffic(CrossTrafficGenerator* generator) override; - EmulatedNetworkManagerInterface* CreateEmulatedNetworkManagerInterface( + absl::Nonnull + CreateEmulatedNetworkManagerInterface( const std::vector& endpoints) override; void GetStats( diff --git a/test/network/network_emulation_pc_unittest.cc b/test/network/network_emulation_pc_unittest.cc index 298e9534f9..331f7fef2b 100644 --- a/test/network/network_emulation_pc_unittest.cc +++ b/test/network/network_emulation_pc_unittest.cc @@ -16,7 +16,6 @@ #include "api/enable_media_with_defaults.h" #include "api/jsep.h" #include "api/media_stream_interface.h" -#include "api/packet_socket_factory.h" #include "api/peer_connection_interface.h" #include "api/rtc_event_log/rtc_event_log_factory.h" #include "api/scoped_refptr.h" @@ -28,7 +27,6 @@ #include "api/transport/field_trial_based_config.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/peer_connection_wrapper.h" #include "pc/test/mock_peer_connection_observers.h" #include "rtc_base/network.h" @@ -64,13 +62,16 @@ bool AddIceCandidates(PeerConnectionWrapper* peer, rtc::scoped_refptr CreatePeerConnectionFactory( rtc::Thread* signaling_thread, - rtc::Thread* network_thread) { + EmulatedNetworkManagerInterface* network) { PeerConnectionFactoryDependencies pcf_deps; pcf_deps.task_queue_factory = CreateDefaultTaskQueueFactory(); pcf_deps.event_log_factory = std::make_unique(); - pcf_deps.network_thread = network_thread; + pcf_deps.network_thread = network->network_thread(); pcf_deps.signaling_thread = signaling_thread; pcf_deps.trials = std::make_unique(); + pcf_deps.socket_factory = network->socket_factory(); + pcf_deps.network_manager = network->ReleaseNetworkManager(); + pcf_deps.adm = TestAudioDeviceModule::Create( pcf_deps.task_queue_factory.get(), TestAudioDeviceModule::CreatePulsedNoiseCapturer(kMaxAptitude, @@ -84,20 +85,13 @@ rtc::scoped_refptr CreatePeerConnectionFactory( rtc::scoped_refptr CreatePeerConnection( const rtc::scoped_refptr& pcf, PeerConnectionObserver* observer, - rtc::PacketSocketFactory* packet_socket_factory, - rtc::NetworkManager* network_manager, EmulatedTURNServerInterface* turn_server = nullptr) { PeerConnectionDependencies pc_deps(observer); - auto port_allocator = std::make_unique( - network_manager, packet_socket_factory); - - // This test does not support TCP - int flags = cricket::PORTALLOCATOR_DISABLE_TCP; - port_allocator->set_flags(port_allocator->flags() | flags); - - pc_deps.allocator = std::move(port_allocator); PeerConnectionInterface::RTCConfiguration rtc_configuration; rtc_configuration.sdp_semantics = SdpSemantics::kUnifiedPlan; + // This test does not support TCP + rtc_configuration.port_allocator_config.flags = + cricket::PORTALLOCATOR_DISABLE_TCP; if (turn_server != nullptr) { webrtc::PeerConnectionInterface::IceServer server; server.username = turn_server->GetIceServerConfig().username; @@ -152,17 +146,12 @@ TEST(NetworkEmulationManagerPCTest, Run) { std::make_unique(); SendTask(signaling_thread.get(), [&]() { - alice_pcf = CreatePeerConnectionFactory(signaling_thread.get(), - alice_network->network_thread()); - alice_pc = CreatePeerConnection(alice_pcf, alice_observer.get(), - alice_network->packet_socket_factory(), - alice_network->network_manager()); + alice_pcf = + CreatePeerConnectionFactory(signaling_thread.get(), alice_network); + alice_pc = CreatePeerConnection(alice_pcf, alice_observer.get()); - bob_pcf = CreatePeerConnectionFactory(signaling_thread.get(), - bob_network->network_thread()); - bob_pc = CreatePeerConnection(bob_pcf, bob_observer.get(), - bob_network->packet_socket_factory(), - bob_network->network_manager()); + bob_pcf = CreatePeerConnectionFactory(signaling_thread.get(), bob_network); + bob_pc = CreatePeerConnection(bob_pcf, bob_observer.get()); }); std::unique_ptr alice = @@ -267,17 +256,13 @@ TEST(NetworkEmulationManagerPCTest, RunTURN) { std::make_unique(); SendTask(signaling_thread.get(), [&]() { - alice_pcf = CreatePeerConnectionFactory(signaling_thread.get(), - alice_network->network_thread()); - alice_pc = CreatePeerConnection( - alice_pcf, alice_observer.get(), alice_network->packet_socket_factory(), - alice_network->network_manager(), alice_turn); + alice_pcf = + CreatePeerConnectionFactory(signaling_thread.get(), alice_network); + alice_pc = + CreatePeerConnection(alice_pcf, alice_observer.get(), alice_turn); - bob_pcf = CreatePeerConnectionFactory(signaling_thread.get(), - bob_network->network_thread()); - bob_pc = CreatePeerConnection(bob_pcf, bob_observer.get(), - bob_network->packet_socket_factory(), - bob_network->network_manager(), bob_turn); + bob_pcf = CreatePeerConnectionFactory(signaling_thread.get(), bob_network); + bob_pc = CreatePeerConnection(bob_pcf, bob_observer.get(), bob_turn); }); std::unique_ptr alice =