From 4765013541e2362620115c727faffded2e025083 Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Fri, 1 Mar 2019 15:12:22 +0100 Subject: [PATCH] Intermediate step: Move ownership of rtc::NetworkManager to test code from PC E2E framework Bug: webrtc:10138 Change-Id: I9b751a1c28d8533cce238d64b8f8c76eabdab5eb Reviewed-on: https://webrtc-review.googlesource.com/c/125182 Reviewed-by: Peter Slatala Reviewed-by: Mirko Bonadei Commit-Queue: Artem Titov Cr-Commit-Position: refs/heads/master@{#26927} --- .../api/peerconnection_quality_test_fixture.h | 13 +++++++-- test/pc/e2e/peer_connection_e2e_smoke_test.cc | 12 ++++---- test/pc/e2e/test_peer.cc | 28 ++++--------------- test/pc/e2e/test_peer.h | 7 +---- 4 files changed, 23 insertions(+), 37 deletions(-) diff --git a/test/pc/e2e/api/peerconnection_quality_test_fixture.h b/test/pc/e2e/api/peerconnection_quality_test_fixture.h index 4c6aca583a..da8ab20263 100644 --- a/test/pc/e2e/api/peerconnection_quality_test_fixture.h +++ b/test/pc/e2e/api/peerconnection_quality_test_fixture.h @@ -73,7 +73,12 @@ class PeerConnectionE2EQualityTestFixture { // so client can't inject its own. Also only network manager can be overridden // inside port allocator. struct PeerConnectionComponents { - std::unique_ptr network_manager; + PeerConnectionComponents(rtc::NetworkManager* network_manager) + : network_manager(network_manager) { + RTC_CHECK(network_manager); + } + + rtc::NetworkManager* const network_manager; std::unique_ptr async_resolver_factory; std::unique_ptr cert_generator; std::unique_ptr tls_cert_verifier; @@ -82,11 +87,13 @@ class PeerConnectionE2EQualityTestFixture { // Contains all components, that can be overridden in peer connection. Also // has a network thread, that will be used to communicate with another peers. struct InjectableComponents { - explicit InjectableComponents(rtc::Thread* network_thread) + explicit InjectableComponents(rtc::Thread* network_thread, + rtc::NetworkManager* network_manager) : network_thread(network_thread), pcf_dependencies( absl::make_unique()), - pc_dependencies(absl::make_unique()) { + pc_dependencies( + absl::make_unique(network_manager)) { RTC_CHECK(network_thread); } diff --git a/test/pc/e2e/peer_connection_e2e_smoke_test.cc b/test/pc/e2e/peer_connection_e2e_smoke_test.cc index b38ea534d8..6dc5539bda 100644 --- a/test/pc/e2e/peer_connection_e2e_smoke_test.cc +++ b/test/pc/e2e/peer_connection_e2e_smoke_test.cc @@ -96,14 +96,14 @@ TEST(PeerConnectionE2EQualityTestSmokeTest, RunWithEmulatedNetwork) { // Setup components. We need to provide rtc::NetworkManager compatible with // emulated network layer. - auto alice_components = - absl::make_unique(alice_network_thread); - alice_components->pc_dependencies->network_manager = + std::unique_ptr alice_network_manager = CreateFakeNetworkManager({alice_endpoint}); - auto bob_components = - absl::make_unique(bob_network_thread); - bob_components->pc_dependencies->network_manager = + auto alice_components = absl::make_unique( + alice_network_thread, alice_network_manager.get()); + std::unique_ptr bob_network_manager = CreateFakeNetworkManager({bob_endpoint}); + auto bob_components = absl::make_unique( + bob_network_thread, bob_network_manager.get()); // Create analyzers. std::unique_ptr video_quality_analyzer = diff --git a/test/pc/e2e/test_peer.cc b/test/pc/e2e/test_peer.cc index 208e35767a..eca404c517 100644 --- a/test/pc/e2e/test_peer.cc +++ b/test/pc/e2e/test_peer.cc @@ -47,13 +47,8 @@ using AudioConfig = PeerConnectionE2EQualityTestFixture::AudioConfig; // dependencies, that won't be specially provided by factory and will be just // transferred to peer connection creation code. void SetMandatoryEntities(InjectableComponents* components) { - if (components->pcf_dependencies == nullptr) { - components->pcf_dependencies = - absl::make_unique(); - } - if (components->pc_dependencies == nullptr) { - components->pc_dependencies = absl::make_unique(); - } + RTC_DCHECK(components->pcf_dependencies); + RTC_DCHECK(components->pc_dependencies); // Setup required peer connection factory dependencies. if (components->pcf_dependencies->call_factory == nullptr) { @@ -209,16 +204,8 @@ PeerConnectionDependencies CreatePCDependencies( PeerConnectionObserver* observer) { PeerConnectionDependencies pc_deps(observer); - // We need to create network manager, because it is required for port - // allocator. TestPeer will take ownership of this object and will store it - // until the end of the test. - if (pc_dependencies->network_manager == nullptr) { - pc_dependencies->network_manager = - // Use real network (on the loopback interface) - absl::make_unique(); - } auto port_allocator = absl::make_unique( - pc_dependencies->network_manager.get()); + pc_dependencies->network_manager); // This test does not support TCP int flags = cricket::PORTALLOCATOR_DISABLE_TCP; @@ -281,8 +268,7 @@ std::unique_ptr TestPeer::CreateTestPeer( pcf->CreatePeerConnection(params->rtc_configuration, std::move(pc_deps)); return absl::WrapUnique( - new TestPeer(pcf, pc, std::move(observer), std::move(params), - std::move(components->pc_dependencies->network_manager))); + new TestPeer(pcf, pc, std::move(observer), std::move(params))); } bool TestPeer::AddIceCandidates( @@ -305,13 +291,11 @@ TestPeer::TestPeer( rtc::scoped_refptr pc_factory, rtc::scoped_refptr pc, std::unique_ptr observer, - std::unique_ptr params, - std::unique_ptr network_manager) + std::unique_ptr params) : PeerConnectionWrapper::PeerConnectionWrapper(std::move(pc_factory), std::move(pc), std::move(observer)), - params_(std::move(params)), - network_manager_(std::move(network_manager)) {} + params_(std::move(params)) {} } // namespace test } // namespace webrtc diff --git a/test/pc/e2e/test_peer.h b/test/pc/e2e/test_peer.h index 22efa14212..86f7f6a20f 100644 --- a/test/pc/e2e/test_peer.h +++ b/test/pc/e2e/test_peer.h @@ -67,14 +67,9 @@ class TestPeer final : public PeerConnectionWrapper { TestPeer(rtc::scoped_refptr pc_factory, rtc::scoped_refptr pc, std::unique_ptr observer, - std::unique_ptr params, - std::unique_ptr network_manager); + std::unique_ptr params); std::unique_ptr params_; - // Test peer will take ownership of network manager and keep it during the - // call. Network manager will be deleted before peer connection, but - // connection will be closed before destruction, so it should be ok. - std::unique_ptr network_manager_; }; } // namespace test