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}
diff --git a/api/async_resolver_factory.h b/api/async_resolver_factory.h
index 93d3f79..ffa9582 100644
--- a/api/async_resolver_factory.h
+++ b/api/async_resolver_factory.h
@@ -18,6 +18,7 @@
// 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 6aae38f..431f333 100644
--- a/api/ice_transport_interface.h
+++ b/api/ice_transport_interface.h
@@ -64,7 +64,8 @@
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 d01d58d..050b61d 100644
--- a/api/peer_connection_interface.cc
+++ b/api/peer_connection_interface.cc
@@ -45,8 +45,13 @@
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 e80550c..37dcfbb 100644
--- a/api/peer_connection_interface.h
+++ b/api/peer_connection_interface.h
@@ -1386,6 +1386,9 @@
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;
diff --git a/api/test/pclf/BUILD.gn b/api/test/pclf/BUILD.gn
index 0526478..f3d7837 100644
--- a/api/test/pclf/BUILD.gn
+++ b/api/test/pclf/BUILD.gn
@@ -65,6 +65,7 @@
deps = [
":media_configuration",
+ "../..:async_dns_resolver",
"../../../api:callfactory_api",
"../../../api:fec_controller_api",
"../../../api:field_trials_view",
@@ -94,6 +95,7 @@
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 2485cfc..a247f34 100644
--- a/api/test/pclf/media_quality_test_params.h
+++ b/api/test/pclf/media_quality_test_params.h
@@ -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 @@
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;
diff --git a/api/test/pclf/peer_configurer.cc b/api/test/pclf/peer_configurer.cc
index 7acadda..b614940 100644
--- a/api/test/pclf/peer_configurer.cc
+++ b/api/test/pclf/peer_configurer.cc
@@ -86,10 +86,11 @@
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(
diff --git a/api/test/pclf/peer_configurer.h b/api/test/pclf/peer_configurer.h
index 3596bb6..d10b53f 100644
--- a/api/test/pclf/peer_configurer.h
+++ b/api/test/pclf/peer_configurer.h
@@ -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 @@
// 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(
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index d916983..17b5968 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -118,6 +118,10 @@
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 @@
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 60006c4..1f83007 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn
@@ -2413,6 +2413,7 @@
"../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 @@
"../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 767e5a4..cde3d91 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -464,6 +464,8 @@
// 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 @@
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>(
diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc
index 54fabce..d933ba6 100644
--- a/pc/peer_connection_factory.cc
+++ b/pc/peer_connection_factory.cc
@@ -224,11 +224,6 @@
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>();
diff --git a/pc/peer_connection_histogram_unittest.cc b/pc/peer_connection_histogram_unittest.cc
index 656b022..0efc1c5 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 @@
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 @@
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(
diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index 58d3c97..d76e5e27 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 @@
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 @@
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 889161e..740e063 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::Contains;
using ::testing::DoAll;
using ::testing::ElementsAre;
+using ::testing::InvokeArgument;
using ::testing::NiceMock;
using ::testing::Return;
using ::testing::SetArgPointee;
@@ -279,8 +281,8 @@
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 @@
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 @@
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_;
diff --git a/rtc_base/async_resolver_interface.h b/rtc_base/async_resolver_interface.h
index 829dc7e..851fa38 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 858676f..9b2f2d6 100644
--- a/test/pc/e2e/test_peer_factory.cc
+++ b/test/pc/e2e/test_peer_factory.cc
@@ -261,9 +261,9 @@
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);
