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}

api/async_resolver_factory.h

@@ -19,13 +19,16 @@ namespace webrtc {
 // 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

api/ice_transport_interface.h

@@ -115,8 +115,11 @@ struct IceTransportInit final {
  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_ =

api/packet_socket_factory.h

@@ -76,7 +76,9 @@ class RTC_EXPORT PacketSocketFactory {
   // 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 @@ class RTC_EXPORT PacketSocketFactory {
     return std::make_unique<webrtc::WrappingAsyncDnsResolver>(
         CreateAsyncResolver());
   }
+#pragma clang diagnostic pop
 
  private:
   PacketSocketFactory(const PacketSocketFactory&) = delete;

api/peer_connection_interface.h

@@ -1386,10 +1386,12 @@ 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.
+#pragma clang diagnostic push
-  // TODO(crbug.com/1475925): Deprecate once Chromium is updated
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-  // [[deprecated("Use async_dns_resolver_factory")]]
+  [[deprecated("Use async_dns_resolver_factory")]] std::unique_ptr<
-  std::unique_ptr<webrtc::AsyncResolverFactory> async_resolver_factory;
+      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;

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 WrappingAsyncDnsResolverResult
 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 @@ class RTC_EXPORT WrappingAsyncDnsResolver : public AsyncDnsResolverInterface,
   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

p2p/BUILD.gn

@@ -393,6 +393,7 @@ rtc_library("libstunprober") {
 
   deps = [
     ":rtc_p2p",
+    "../api:async_dns_resolver",
     "../api:packet_socket_factory",
     "../api:sequence_checker",
     "../api/task_queue:pending_task_safety_flag",

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 @@ class BasicAsyncDnsResolverFactory final
 // 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 @@ class WrappingAsyncDnsResolverFactory final
   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,

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 @@ TEST(WrappingAsyncDnsResolverFactoryDeathTest, DestroyResolverInCallback) {
 }
 #endif
 
+#pragma clang diagnostic pop
+
 }  // namespace webrtc

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 @@ AsyncResolverInterface* BasicPacketSocketFactory::CreateAsyncResolver() {
   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>(
+  return std::make_unique<webrtc::AsyncDnsResolver>();
-      new AsyncResolver());
 }
-#pragma clang diagnostic pop
 
 int BasicPacketSocketFactory::BindSocket(Socket* socket,
                                          const SocketAddress& local_address,

p2p/base/mock_async_resolver.h

@@ -20,7 +20,10 @@ namespace rtc {
 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 @@ class MockAsyncResolver : public AsyncResolverInterface {
 
 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_

p2p/base/turn_port.h

@@ -267,7 +267,7 @@ class TurnPort : public Port {
   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);

p2p/stunprober/stun_prober.cc

@@ -329,13 +329,12 @@ bool StunProber::Start(StunProber::Observer* observer) {
 }
 
 bool StunProber::ResolveServerName(const rtc::SocketAddress& addr) {
-  rtc::AsyncResolverInterface* resolver =
+  RTC_DCHECK(!resolver_);
-      socket_factory_->CreateAsyncResolver();
+  resolver_ = socket_factory_->CreateAsyncDnsResolver();
-  if (!resolver) {
+  if (!resolver_) {
     return false;
   }
-  resolver->SignalDone.connect(this, &StunProber::OnServerResolved);
+  resolver_->Start(addr, [this] { OnServerResolved(resolver_->result()); });
-  resolver->Start(addr);
   return true;
 }
 
@@ -347,20 +346,17 @@ void StunProber::OnSocketReady(rtc::AsyncPacketSocket* socket,
   }
 }
 
-void StunProber::OnServerResolved(rtc::AsyncResolverInterface* resolver) {
+void StunProber::OnServerResolved(
+    const webrtc::AsyncDnsResolverResult& result) {
   RTC_DCHECK(thread_checker_.IsCurrent());
-
+  rtc::SocketAddress received_address;
-  if (resolver->GetError() == 0) {
+  if (result.GetResolvedAddress(AF_INET, &received_address)) {
-    rtc::SocketAddress addr(resolver->address().ipaddr(),
+    // Construct an address without the name in it.
-                            resolver->address().port());
+    rtc::SocketAddress addr(received_address.ipaddr(), received_address.port());
     all_servers_addrs_.push_back(addr);
   }
-
+  resolver_.reset();
-  // Deletion of AsyncResolverInterface can't be done in OnResolveResult which
-  // handles SignalDone.
-  thread_->PostTask([resolver] { resolver->Destroy(false); });
   servers_.pop_back();
-
   if (servers_.size()) {
     if (!ResolveServerName(servers_.back())) {
       ReportOnPrepared(RESOLVE_FAILED);

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 @@ class RTC_EXPORT StunProber : public sigslot::has_slots<> {
   };
 
   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 @@ class RTC_EXPORT StunProber : public sigslot::has_slots<> {
   ObserverAdapter observer_adapter_;
 
   const std::vector<const rtc::Network*> networks_;
+  std::unique_ptr<webrtc::AsyncDnsResolverInterface> resolver_;
 
   webrtc::ScopedTaskSafety task_safety_;
 };

p2p/stunprober/stun_prober_unittest.cc

@@ -51,16 +51,23 @@ class StunProberTest : public ::testing::Test {
     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>(
+    CreateProber(socket_factory, networks);
-        socket_factory, rtc::Thread::Current(), std::move(networks));
     prober_->Start(addrs, shared_socket, interval, pings_per_ip,
                    100 /* timeout_ms */,
                    [this](StunProber* prober, int result) {
@@ -69,13 +76,17 @@ class StunProberTest : public ::testing::Test {
   }
 
   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,16 +111,20 @@ class StunProberTest : public ::testing::Test {
 
     WAIT(stopped_, 1000);
 
-    StunProber::Stats stats;
+    EXPECT_TRUE(prober_->GetStats(&stats_));
-    EXPECT_TRUE(prober_->GetStats(&stats));
+    if (check_results) {
-    EXPECT_EQ(stats.success_percent, 100);
+      EXPECT_EQ(stats_.success_percent, 100);
-    EXPECT_TRUE(stats.nat_type > stunprober::NATTYPE_NONE);
+      EXPECT_TRUE(stats_.nat_type > stunprober::NATTYPE_NONE);
-    EXPECT_EQ(stats.srflx_addrs, srflx_addresses);
+      EXPECT_EQ(stats_.srflx_addrs, srflx_addresses);
-    EXPECT_EQ(static_cast<uint32_t>(stats.num_request_sent),
+      EXPECT_EQ(static_cast<uint32_t>(stats_.num_request_sent),
                 total_pings_reported);
-    EXPECT_EQ(static_cast<uint32_t>(stats.num_response_received),
+      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) {
@@ -124,6 +139,7 @@ class StunProberTest : public ::testing::Test {
   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 @@ TEST_F(StunProberTest, SharedMode) {
   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

rtc_base/async_resolver.h

@@ -39,7 +39,12 @@ namespace rtc {
 // 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;

rtc_base/async_resolver_interface.h

@@ -20,7 +20,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 {
+class [[deprecated("Use AsyncDnsResolver")]] RTC_EXPORT AsyncResolverInterface {
  public:
   AsyncResolverInterface();
   virtual ~AsyncResolverInterface();