diff --git a/api/async_resolver_factory.h b/api/async_resolver_factory.h index 93d3f7934b..ffa958268d 100644 --- a/api/async_resolver_factory.h +++ b/api/async_resolver_factory.h @@ -18,6 +18,7 @@ namespace webrtc { // An abstract factory for creating AsyncResolverInterfaces. This allows // client applications to provide WebRTC with their own mechanism for // performing DNS resolution. +// TODO(bugs.webrtc.org/12598): Deprecate and remove. class AsyncResolverFactory { public: AsyncResolverFactory() = default; diff --git a/api/ice_transport_interface.h b/api/ice_transport_interface.h index 6aae38fc3b..431f3330a5 100644 --- a/api/ice_transport_interface.h +++ b/api/ice_transport_interface.h @@ -64,7 +64,8 @@ struct IceTransportInit final { RTC_DCHECK(!async_resolver_factory_); async_dns_resolver_factory_ = async_dns_resolver_factory; } - AsyncResolverFactory* async_resolver_factory() { + [[deprecated("Use async_dns_resolver_factory")]] AsyncResolverFactory* + async_resolver_factory() { return async_resolver_factory_; } ABSL_DEPRECATED("bugs.webrtc.org/12598") diff --git a/api/peer_connection_interface.cc b/api/peer_connection_interface.cc index d01d58d32b..050b61da95 100644 --- a/api/peer_connection_interface.cc +++ b/api/peer_connection_interface.cc @@ -45,8 +45,13 @@ PeerConnectionDependencies::PeerConnectionDependencies( PeerConnectionObserver* observer_in) : observer(observer_in) {} +// TODO(bugs.webrtc.org/12598: remove pragma once async_resolver_factory +// is removed from PeerConnectionDependencies +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" PeerConnectionDependencies::PeerConnectionDependencies( PeerConnectionDependencies&&) = default; +#pragma clang diagnostic pop PeerConnectionDependencies::~PeerConnectionDependencies() = default; diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index e80550cb07..37dcfbbc27 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -1386,6 +1386,9 @@ struct RTC_EXPORT PeerConnectionDependencies final { std::unique_ptr async_dns_resolver_factory; // Deprecated - use async_dns_resolver_factory + // Deprecation is in abeyance until Chromium is updated. + // TODO(crbug.com/1475925): Deprecate once Chromium is updated + // [[deprecated("Use async_dns_resolver_factory")]] std::unique_ptr async_resolver_factory; std::unique_ptr ice_transport_factory; std::unique_ptr cert_generator; diff --git a/api/test/pclf/BUILD.gn b/api/test/pclf/BUILD.gn index 0526478e8b..f3d78370ed 100644 --- a/api/test/pclf/BUILD.gn +++ b/api/test/pclf/BUILD.gn @@ -65,6 +65,7 @@ rtc_library("media_quality_test_params") { deps = [ ":media_configuration", + "../..:async_dns_resolver", "../../../api:callfactory_api", "../../../api:fec_controller_api", "../../../api:field_trials_view", @@ -94,6 +95,7 @@ rtc_library("peer_configurer") { deps = [ ":media_configuration", ":media_quality_test_params", + "../..:async_dns_resolver", "../../../api:callfactory_api", "../../../api:create_peer_connection_quality_test_frame_generator", "../../../api:fec_controller_api", diff --git a/api/test/pclf/media_quality_test_params.h b/api/test/pclf/media_quality_test_params.h index 2485cfca00..a247f342b0 100644 --- a/api/test/pclf/media_quality_test_params.h +++ b/api/test/pclf/media_quality_test_params.h @@ -15,7 +15,7 @@ #include #include -#include "api/async_resolver_factory.h" +#include "api/async_dns_resolver.h" #include "api/audio/audio_mixer.h" #include "api/call/call_factory_interface.h" #include "api/fec_controller.h" @@ -85,7 +85,8 @@ struct PeerConnectionComponents { rtc::NetworkManager* const network_manager; rtc::PacketSocketFactory* const packet_socket_factory; - std::unique_ptr async_resolver_factory; + std::unique_ptr + async_dns_resolver_factory; std::unique_ptr cert_generator; std::unique_ptr tls_cert_verifier; std::unique_ptr ice_transport_factory; diff --git a/api/test/pclf/peer_configurer.cc b/api/test/pclf/peer_configurer.cc index 7acaddadda..b614940c99 100644 --- a/api/test/pclf/peer_configurer.cc +++ b/api/test/pclf/peer_configurer.cc @@ -86,10 +86,11 @@ PeerConfigurer* PeerConfigurer::SetAudioDecoderFactory( components_->pcf_dependencies->audio_decoder_factory = audio_decoder_factory; return this; } -PeerConfigurer* PeerConfigurer::SetAsyncResolverFactory( - std::unique_ptr async_resolver_factory) { - components_->pc_dependencies->async_resolver_factory = - std::move(async_resolver_factory); +PeerConfigurer* PeerConfigurer::SetAsyncDnsResolverFactory( + std::unique_ptr + async_dns_resolver_factory) { + components_->pc_dependencies->async_dns_resolver_factory = + std::move(async_dns_resolver_factory); return this; } PeerConfigurer* PeerConfigurer::SetRTCCertificateGenerator( diff --git a/api/test/pclf/peer_configurer.h b/api/test/pclf/peer_configurer.h index 3596bb67ab..d10b53fa3d 100644 --- a/api/test/pclf/peer_configurer.h +++ b/api/test/pclf/peer_configurer.h @@ -16,7 +16,7 @@ #include #include "absl/strings/string_view.h" -#include "api/async_resolver_factory.h" +#include "api/async_dns_resolver.h" #include "api/audio/audio_mixer.h" #include "api/call/call_factory_interface.h" #include "api/fec_controller.h" @@ -88,8 +88,9 @@ class PeerConfigurer { // The parameters of the following 4 methods will be passed to the // PeerConnectionInterface implementation that will be created for this // peer. - PeerConfigurer* SetAsyncResolverFactory( - std::unique_ptr async_resolver_factory); + PeerConfigurer* SetAsyncDnsResolverFactory( + std::unique_ptr + async_resolver_factory); PeerConfigurer* SetRTCCertificateGenerator( std::unique_ptr cert_generator); PeerConfigurer* SetSSLCertificateVerifier( diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index d916983fe2..17b59680e1 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -118,6 +118,10 @@ std::unique_ptr P2PTransportChannel::Create( absl::string_view transport_name, int component, webrtc::IceTransportInit init) { + // TODO(bugs.webrtc.org/12598): Remove pragma and fallback once + // async_resolver_factory is gone +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" if (init.async_resolver_factory()) { return absl::WrapUnique(new P2PTransportChannel( transport_name, component, init.port_allocator(), nullptr, @@ -125,6 +129,7 @@ std::unique_ptr P2PTransportChannel::Create( init.async_resolver_factory()), init.event_log(), init.ice_controller_factory(), init.active_ice_controller_factory(), init.field_trials())); +#pragma clang diagnostic pop } else { return absl::WrapUnique(new P2PTransportChannel( transport_name, component, init.port_allocator(), diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 60006c4981..1f830072f3 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -2413,6 +2413,7 @@ if (rtc_include_tests && !build_with_chromium) { "../api:libjingle_peerconnection_api", "../api:make_ref_counted", "../api:media_stream_interface", + "../api:mock_async_dns_resolver", "../api:mock_encoder_selector", "../api:mock_packet_socket_factory", "../api:mock_video_track", @@ -2636,6 +2637,7 @@ if (rtc_include_tests && !build_with_chromium) { "../api:libjingle_logging_api", "../api:libjingle_peerconnection_api", "../api:media_stream_interface", + "../api:mock_async_dns_resolver", "../api:mock_rtp", "../api:packet_socket_factory", "../api:rtc_error", diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 767e5a4145..cde3d91047 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -464,6 +464,8 @@ RTCErrorOr> PeerConnection::Create( // If neither is given, create a BasicAsyncDnsResolverFactory. // TODO(bugs.webrtc.org/12598): Remove code once all callers pass a // AsyncDnsResolverFactory. +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" if (dependencies.async_dns_resolver_factory && dependencies.async_resolver_factory) { RTC_LOG(LS_ERROR) @@ -471,14 +473,17 @@ RTCErrorOr> PeerConnection::Create( return RTCError(RTCErrorType::INVALID_PARAMETER, "Both old and new type of DNS resolver given"); } - if (dependencies.async_resolver_factory) { - dependencies.async_dns_resolver_factory = - std::make_unique( - std::move(dependencies.async_resolver_factory)); - } else { - dependencies.async_dns_resolver_factory = - std::make_unique(); + if (!dependencies.async_dns_resolver_factory) { + if (dependencies.async_resolver_factory) { + dependencies.async_dns_resolver_factory = + std::make_unique( + std::move(dependencies.async_resolver_factory)); + } else { + dependencies.async_dns_resolver_factory = + std::make_unique(); + } } +#pragma clang diagnostic pop // The PeerConnection constructor consumes some, but not all, dependencies. auto pc = rtc::make_ref_counted( diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc index 54fabce6ac..d933ba6aea 100644 --- a/pc/peer_connection_factory.cc +++ b/pc/peer_connection_factory.cc @@ -224,11 +224,6 @@ PeerConnectionFactory::CreatePeerConnectionOrError( configuration.port_allocator_config.flags); } - if (!dependencies.async_resolver_factory) { - dependencies.async_resolver_factory = - std::make_unique(); - } - if (!dependencies.ice_transport_factory) { dependencies.ice_transport_factory = std::make_unique(); diff --git a/pc/peer_connection_histogram_unittest.cc b/pc/peer_connection_histogram_unittest.cc index 656b022ebd..0efc1c5744 100644 --- a/pc/peer_connection_histogram_unittest.cc +++ b/pc/peer_connection_histogram_unittest.cc @@ -24,6 +24,7 @@ #include "api/scoped_refptr.h" #include "api/task_queue/default_task_queue_factory.h" #include "api/task_queue/task_queue_factory.h" +#include "api/test/mock_async_dns_resolver.h" #include "media/base/fake_media_engine.h" #include "media/base/media_engine.h" #include "p2p/base/mock_async_resolver.h" @@ -259,7 +260,7 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test { WrapperPtr CreatePeerConnectionWithMdns(const RTCConfiguration& config) { auto resolver_factory = - std::make_unique>(); + std::make_unique>(); webrtc::PeerConnectionDependencies deps(nullptr /* observer_in */); @@ -273,7 +274,7 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test { fake_network, std::make_unique(vss_.get()))); - deps.async_resolver_factory = std::move(resolver_factory); + deps.async_dns_resolver_factory = std::move(resolver_factory); deps.allocator = std::move(port_allocator); return CreatePeerConnection( diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index 58d3c9762e..d76e5e27d5 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -50,6 +50,7 @@ #include "api/stats/rtc_stats.h" #include "api/stats/rtc_stats_report.h" #include "api/stats/rtcstats_objects.h" +#include "api/test/mock_async_dns_resolver.h" #include "api/test/mock_encoder_selector.h" #include "api/transport/rtp/rtp_source.h" #include "api/uma_metrics.h" @@ -1678,23 +1679,29 @@ constexpr int kOnlyLocalPorts = cricket::PORTALLOCATOR_DISABLE_STUN | TEST_P(PeerConnectionIntegrationTest, IceStatesReachCompletionWithRemoteHostname) { auto caller_resolver_factory = - std::make_unique>(); + std::make_unique>(); auto callee_resolver_factory = - std::make_unique>(); - NiceMock callee_async_resolver; - NiceMock caller_async_resolver; + std::make_unique>(); + auto callee_async_resolver = + std::make_unique>(); + auto caller_async_resolver = + std::make_unique>(); + // Keep raw pointers to the mock resolvers, for use after init, + // where the std::unique_ptr values have been moved away. + auto* callee_resolver_ptr = callee_async_resolver.get(); + auto* caller_resolver_ptr = caller_async_resolver.get(); // This also verifies that the injected AsyncResolverFactory is used by // P2PTransportChannel. EXPECT_CALL(*caller_resolver_factory, Create()) - .WillOnce(Return(&caller_async_resolver)); + .WillOnce(Return(ByMove(std::move(caller_async_resolver)))); webrtc::PeerConnectionDependencies caller_deps(nullptr); - caller_deps.async_resolver_factory = std::move(caller_resolver_factory); + caller_deps.async_dns_resolver_factory = std::move(caller_resolver_factory); EXPECT_CALL(*callee_resolver_factory, Create()) - .WillOnce(Return(&callee_async_resolver)); + .WillOnce(Return(ByMove(std::move(callee_async_resolver)))); webrtc::PeerConnectionDependencies callee_deps(nullptr); - callee_deps.async_resolver_factory = std::move(callee_resolver_factory); + callee_deps.async_dns_resolver_factory = std::move(callee_resolver_factory); PeerConnectionInterface::RTCConfiguration config; config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; @@ -1703,8 +1710,12 @@ TEST_P(PeerConnectionIntegrationTest, ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndDeps( config, std::move(caller_deps), config, std::move(callee_deps))); - caller()->SetRemoteAsyncResolver(&callee_async_resolver); - callee()->SetRemoteAsyncResolver(&caller_async_resolver); + // TEMP NOTE - this is probably bogus since the resolvers have been + // moved out of their slots prior to these code lines. + RTC_LOG(LS_ERROR) << "callee async resolver is " + << callee_async_resolver.get(); + caller()->SetRemoteAsyncResolver(callee_resolver_ptr); + callee()->SetRemoteAsyncResolver(caller_resolver_ptr); // Enable hostname candidates with mDNS names. caller()->SetMdnsResponder( diff --git a/pc/test/integration_test_helpers.h b/pc/test/integration_test_helpers.h index 889161e797..740e063271 100644 --- a/pc/test/integration_test_helpers.h +++ b/pc/test/integration_test_helpers.h @@ -55,6 +55,7 @@ #include "api/task_queue/default_task_queue_factory.h" #include "api/task_queue/pending_task_safety_flag.h" #include "api/task_queue/task_queue_factory.h" +#include "api/test/mock_async_dns_resolver.h" #include "api/transport/field_trial_based_config.h" #include "api/uma_metrics.h" #include "api/units/time_delta.h" @@ -134,6 +135,7 @@ using ::testing::Combine; using ::testing::Contains; using ::testing::DoAll; using ::testing::ElementsAre; +using ::testing::InvokeArgument; using ::testing::NiceMock; using ::testing::Return; using ::testing::SetArgPointee; @@ -279,8 +281,8 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver, remote_offer_handler_ = std::move(handler); } - void SetRemoteAsyncResolver(rtc::MockAsyncResolver* resolver) { - remote_async_resolver_ = resolver; + void SetRemoteAsyncResolver(MockAsyncDnsResolver* resolver) { + remote_async_dns_resolver_ = resolver; } // Every ICE connection state in order that has been seen by the observer. @@ -1120,18 +1122,22 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver, void OnIceCandidate(const webrtc::IceCandidateInterface* candidate) override { RTC_LOG(LS_INFO) << debug_name_ << ": OnIceCandidate"; - if (remote_async_resolver_) { + if (remote_async_dns_resolver_) { const auto& local_candidate = candidate->candidate(); if (local_candidate.address().IsUnresolvedIP()) { RTC_DCHECK(local_candidate.type() == cricket::LOCAL_PORT_TYPE); - rtc::SocketAddress resolved_addr(local_candidate.address()); const auto resolved_ip = mdns_responder_->GetMappedAddressForName( local_candidate.address().hostname()); RTC_DCHECK(!resolved_ip.IsNil()); - resolved_addr.SetResolvedIP(resolved_ip); - EXPECT_CALL(*remote_async_resolver_, GetResolvedAddress(_, _)) - .WillOnce(DoAll(SetArgPointee<1>(resolved_addr), Return(true))); - EXPECT_CALL(*remote_async_resolver_, Destroy(_)); + remote_async_dns_resolved_addr_ = local_candidate.address(); + remote_async_dns_resolved_addr_.SetResolvedIP(resolved_ip); + EXPECT_CALL(*remote_async_dns_resolver_, Start(_, _)) + .WillOnce(InvokeArgument<1>()); + EXPECT_CALL(*remote_async_dns_resolver_, result()) + .WillOnce(ReturnRef(remote_async_dns_resolver_result_)); + EXPECT_CALL(remote_async_dns_resolver_result_, GetResolvedAddress(_, _)) + .WillOnce(DoAll(SetArgPointee<1>(remote_async_dns_resolved_addr_), + Return(true))); } } @@ -1202,7 +1208,11 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver, std::function received_sdp_munger_; std::function generated_sdp_munger_; std::function remote_offer_handler_; - rtc::MockAsyncResolver* remote_async_resolver_ = nullptr; + MockAsyncDnsResolver* remote_async_dns_resolver_ = nullptr; + // Result variables for the mock DNS resolver + NiceMock remote_async_dns_resolver_result_; + rtc::SocketAddress remote_async_dns_resolved_addr_; + // All data channels either created or observed on this peerconnection std::vector> data_channels_; std::vector> data_observers_; diff --git a/rtc_base/async_resolver_interface.h b/rtc_base/async_resolver_interface.h index 829dc7e23b..851fa38ce1 100644 --- a/rtc_base/async_resolver_interface.h +++ b/rtc_base/async_resolver_interface.h @@ -19,6 +19,7 @@ namespace rtc { // This interface defines the methods to resolve the address asynchronously. +// TODO(bugs.webrtc.org/12598): Deprecate and remove. class RTC_EXPORT AsyncResolverInterface { public: AsyncResolverInterface(); diff --git a/test/pc/e2e/test_peer_factory.cc b/test/pc/e2e/test_peer_factory.cc index 858676fd7a..9b2f2d6953 100644 --- a/test/pc/e2e/test_peer_factory.cc +++ b/test/pc/e2e/test_peer_factory.cc @@ -261,9 +261,9 @@ PeerConnectionDependencies CreatePCDependencies( pc_deps.allocator = std::move(port_allocator); - if (pc_dependencies->async_resolver_factory != nullptr) { - pc_deps.async_resolver_factory = - std::move(pc_dependencies->async_resolver_factory); + if (pc_dependencies->async_dns_resolver_factory != nullptr) { + pc_deps.async_dns_resolver_factory = + std::move(pc_dependencies->async_dns_resolver_factory); } if (pc_dependencies->cert_generator != nullptr) { pc_deps.cert_generator = std::move(pc_dependencies->cert_generator);