[net] HostResolverImpl + DnsTransaction + DnsConfigService = Asynchronous DNS ready for experiments.

net/base/host_resolver_impl.h

Index: net/base/host_resolver_impl.h
diff --git a/net/base/host_resolver_impl.h b/net/base/host_resolver_impl.h
index d5ff027aabbcf50a4dcfe15c1617699600d4f160..2488a6365984e00717c2d3f30926bf3946021ca0 100644
--- a/net/base/host_resolver_impl.h
+++ b/net/base/host_resolver_impl.h
@@ -23,11 +23,14 @@
 #include "net/base/net_log.h"
 #include "net/base/network_change_notifier.h"
 #include "net/base/prioritized_dispatcher.h"
+#include "net/dns/dns_config_service.h"
 
 namespace net {
 
+class DnsTransactionFactory;
+
 // For each hostname that is requested, HostResolver creates a
-// HostResolverImpl::Job. When this job gets dispatched it creates a ProcJob
+// HostResolverImpl::Job. When this job gets dispatched it creates a ProcTask
 // which runs the given HostResolverProc on a WorkerPool thread. If requests for
 // that same host are made during the job's lifetime, they are attached to the
 // existing job rather than creating a new one. This avoids doing parallel
@@ -55,11 +58,13 @@ namespace net {
 //
 // Jobs are ordered in the queue based on their priority and order of arrival.
 //
+// TODO(szym): Change DnsConfigService::Observer to Callback.
 class NET_EXPORT HostResolverImpl
     : public HostResolver,
       NON_EXPORTED_BASE(public base::NonThreadSafe),
       public NetworkChangeNotifier::IPAddressObserver,
       public NetworkChangeNotifier::DNSObserver,
+      public DnsConfigService::Observer,

[cbentzel, 2012/02/10 19:51:08]

NON_EXPORTED_BASE needed. 

[szym, 2012/02/10 21:49:36]

In the long run, I don't think we should be keeping the Observer pattern given
that HostResolverImpl owns the DnsConfigService.

[cbentzel, 2012/02/10 22:18:49]

Agree. But it doesn't compile without this on a component build.

[szym, 2012/02/11 00:01:37]

Oh yes, absolutely, that is fixed in the next patch.

       public base::SupportsWeakPtr<HostResolverImpl> {
  public:
   // Parameters for ProcTask which resolves hostnames using HostResolveProc.
@@ -105,13 +110,23 @@ class NET_EXPORT HostResolverImpl
   // ownership of the |cache| pointer, and will free it during destruction.
   //
   // |job_limits| specifies the maximum number of jobs that the resolver will
-  // run at once (not counting potential duplicate attempts).
+  // run at once. This upper-bounds the total number of outstanding
+  // DNS transactions (not counting retransmissions and retries).
+  //
+  // |proc_job_limits| specifies the maximum number of calls to
+  // SystemHostResolverProc the resolver will make on WorkerPool (not counting
+  // retries).
+  //
+  // |dns_config_service| will be used to obtain DnsConfig for
+  // DnsTransactionFactory.
   //
   // |net_log| must remain valid for the life of the HostResolverImpl.
   // TODO(szym): change to scoped_ptr<HostCache>.
   HostResolverImpl(HostCache* cache,
                    const PrioritizedDispatcher::Limits& job_limits,
+                   const PrioritizedDispatcher::Limits& proc_limits,
                    const ProcTaskParams& proc_params,
+                   scoped_ptr<DnsConfigService> dns_config_service,
                    NetLog* net_log);
 
   // If any completion callbacks are pending when the resolver is destroyed,
@@ -142,6 +157,7 @@ class NET_EXPORT HostResolverImpl
   class Job;
   class ProcTask;
   class IPv6ProbeJob;
+  class DnsTask;
   class Request;
   typedef HostCache::Key Key;
   typedef std::map<Key, Job*> JobMap;
@@ -185,7 +201,9 @@ class NET_EXPORT HostResolverImpl
 
   // Called by |job| when it has finished running. Records the result in cache
   // if necessary and dispatches another job if possible.
-  void OnJobFinished(Job* job, const AddressList& addrlist);
+  void OnJobFinished(Job* job,
+                     const AddressList& addrlist,

[cbentzel, 2012/02/10 19:51:08]

Should ttl be part of the AddressList?

[szym, 2012/02/10 21:49:36]

An excellent question. Makes sense for HostCache and DnsTask, but not quite for
HostResolverProc and getaddrinfo. (There's no explicit permission in POSIX to
cache the results of getaddrinfo.)

+                     base::TimeDelta ttl);
 
   // Removes |job| from |jobs_|.
   void RemoveJob(Job* job);
@@ -194,12 +212,15 @@ class NET_EXPORT HostResolverImpl
   // Might start new jobs.
   void AbortAllInProgressJobs();
 
-  // NetworkChangeNotifier::IPAddressObserver methods:
+  // NetworkChangeNotifier::IPAddressObserver interface.

[cbentzel, 2012/02/10 19:51:08]

Nit: the examples on http://dev.chromium.org/developers/coding-style show

// NetworkChangeNotifier::IPAddressObserver:

   virtual void OnIPAddressChanged() OVERRIDE;
 
-  // NetworkChangeNotifier::OnDNSChanged methods:
+  // NetworkChangeNotifier::DNSObserver interface.
   virtual void OnDNSChanged() OVERRIDE;
 
+  // DnsConfigService::Observer interface.
+  virtual void OnConfigChanged(const DnsConfig& dns_config) OVERRIDE;
+
   // Cache of host resolution results.
   scoped_ptr<HostCache> cache_;
 
@@ -209,15 +230,23 @@ class NET_EXPORT HostResolverImpl
   // Starts Jobs according to their priority and the configured limits.
   PrioritizedDispatcher dispatcher_;
 
+  // Starts ProcTask if DnsTask fails. Enforces tighter limits than
+  // |dispatcher_|.
+  PrioritizedDispatcher proc_dispatcher_;
+
   // Limit on the maximum number of jobs queued in |dispatcher_|.
   size_t max_queued_jobs_;
 
   // Parameters for ProcTask.
   ProcTaskParams proc_params_;
 
+  scoped_ptr<DnsTransactionFactory> dns_transaction_factory_;
+
   // Address family to use when the request doesn't specify one.
   AddressFamily default_address_family_;
 
+  scoped_ptr<DnsConfigService> dns_config_service_;
+
   // Indicate if probing is done after each network change event to set address
   // family.
   // When false, explicit setting of address family is used.