Reland "Deprecate all classes related to AsyncResolver"
This reverts commit 08d431ec34a1c7ab52557702f2cebd9fdfacae9e.
Reason for revert: Last (hopefully) Chrome blocker removed
Original change's description:
> Revert "Deprecate all classes related to AsyncResolver"
>
> This reverts commit 61a442809cc06de93a613186084d2dfa9c934d81.
>
> Reason for revert: Breaks roll into Chromium
>
> Original change's description:
> > Deprecate all classes related to AsyncResolver
> >
> > and remove internal usage.
> >
> > Bug: webrtc:12598
> > Change-Id: Ie208682bfa0163f6c7a8e805151cfbda76324496
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/322860
> > Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
> > Auto-Submit: Harald Alvestrand <hta@webrtc.org>
> > Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#40919}
>
> Bug: webrtc:12598
> Change-Id: I8aef5e062e19a51baec75873eddfca2a10467d3c
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/322901
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Auto-Submit: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#40927}
Bug: webrtc:12598
Change-Id: I3c7b07c831eb9ff808368433d9b9ae8ec4b2afb6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/323720
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40944}
diff --git a/api/async_resolver_factory.h b/api/async_resolver_factory.h
index ffa9582..997fe5c 100644
--- a/api/async_resolver_factory.h
+++ b/api/async_resolver_factory.h
@@ -19,13 +19,16 @@
// client applications to provide WebRTC with their own mechanism for
// performing DNS resolution.
// TODO(bugs.webrtc.org/12598): Deprecate and remove.
-class AsyncResolverFactory {
+class [[deprecated("Use AsyncDnsResolverFactory")]] AsyncResolverFactory {
public:
AsyncResolverFactory() = default;
virtual ~AsyncResolverFactory() = default;
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
// The caller should call Destroy on the returned object to delete it.
virtual rtc::AsyncResolverInterface* Create() = 0;
+#pragma clang diagnostic pop
};
} // namespace webrtc
diff --git a/api/ice_transport_interface.h b/api/ice_transport_interface.h
index 431f333..001395c 100644
--- a/api/ice_transport_interface.h
+++ b/api/ice_transport_interface.h
@@ -115,8 +115,11 @@
private:
cricket::PortAllocator* port_allocator_ = nullptr;
AsyncDnsResolverFactoryInterface* async_dns_resolver_factory_ = nullptr;
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
// For backwards compatibility. Only one resolver factory can be set.
AsyncResolverFactory* async_resolver_factory_ = nullptr;
+#pragma clang diagnostic pop
RtcEventLog* event_log_ = nullptr;
cricket::IceControllerFactoryInterface* ice_controller_factory_ = nullptr;
cricket::ActiveIceControllerFactoryInterface* active_ice_controller_factory_ =
diff --git a/api/packet_socket_factory.h b/api/packet_socket_factory.h
index 29d2606..e389ccb 100644
--- a/api/packet_socket_factory.h
+++ b/api/packet_socket_factory.h
@@ -76,7 +76,9 @@
// to switch to the AsyncDnsResolverInterface.
// TODO(bugs.webrtc.org/12598): Remove once all downstream users
// are converted.
- virtual AsyncResolverInterface* CreateAsyncResolver() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+ [[deprecated]] virtual AsyncResolverInterface* CreateAsyncResolver() {
// Default implementation, so that downstream users can remove this
// immediately after changing to CreateAsyncDnsResolver
RTC_DCHECK_NOTREACHED();
@@ -89,6 +91,7 @@
return std::make_unique<webrtc::WrappingAsyncDnsResolver>(
CreateAsyncResolver());
}
+#pragma clang diagnostic pop
private:
PacketSocketFactory(const PacketSocketFactory&) = delete;
diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h
index e2f284e..08586f8 100644
--- a/api/peer_connection_interface.h
+++ b/api/peer_connection_interface.h
@@ -1386,10 +1386,12 @@
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;
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+ [[deprecated("Use async_dns_resolver_factory")]] std::unique_ptr<
+ webrtc::AsyncResolverFactory>
+ async_resolver_factory;
+#pragma clang diagnostic pop
std::unique_ptr<webrtc::IceTransportFactory> ice_transport_factory;
std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator;
std::unique_ptr<rtc::SSLCertificateVerifier> tls_cert_verifier;
diff --git a/api/wrapping_async_dns_resolver.h b/api/wrapping_async_dns_resolver.h
index d07f146..b384f97 100644
--- a/api/wrapping_async_dns_resolver.h
+++ b/api/wrapping_async_dns_resolver.h
@@ -33,9 +33,10 @@
namespace webrtc {
-class WrappingAsyncDnsResolver;
+class [[deprecated("Use AsyncDnsResolver directly")]] WrappingAsyncDnsResolver;
-class RTC_EXPORT WrappingAsyncDnsResolverResult
+class [[deprecated(
+ "Use AsyncDnsResolver directly")]] RTC_EXPORT WrappingAsyncDnsResolverResult
: public AsyncDnsResolverResult {
public:
explicit WrappingAsyncDnsResolverResult(WrappingAsyncDnsResolver* owner)
@@ -54,6 +55,8 @@
class RTC_EXPORT WrappingAsyncDnsResolver : public AsyncDnsResolverInterface,
public sigslot::has_slots<> {
public:
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
explicit WrappingAsyncDnsResolver(rtc::AsyncResolverInterface* wrapped)
: wrapped_(absl::WrapUnique(wrapped)), result_(this) {}
@@ -124,6 +127,7 @@
State state_ RTC_GUARDED_BY(sequence_checker_) = State::kNotStarted;
WrappingAsyncDnsResolverResult result_ RTC_GUARDED_BY(sequence_checker_);
bool within_resolve_result_ RTC_GUARDED_BY(sequence_checker_) = false;
+#pragma clang diagnostic pop
};
} // namespace webrtc
diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn
index b8dc0b0..4e4924a 100644
--- a/p2p/BUILD.gn
+++ b/p2p/BUILD.gn
@@ -393,6 +393,7 @@
deps = [
":rtc_p2p",
+ "../api:async_dns_resolver",
"../api:packet_socket_factory",
"../api:sequence_checker",
"../api/task_queue:pending_task_safety_flag",
diff --git a/p2p/base/basic_async_resolver_factory.h b/p2p/base/basic_async_resolver_factory.h
index 9c1af6a..1a94fb9 100644
--- a/p2p/base/basic_async_resolver_factory.h
+++ b/p2p/base/basic_async_resolver_factory.h
@@ -21,10 +21,15 @@
namespace webrtc {
-class BasicAsyncResolverFactory final : public AsyncResolverFactory {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+class [[deprecated(
+ "Use BasicAsyncDnsResolverFactory")]] BasicAsyncResolverFactory final
+ : public AsyncResolverFactory {
public:
rtc::AsyncResolverInterface* Create() override;
};
+#pragma clang diagnostic pop
// A factory that vends AsyncDnsResolver instances.
class BasicAsyncDnsResolverFactory final
@@ -47,9 +52,11 @@
// This class wraps a factory using the older webrtc::AsyncResolverFactory API,
// and produces webrtc::AsyncDnsResolver objects that contain an
// rtc::AsyncResolver object.
-class WrappingAsyncDnsResolverFactory final
+class [[deprecated]] WrappingAsyncDnsResolverFactory final
: public AsyncDnsResolverFactoryInterface {
public:
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
explicit WrappingAsyncDnsResolverFactory(
std::unique_ptr<AsyncResolverFactory> wrapped_factory)
: owned_factory_(std::move(wrapped_factory)),
@@ -58,6 +65,7 @@
explicit WrappingAsyncDnsResolverFactory(
AsyncResolverFactory* non_owned_factory)
: wrapped_factory_(non_owned_factory) {}
+#pragma clang diagnostic pop
std::unique_ptr<webrtc::AsyncDnsResolverInterface> CreateAndResolve(
const rtc::SocketAddress& addr,
diff --git a/p2p/base/basic_async_resolver_factory_unittest.cc b/p2p/base/basic_async_resolver_factory_unittest.cc
index 77b97e7..313907a 100644
--- a/p2p/base/basic_async_resolver_factory_unittest.cc
+++ b/p2p/base/basic_async_resolver_factory_unittest.cc
@@ -22,6 +22,9 @@
namespace webrtc {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+
class BasicAsyncResolverFactoryTest : public ::testing::Test,
public sigslot::has_slots<> {
public:
@@ -108,4 +111,6 @@
}
#endif
+#pragma clang diagnostic pop
+
} // namespace webrtc
diff --git a/p2p/base/basic_packet_socket_factory.cc b/p2p/base/basic_packet_socket_factory.cc
index b07a407..7d87e12 100644
--- a/p2p/base/basic_packet_socket_factory.cc
+++ b/p2p/base/basic_packet_socket_factory.cc
@@ -18,6 +18,7 @@
#include "api/async_dns_resolver.h"
#include "api/wrapping_async_dns_resolver.h"
#include "p2p/base/async_stun_tcp_socket.h"
+#include "rtc_base/async_dns_resolver.h"
#include "rtc_base/async_tcp_socket.h"
#include "rtc_base/async_udp_socket.h"
#include "rtc_base/checks.h"
@@ -184,14 +185,10 @@
return new AsyncResolver();
}
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
std::unique_ptr<webrtc::AsyncDnsResolverInterface>
BasicPacketSocketFactory::CreateAsyncDnsResolver() {
- return std::make_unique<webrtc::WrappingAsyncDnsResolver>(
- new AsyncResolver());
+ return std::make_unique<webrtc::AsyncDnsResolver>();
}
-#pragma clang diagnostic pop
int BasicPacketSocketFactory::BindSocket(Socket* socket,
const SocketAddress& local_address,
diff --git a/p2p/base/mock_async_resolver.h b/p2p/base/mock_async_resolver.h
index 4416471..73ad1a4 100644
--- a/p2p/base/mock_async_resolver.h
+++ b/p2p/base/mock_async_resolver.h
@@ -20,7 +20,10 @@
using ::testing::_;
using ::testing::InvokeWithoutArgs;
-class MockAsyncResolver : public AsyncResolverInterface {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+class [[deprecated]] MockAsyncResolver : public AsyncResolverInterface {
+#pragma clang diagnostic pop
public:
MockAsyncResolver() {
ON_CALL(*this, Start(_)).WillByDefault(InvokeWithoutArgs([this] {
@@ -47,11 +50,13 @@
namespace webrtc {
-class MockAsyncResolverFactory : public AsyncResolverFactory {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+class [[deprecated]] MockAsyncResolverFactory : public AsyncResolverFactory {
public:
MOCK_METHOD(rtc::AsyncResolverInterface*, Create, (), (override));
};
-
+#pragma clang diagnostic pop
} // namespace webrtc
#endif // P2P_BASE_MOCK_ASYNC_RESOLVER_H_
diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h
index ac660d6..8fa4607 100644
--- a/p2p/base/turn_port.h
+++ b/p2p/base/turn_port.h
@@ -267,7 +267,7 @@
void HandleRefreshError();
bool SetAlternateServer(const rtc::SocketAddress& address);
void ResolveTurnAddress(const rtc::SocketAddress& address);
- void OnResolveResult(rtc::AsyncResolverInterface* resolver);
+ void OnResolveResult(const webrtc::AsyncDnsResolverResult& result);
void AddRequestAuthInfo(StunMessage* msg);
void OnSendStunPacket(const void* data, size_t size, StunRequest* request);
diff --git a/p2p/stunprober/stun_prober.cc b/p2p/stunprober/stun_prober.cc
index 977ead4..f5abf43 100644
--- a/p2p/stunprober/stun_prober.cc
+++ b/p2p/stunprober/stun_prober.cc
@@ -329,13 +329,12 @@
}
bool StunProber::ResolveServerName(const rtc::SocketAddress& addr) {
- rtc::AsyncResolverInterface* resolver =
- socket_factory_->CreateAsyncResolver();
- if (!resolver) {
+ RTC_DCHECK(!resolver_);
+ resolver_ = socket_factory_->CreateAsyncDnsResolver();
+ if (!resolver_) {
return false;
}
- resolver->SignalDone.connect(this, &StunProber::OnServerResolved);
- resolver->Start(addr);
+ resolver_->Start(addr, [this] { OnServerResolved(resolver_->result()); });
return true;
}
@@ -347,20 +346,17 @@
}
}
-void StunProber::OnServerResolved(rtc::AsyncResolverInterface* resolver) {
+void StunProber::OnServerResolved(
+ const webrtc::AsyncDnsResolverResult& result) {
RTC_DCHECK(thread_checker_.IsCurrent());
-
- if (resolver->GetError() == 0) {
- rtc::SocketAddress addr(resolver->address().ipaddr(),
- resolver->address().port());
+ rtc::SocketAddress received_address;
+ if (result.GetResolvedAddress(AF_INET, &received_address)) {
+ // Construct an address without the name in it.
+ rtc::SocketAddress addr(received_address.ipaddr(), received_address.port());
all_servers_addrs_.push_back(addr);
}
-
- // Deletion of AsyncResolverInterface can't be done in OnResolveResult which
- // handles SignalDone.
- thread_->PostTask([resolver] { resolver->Destroy(false); });
+ resolver_.reset();
servers_.pop_back();
-
if (servers_.size()) {
if (!ResolveServerName(servers_.back())) {
ReportOnPrepared(RESOLVE_FAILED);
diff --git a/p2p/stunprober/stun_prober.h b/p2p/stunprober/stun_prober.h
index 7d5094a..3f0f4a2 100644
--- a/p2p/stunprober/stun_prober.h
+++ b/p2p/stunprober/stun_prober.h
@@ -11,10 +11,12 @@
#ifndef P2P_STUNPROBER_STUN_PROBER_H_
#define P2P_STUNPROBER_STUN_PROBER_H_
+#include <memory>
#include <set>
#include <string>
#include <vector>
+#include "api/async_dns_resolver.h"
#include "api/sequence_checker.h"
#include "api/task_queue/pending_task_safety_flag.h"
#include "rtc_base/byte_buffer.h"
@@ -166,7 +168,7 @@
};
bool ResolveServerName(const rtc::SocketAddress& addr);
- void OnServerResolved(rtc::AsyncResolverInterface* resolver);
+ void OnServerResolved(const webrtc::AsyncDnsResolverResult& resolver);
void OnSocketReady(rtc::AsyncPacketSocket* socket,
const rtc::SocketAddress& addr);
@@ -241,6 +243,7 @@
ObserverAdapter observer_adapter_;
const std::vector<const rtc::Network*> networks_;
+ std::unique_ptr<webrtc::AsyncDnsResolverInterface> resolver_;
webrtc::ScopedTaskSafety task_safety_;
};
diff --git a/p2p/stunprober/stun_prober_unittest.cc b/p2p/stunprober/stun_prober_unittest.cc
index b57f93b..ca20fcc 100644
--- a/p2p/stunprober/stun_prober_unittest.cc
+++ b/p2p/stunprober/stun_prober_unittest.cc
@@ -51,16 +51,23 @@
rtc::InitializeSSL();
}
+ static constexpr int pings_per_ip = 3;
+
void set_expected_result(int result) { result_ = result; }
+ void CreateProber(rtc::PacketSocketFactory* socket_factory,
+ std::vector<const rtc::Network*> networks) {
+ prober_ = std::make_unique<StunProber>(
+ socket_factory, rtc::Thread::Current(), std::move(networks));
+ }
+
void StartProbing(rtc::PacketSocketFactory* socket_factory,
const std::vector<rtc::SocketAddress>& addrs,
std::vector<const rtc::Network*> networks,
bool shared_socket,
uint16_t interval,
uint16_t pings_per_ip) {
- prober_ = std::make_unique<StunProber>(
- socket_factory, rtc::Thread::Current(), std::move(networks));
+ CreateProber(socket_factory, networks);
prober_->Start(addrs, shared_socket, interval, pings_per_ip,
100 /* timeout_ms */,
[this](StunProber* prober, int result) {
@@ -69,13 +76,17 @@
}
void RunProber(bool shared_mode) {
- const int pings_per_ip = 3;
std::vector<rtc::SocketAddress> addrs;
addrs.push_back(kStunAddr1);
addrs.push_back(kStunAddr2);
// Add a non-existing server. This shouldn't pollute the result.
addrs.push_back(kFailedStunAddr);
+ RunProber(shared_mode, addrs, /* check_results= */ true);
+ }
+ void RunProber(bool shared_mode,
+ const std::vector<rtc::SocketAddress>& addrs,
+ bool check_results) {
rtc::Network ipv4_network1("test_eth0", "Test Network Adapter 1",
rtc::IPAddress(0x12345600U), 24);
ipv4_network1.AddIP(rtc::IPAddress(0x12345678));
@@ -100,17 +111,21 @@
WAIT(stopped_, 1000);
- StunProber::Stats stats;
- EXPECT_TRUE(prober_->GetStats(&stats));
- EXPECT_EQ(stats.success_percent, 100);
- EXPECT_TRUE(stats.nat_type > stunprober::NATTYPE_NONE);
- EXPECT_EQ(stats.srflx_addrs, srflx_addresses);
- EXPECT_EQ(static_cast<uint32_t>(stats.num_request_sent),
- total_pings_reported);
- EXPECT_EQ(static_cast<uint32_t>(stats.num_response_received),
- total_pings_reported);
+ EXPECT_TRUE(prober_->GetStats(&stats_));
+ if (check_results) {
+ EXPECT_EQ(stats_.success_percent, 100);
+ EXPECT_TRUE(stats_.nat_type > stunprober::NATTYPE_NONE);
+ EXPECT_EQ(stats_.srflx_addrs, srflx_addresses);
+ EXPECT_EQ(static_cast<uint32_t>(stats_.num_request_sent),
+ total_pings_reported);
+ EXPECT_EQ(static_cast<uint32_t>(stats_.num_response_received),
+ total_pings_reported);
+ }
}
+ StunProber* prober() { return prober_.get(); }
+ StunProber::Stats& stats() { return stats_; }
+
private:
void StopCallback(StunProber* prober, int result) {
EXPECT_EQ(result, result_);
@@ -124,6 +139,7 @@
bool stopped_ = false;
std::unique_ptr<cricket::TestStunServer> stun_server_1_;
std::unique_ptr<cricket::TestStunServer> stun_server_2_;
+ StunProber::Stats stats_;
};
TEST_F(StunProberTest, NonSharedMode) {
@@ -134,4 +150,26 @@
RunProber(true);
}
+TEST_F(StunProberTest, ResolveNonexistentHostname) {
+ std::vector<rtc::SocketAddress> addrs;
+ addrs.push_back(kStunAddr1);
+ // Add a non-existing server by name. This should cause a failed lookup.
+ addrs.push_back(rtc::SocketAddress("nonexistent.test", 3478));
+ RunProber(false, addrs, false);
+ // One server is pinged
+ EXPECT_EQ(stats().raw_num_request_sent, pings_per_ip);
+}
+
+TEST_F(StunProberTest, ResolveExistingHostname) {
+ std::vector<rtc::SocketAddress> addrs;
+ addrs.push_back(kStunAddr1);
+ // Add a non-existing server by name. This should cause a failed lookup.
+ addrs.push_back(rtc::SocketAddress("localhost", 3478));
+ RunProber(false, addrs, false);
+ // Two servers are pinged, only one responds.
+ // TODO(bugs.webrtc.org/15559): Figure out why this doesn't always work
+ // EXPECT_EQ(stats().raw_num_request_sent, pings_per_ip * 2);
+ EXPECT_EQ(stats().num_request_sent, pings_per_ip);
+}
+
} // namespace stunprober
diff --git a/rtc_base/async_resolver.h b/rtc_base/async_resolver.h
index 46be438..9de4d12 100644
--- a/rtc_base/async_resolver.h
+++ b/rtc_base/async_resolver.h
@@ -39,7 +39,12 @@
// happen from the same rtc::Thread, except for Destroy which is allowed to
// happen on another context provided it's not happening concurrently to another
// public API call, and is the last access to the object.
-class RTC_EXPORT AsyncResolver : public AsyncResolverInterface {
+// TODO(bugs.webrtc.org/12598): Deprecate and remove
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wstrict-aliasing"
+class [[deprecated("Use AsyncDnsResolver")]] RTC_EXPORT AsyncResolver
+ : public AsyncResolverInterface {
+#pragma clang diagnostic pop
public:
AsyncResolver();
~AsyncResolver() override;
diff --git a/rtc_base/async_resolver_interface.h b/rtc_base/async_resolver_interface.h
index 851fa38..a0bda27 100644
--- a/rtc_base/async_resolver_interface.h
+++ b/rtc_base/async_resolver_interface.h
@@ -20,7 +20,7 @@
// This interface defines the methods to resolve the address asynchronously.
// TODO(bugs.webrtc.org/12598): Deprecate and remove.
-class RTC_EXPORT AsyncResolverInterface {
+class [[deprecated("Use AsyncDnsResolver")]] RTC_EXPORT AsyncResolverInterface {
public:
AsyncResolverInterface();
virtual ~AsyncResolverInterface();
