Deprecate AsyncResolver config fields and remove internal usage.

Bug: webrtc:12598
Change-Id: Ic43cbcd13e4de44b02351c89da12844606368623
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/317604
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40627}
This commit is contained in:
Harald Alvestrand 2023-08-25 11:07:28 +00:00 committed by WebRTC LUCI CQ
parent 92fe5c4db3
commit 4d25a77fd3
17 changed files with 91 additions and 46 deletions

View File

@ -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;

View File

@ -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")

View File

@ -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;

View File

@ -1386,6 +1386,9 @@ struct RTC_EXPORT PeerConnectionDependencies final {
std::unique_ptr<webrtc::AsyncDnsResolverFactoryInterface>
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<webrtc::AsyncResolverFactory> async_resolver_factory;
std::unique_ptr<webrtc::IceTransportFactory> ice_transport_factory;
std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator;

View File

@ -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",

View File

@ -15,7 +15,7 @@
#include <string>
#include <vector>
#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<webrtc::AsyncResolverFactory> async_resolver_factory;
std::unique_ptr<webrtc::AsyncDnsResolverFactoryInterface>
async_dns_resolver_factory;
std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator;
std::unique_ptr<rtc::SSLCertificateVerifier> tls_cert_verifier;
std::unique_ptr<IceTransportFactory> ice_transport_factory;

View File

@ -86,10 +86,11 @@ PeerConfigurer* PeerConfigurer::SetAudioDecoderFactory(
components_->pcf_dependencies->audio_decoder_factory = audio_decoder_factory;
return this;
}
PeerConfigurer* PeerConfigurer::SetAsyncResolverFactory(
std::unique_ptr<webrtc::AsyncResolverFactory> async_resolver_factory) {
components_->pc_dependencies->async_resolver_factory =
std::move(async_resolver_factory);
PeerConfigurer* PeerConfigurer::SetAsyncDnsResolverFactory(
std::unique_ptr<webrtc::AsyncDnsResolverFactoryInterface>
async_dns_resolver_factory) {
components_->pc_dependencies->async_dns_resolver_factory =
std::move(async_dns_resolver_factory);
return this;
}
PeerConfigurer* PeerConfigurer::SetRTCCertificateGenerator(

View File

@ -16,7 +16,7 @@
#include <vector>
#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<webrtc::AsyncResolverFactory> async_resolver_factory);
PeerConfigurer* SetAsyncDnsResolverFactory(
std::unique_ptr<webrtc::AsyncDnsResolverFactoryInterface>
async_resolver_factory);
PeerConfigurer* SetRTCCertificateGenerator(
std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator);
PeerConfigurer* SetSSLCertificateVerifier(

View File

@ -118,6 +118,10 @@ std::unique_ptr<P2PTransportChannel> 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> 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(),

View File

@ -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",

View File

@ -464,6 +464,8 @@ RTCErrorOr<rtc::scoped_refptr<PeerConnection>> 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<rtc::scoped_refptr<PeerConnection>> 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<WrappingAsyncDnsResolverFactory>(
std::move(dependencies.async_resolver_factory));
} else {
dependencies.async_dns_resolver_factory =
std::make_unique<BasicAsyncDnsResolverFactory>();
if (!dependencies.async_dns_resolver_factory) {
if (dependencies.async_resolver_factory) {
dependencies.async_dns_resolver_factory =
std::make_unique<WrappingAsyncDnsResolverFactory>(
std::move(dependencies.async_resolver_factory));
} else {
dependencies.async_dns_resolver_factory =
std::make_unique<BasicAsyncDnsResolverFactory>();
}
}
#pragma clang diagnostic pop
// The PeerConnection constructor consumes some, but not all, dependencies.
auto pc = rtc::make_ref_counted<PeerConnection>(

View File

@ -224,11 +224,6 @@ PeerConnectionFactory::CreatePeerConnectionOrError(
configuration.port_allocator_config.flags);
}
if (!dependencies.async_resolver_factory) {
dependencies.async_resolver_factory =
std::make_unique<webrtc::BasicAsyncResolverFactory>();
}
if (!dependencies.ice_transport_factory) {
dependencies.ice_transport_factory =
std::make_unique<DefaultIceTransportFactory>();

View File

@ -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<NiceMock<webrtc::MockAsyncResolverFactory>>();
std::make_unique<NiceMock<webrtc::MockAsyncDnsResolverFactory>>();
webrtc::PeerConnectionDependencies deps(nullptr /* observer_in */);
@ -273,7 +274,7 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
fake_network,
std::make_unique<rtc::BasicPacketSocketFactory>(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(

View File

@ -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<NiceMock<webrtc::MockAsyncResolverFactory>>();
std::make_unique<NiceMock<webrtc::MockAsyncDnsResolverFactory>>();
auto callee_resolver_factory =
std::make_unique<NiceMock<webrtc::MockAsyncResolverFactory>>();
NiceMock<rtc::MockAsyncResolver> callee_async_resolver;
NiceMock<rtc::MockAsyncResolver> caller_async_resolver;
std::make_unique<NiceMock<webrtc::MockAsyncDnsResolverFactory>>();
auto callee_async_resolver =
std::make_unique<NiceMock<MockAsyncDnsResolver>>();
auto caller_async_resolver =
std::make_unique<NiceMock<MockAsyncDnsResolver>>();
// 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(

View File

@ -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<void(cricket::SessionDescription*)> received_sdp_munger_;
std::function<void(cricket::SessionDescription*)> generated_sdp_munger_;
std::function<void()> remote_offer_handler_;
rtc::MockAsyncResolver* remote_async_resolver_ = nullptr;
MockAsyncDnsResolver* remote_async_dns_resolver_ = nullptr;
// Result variables for the mock DNS resolver
NiceMock<MockAsyncDnsResolverResult> remote_async_dns_resolver_result_;
rtc::SocketAddress remote_async_dns_resolved_addr_;
// All data channels either created or observed on this peerconnection
std::vector<rtc::scoped_refptr<DataChannelInterface>> data_channels_;
std::vector<std::unique_ptr<MockDataChannelObserver>> data_observers_;

View File

@ -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();

View File

@ -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);