Retry "Add new ResolveHost API."

This reverts commit e5a94e05.
Original commit fac52799.

Patchset 1 is a clean revert of the revert.  Fixes in subsequent patchsets.

Only change is fixing some work that was done inside a DCHECK in the tests.

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ic1649dc90711c23c33255cc5f1ed9d902eb29f07
Reviewed-on: https://chromium-review.googlesource.com/1147140Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577362}

components/cronet/stale_host_resolver.cc

@@ -381,6 +381,14 @@ StaleHostResolver::StaleHostResolver(
 
 StaleHostResolver::~StaleHostResolver() {}
 
+std::unique_ptr<net::HostResolver::ResolveHostRequest>
+StaleHostResolver::CreateRequest(const net::HostPortPair& host,
+                                 const net::NetLogWithSource& net_log) {
+  // TODO(crbug.com/821021): Implement.
+  NOTIMPLEMENTED();
+  return nullptr;
+}
+
 int StaleHostResolver::Resolve(const RequestInfo& info,
                                net::RequestPriority priority,
                                net::AddressList* addresses,

components/cronet/stale_host_resolver.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_CRONET_STALE_HOST_RESOLVER_H_
 #define COMPONENTS_CRONET_STALE_HOST_RESOLVER_H_
 
+#include <memory>
 #include <unordered_set>
 
 #include "base/time/default_tick_clock.h"
@@ -61,6 +62,10 @@ class StaleHostResolver : public net::HostResolver {
 
   // HostResolver implementation:
 
+  std::unique_ptr<ResolveHostRequest> CreateRequest(
+      const net::HostPortPair& host,
+      const net::NetLogWithSource& net_log) override;
+
   // Resolves as a regular HostResolver, but if stale data is available and
   // usable (according to the options passed to the constructor), and fresh data
   // is not returned before the specified delay, returns the stale data instead.

net/dns/host_resolver.h

@@ -10,6 +10,7 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "net/base/address_family.h"
 #include "net/base/completion_once_callback.h"
@@ -45,6 +46,9 @@ class NET_EXPORT HostResolver {
  public:
   // HostResolver::Request class is used to cancel the request and change it's
   // priority. It must be owned by consumer. Deletion cancels the request.
+  //
+  // TODO(crbug.com/821021): Delete this class once all usage has been
+  // converted to the new CreateRequest() API.
   class Request {
    public:
     virtual ~Request() {}
@@ -55,6 +59,41 @@ class NET_EXPORT HostResolver {
     virtual void ChangeRequestPriority(RequestPriority priority) = 0;
   };
 
+  // Handler for an individual host resolution request. Created by
+  // HostResolver::CreateRequest().
+  class ResolveHostRequest {
+   public:
+    // Destruction cancels the request if running asynchronously, causing the
+    // callback to never be invoked.
+    virtual ~ResolveHostRequest() {}
+
+    // Starts the request and returns a network error code.
+    //
+    // If the request could not be handled synchronously, returns
+    // |ERR_IO_PENDING|, and completion will be signaled later via |callback|.
+    // On any other returned value, the request was handled synchronously and
+    // |callback| will not be invoked.
+    //
+    // Results in ERR_NAME_NOT_RESOLVED if the hostname is invalid, or if it is
+    // an incompatible IP literal (e.g. IPv6 is disabled and it is an IPv6
+    // literal).
+    //
+    // The parent HostResolver must still be alive when Start() is called,  but
+    // if it is destroyed before an asynchronous result completes, the request
+    // will be automatically cancelled.
+    //
+    // If cancelled before |callback| is invoked, it will never be invoked.
+    virtual int Start(CompletionOnceCallback callback) = 0;
+
+    // Result of the request. Should only be called after Start() signals
+    // completion, either by invoking the callback or by returning a result
+    // other than |ERR_IO_PENDING|.
+    //
+    // TODO(crbug.com/821021): Implement other GetResults() methods for requests
+    // that return other data (eg DNS TXT requests).
+    virtual const base::Optional<AddressList>& GetAddressResults() const = 0;
+  };
+
   // |max_concurrent_resolves| is how many resolve requests will be allowed to
   // run in parallel. Pass HostResolver::kDefaultParallelism to choose a
   // default value.
@@ -144,6 +183,21 @@ class NET_EXPORT HostResolver {
   // be called.
   virtual ~HostResolver();
 
+  // Creates a request to resolve the given hostname (or IP address literal).
+  // Profiling information for the request is saved to |net_log| if non-NULL.
+  //
+  // This method is intended as a direct replacement for the old Resolve()
+  // method, but it may not yet cover all the capabilities of the old method.
+  //
+  // TODO(crbug.com/821021): Implement more complex functionality to meet
+  // capabilities of Resolve() and M/DnsClient functionality.
+  virtual std::unique_ptr<ResolveHostRequest> CreateRequest(
+      const HostPortPair& host,
+      const NetLogWithSource& net_log) = 0;
+
+  // DEPRECATION NOTE: This method is being replaced by CreateRequest(). New
+  // callers should prefer CreateRequest() if it works for their needs.
+  //
   // Resolves the given hostname (or IP address literal), filling out the
   // |addresses| object upon success.  The |info.port| parameter will be set as
   // the sin(6)_port field of the sockaddr_in{6} struct.  Returns OK if
@@ -165,6 +219,9 @@ class NET_EXPORT HostResolver {
   // |out_req| will cancel the request, and cause |callback| not to be invoked.
   //
   // Profiling information for the request is saved to |net_log| if non-NULL.
+  //
+  // TODO(crbug.com/821021): Delete this method once all usage has been
+  // converted to ResolveHost().
   virtual int Resolve(const RequestInfo& info,
                       RequestPriority priority,
                       AddressList* addresses,

net/dns/host_resolver_impl.h

@@ -10,6 +10,9 @@
 
 #include <map>
 #include <memory>
+#include <set>
+#include <string>
+#include <vector>
 
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
@@ -125,10 +128,6 @@ class NET_EXPORT HostResolverImpl
   // be called.
   ~HostResolverImpl() override;
 
-  // Configures maximum number of Jobs in the queue. Exposed for testing.
-  // Only allowed when the queue is empty.
-  void SetMaxQueuedJobs(size_t value);
-
   // Set the DnsClient to be used for resolution. In case of failure, the
   // HostResolverProc from ProcTaskParams will be queried. If the DnsClient is
   // not pre-configured with a valid DnsConfig, a new config is fetched from
@@ -136,6 +135,9 @@ class NET_EXPORT HostResolverImpl
   void SetDnsClient(std::unique_ptr<DnsClient> dns_client);
 
   // HostResolver methods:
+  std::unique_ptr<ResolveHostRequest> CreateRequest(
+      const HostPortPair& host,
+      const NetLogWithSource& net_log) override;
   int Resolve(const RequestInfo& info,
               RequestPriority priority,
               AddressList* addresses,
@@ -179,6 +181,10 @@ class NET_EXPORT HostResolverImpl
 
   void SetTickClockForTesting(const base::TickClock* tick_clock);
 
+  // Configures maximum number of Jobs in the queue. Exposed for testing.
+  // Only allowed when the queue is empty.
+  void SetMaxQueuedJobsForTesting(size_t value);
+
  protected:
   // Callback from HaveOnlyLoopbackAddresses probe.
   void SetHaveOnlyLoopbackAddresses(bool result);
@@ -193,6 +199,7 @@ class NET_EXPORT HostResolverImpl
   class LoopbackProbeJob;
   class DnsTask;
   class RequestImpl;
+  class LegacyRequestImpl;
   using Key = HostCache::Key;
   using JobMap = std::map<Key, std::unique_ptr<Job>>;
 
@@ -200,11 +207,14 @@ class NET_EXPORT HostResolverImpl
   // ProcTask) before the DnsClient is disabled until the next DNS change.
   static const unsigned kMaximumDnsFailures;
 
-  // Helper used by |Resolve()| and |ResolveFromCache()|.  Performs IP
-  // literal, cache and HOSTS lookup (if enabled), returns OK if successful,
-  // ERR_NAME_NOT_RESOLVED if either hostname is invalid or IP literal is
-  // incompatible, ERR_DNS_CACHE_MISS if entry was not found in cache and
-  // HOSTS and is not localhost.
+  // Attempts host resolution for |request|. Generally only expected to be
+  // called from RequestImpl::Start().
+  int Resolve(RequestImpl* request);
+
+  // Attempts host resolution using fast local sources: IP literal resolution,
+  // cache lookup, HOSTS lookup (if enabled), and localhost. Returns OK if
+  // successful, ERR_NAME_NOT_RESOLVED if input is invalid, or
+  // ERR_DNS_CACHE_MISS if the host could not be resolved using local sources.
   //
   // On success, the resulting addresses are written to |addresses|.
   //
@@ -217,17 +227,25 @@ class NET_EXPORT HostResolverImpl
   //
   // If |allow_stale| is false, then stale cache entries will not be returned,
   // and |stale_info| must be null.
-  int ResolveHelper(const RequestInfo& info,
-                    bool allow_stale,
-                    HostCache::EntryStaleness* stale_info,
-                    const NetLogWithSource& request_net_log,
-                    AddressList* addresses,
-                    Key* key);
+  int ResolveLocally(const HostPortPair& host,
+                     AddressFamily requested_address_family,
+                     HostResolverFlags flags,
+                     bool allow_cache,
+                     bool allow_stale,
+                     HostCache::EntryStaleness* stale_info,
+                     const NetLogWithSource& request_net_log,
+                     AddressList* addresses,
+                     Key* key);
+
+  // Attempts to create and start a Job to asynchronously attempt to resolve
+  // |key|. On success, returns ERR_IO_PENDING and attaches the Job to
+  // |request|. On error, marks |request| completed and returns the error.
+  int CreateAndStartJob(const Key& key, RequestImpl* request);
 
   // Tries to resolve |key| as an IP, returns true and sets |net_error| if
   // succeeds, returns false otherwise.
   bool ResolveAsIP(const Key& key,
-                   const RequestInfo& info,
+                   uint16_t host_port,
                    const IPAddress* ip_address,
                    int* net_error,
                    AddressList* addresses);
@@ -243,7 +261,7 @@ class NET_EXPORT HostResolverImpl
   // If |allow_stale| is false, then stale cache entries will not be returned,
   // and |stale_info| must be null.
   bool ServeFromCache(const Key& key,
-                      const RequestInfo& info,
+                      uint16_t host_port,
                       int* net_error,
                       AddressList* addresses,
                       bool allow_stale,
@@ -252,19 +270,21 @@ class NET_EXPORT HostResolverImpl
   // If we have a DnsClient with a valid DnsConfig, and |key| is found in the
   // HOSTS file, returns true and fills |addresses|. Otherwise returns false.
   bool ServeFromHosts(const Key& key,
-                      const RequestInfo& info,
+                      uint16_t host_port,
                       AddressList* addresses);
 
   // If |key| is for a localhost name (RFC 6761), returns true and fills
   // |addresses| with the loopback IP. Otherwise returns false.
   bool ServeLocalhost(const Key& key,
-                      const RequestInfo& info,
+                      uint16_t host_port,
                       AddressList* addresses);
 
   // Returns the (hostname, address_family) key to use for |info|, choosing an
   // "effective" address family by inheriting the resolver's default address
   // family when the request leaves it unspecified.
-  Key GetEffectiveKeyForRequest(const RequestInfo& info,
+  Key GetEffectiveKeyForRequest(const std::string& hostname,
+                                AddressFamily requested_address_family,
+                                HostResolverFlags flags,
                                 const IPAddress* ip_address,
                                 const NetLogWithSource& net_log);
 

net/dns/host_resolver_mojo.cc

@@ -70,6 +70,14 @@ HostResolverMojo::HostResolverMojo(Impl* impl)
 
 HostResolverMojo::~HostResolverMojo() = default;
 
+std::unique_ptr<HostResolver::ResolveHostRequest>
+HostResolverMojo::CreateRequest(const HostPortPair& host,
+                                const NetLogWithSource& source_net_log) {
+  // TODO(crbug.com/821021): Implement.
+  NOTIMPLEMENTED();
+  return nullptr;
+}
+
 int HostResolverMojo::Resolve(const RequestInfo& info,
                               RequestPriority priority,
                               AddressList* addresses,

net/dns/host_resolver_mojo.h

@@ -5,6 +5,8 @@
 #ifndef NET_DNS_HOST_RESOLVER_MOJO_H_
 #define NET_DNS_HOST_RESOLVER_MOJO_H_
 
+#include <memory>
+
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
@@ -34,6 +36,9 @@ class HostResolverMojo : public HostResolver {
   ~HostResolverMojo() override;
 
   // HostResolver overrides.
+  std::unique_ptr<ResolveHostRequest> CreateRequest(
+      const HostPortPair& host,
+      const NetLogWithSource& net_log) override;
   // Note: |Resolve()| currently ignores |priority|.
   int Resolve(const RequestInfo& info,
               RequestPriority priority,

net/dns/mapped_host_resolver.cc

@@ -18,6 +18,14 @@ MappedHostResolver::MappedHostResolver(std::unique_ptr<HostResolver> impl)
 
 MappedHostResolver::~MappedHostResolver() = default;
 
+std::unique_ptr<HostResolver::ResolveHostRequest>
+MappedHostResolver::CreateRequest(const HostPortPair& host,
+                                  const NetLogWithSource& source_net_log) {
+  // TODO(crbug.com/821021): Implement.
+  NOTIMPLEMENTED();
+  return nullptr;
+}
+
 int MappedHostResolver::Resolve(const RequestInfo& original_info,
                                 RequestPriority priority,
                                 AddressList* addresses,

net/dns/mapped_host_resolver.h

@@ -46,6 +46,9 @@ class NET_EXPORT MappedHostResolver : public HostResolver {
   }
 
   // HostResolver methods:
+  std::unique_ptr<ResolveHostRequest> CreateRequest(
+      const HostPortPair& host,
+      const NetLogWithSource& net_log) override;
   int Resolve(const RequestInfo& info,
               RequestPriority priority,
               AddressList* addresses,

net/dns/mock_host_resolver.cc

@@ -103,6 +103,14 @@ MockHostResolverBase::~MockHostResolverBase() {
   DCHECK(requests_.empty());
 }
 
+std::unique_ptr<HostResolver::ResolveHostRequest>
+MockHostResolverBase::CreateRequest(const HostPortPair& host,
+                                    const NetLogWithSource& source_net_log) {
+  // TODO(crbug.com/821021): Implement.
+  NOTIMPLEMENTED();
+  return nullptr;
+}
+
 int MockHostResolverBase::Resolve(const RequestInfo& info,
                                   RequestPriority priority,
                                   AddressList* addresses,
@@ -514,6 +522,14 @@ RuleBasedHostResolverProc* CreateCatchAllHostResolverProc() {
 
 //-----------------------------------------------------------------------------
 
+std::unique_ptr<HostResolver::ResolveHostRequest>
+HangingHostResolver::CreateRequest(const HostPortPair& host,
+                                   const NetLogWithSource& source_net_log) {
+  // TODO(crbug.com/821021): Implement.
+  NOTIMPLEMENTED();
+  return nullptr;
+}
+
 int HangingHostResolver::Resolve(const RequestInfo& info,
                                  RequestPriority priority,
                                  AddressList* addresses,

net/dns/mock_host_resolver.h

@@ -9,6 +9,7 @@
 
 #include <list>
 #include <map>
+#include <memory>
 #include <string>
 
 #include "base/macros.h"
@@ -83,6 +84,9 @@ class MockHostResolverBase
   }
 
   // HostResolver methods:
+  std::unique_ptr<ResolveHostRequest> CreateRequest(
+      const HostPortPair& host,
+      const NetLogWithSource& net_log) override;
   int Resolve(const RequestInfo& info,
               RequestPriority priority,
               AddressList* addresses,
@@ -287,6 +291,9 @@ RuleBasedHostResolverProc* CreateCatchAllHostResolverProc();
 // HangingHostResolver never completes its |Resolve| request.
 class HangingHostResolver : public HostResolver {
  public:
+  std::unique_ptr<ResolveHostRequest> CreateRequest(
+      const HostPortPair& host,
+      const NetLogWithSource& net_log) override;
   int Resolve(const RequestInfo& info,
               RequestPriority priority,
               AddressList* addresses,

net/proxy_resolution/proxy_resolver_v8_tracing_unittest.cc

@@ -693,6 +693,13 @@ class BlockableHostResolver : public HostResolver {
   BlockableHostResolver()
       : num_cancelled_requests_(0), waiting_for_resolve_(false) {}
 
+  std::unique_ptr<ResolveHostRequest> CreateRequest(
+      const HostPortPair& host,
+      const NetLogWithSource& net_log) override {
+    NOTIMPLEMENTED();
+    return nullptr;
+  }
+
   int Resolve(const RequestInfo& info,
               RequestPriority priority,
               AddressList* addresses,

net/proxy_resolution/proxy_resolver_v8_tracing_wrapper_unittest.cc

@@ -790,6 +790,13 @@ class BlockableHostResolver : public HostResolver {
   BlockableHostResolver()
       : num_cancelled_requests_(0), waiting_for_resolve_(false) {}
 
+  std::unique_ptr<ResolveHostRequest> CreateRequest(
+      const HostPortPair& host,
+      const NetLogWithSource& net_log) override {
+    NOTIMPLEMENTED();
+    return nullptr;
+  }
+
   int Resolve(const RequestInfo& info,
               RequestPriority priority,
               AddressList* addresses,

net/socket/socks_client_socket_unittest.cc

@@ -107,6 +107,13 @@ class HangingHostResolverWithCancel : public HostResolver {
  public:
   HangingHostResolverWithCancel() : outstanding_request_(NULL) {}
 
+  std::unique_ptr<ResolveHostRequest> CreateRequest(
+      const HostPortPair& host,
+      const NetLogWithSource& net_log) override {
+    NOTIMPLEMENTED();
+    return nullptr;
+  }
+
   int Resolve(const RequestInfo& info,
               RequestPriority priority,
               AddressList* addresses,

services/network/proxy_resolver_factory_mojo_unittest.cc

@@ -478,6 +478,14 @@ class MockHostResolver : public net::HostResolver {
   };
 
   // net::HostResolver overrides.
+  std::unique_ptr<HostResolver::ResolveHostRequest> CreateRequest(
+      const net::HostPortPair& host,
+      const net::NetLogWithSource& source_net_log) override {
+    // TODO(crbug.com/821021): Implement.
+    NOTIMPLEMENTED();
+    return nullptr;
+  }
+
   int Resolve(const RequestInfo& info,
               net::RequestPriority priority,
               net::AddressList* addresses,