[net/dns] Refactoring of job dispatch in HostResolverImpl in preparation for DnsTransactionFactory.

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 da694519fc1eaec22ffed3e3f349045de5ff7469..d011b8a66f16f611edd6d22a7b7691b837d1a7a0 100644
--- a/net/base/host_resolver_impl.h
+++ b/net/base/host_resolver_impl.h
@@ -6,11 +6,13 @@
 #define NET_BASE_HOST_RESOLVER_IMPL_H_
 #pragma once
 
+#include <map>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/threading/non_thread_safe.h"
 #include "base/time.h"
 #include "net/base/capturing_net_log.h"
@@ -20,15 +22,16 @@
 #include "net/base/net_export.h"
 #include "net/base/net_log.h"
 #include "net/base/network_change_notifier.h"
+#include "net/base/prioritized_dispatcher.h"
 
 namespace net {
 
 // For each hostname that is requested, HostResolver creates a
-// HostResolverImpl::Job. This job gets dispatched to a thread in the global
-// WorkerPool, where it runs SystemHostResolverProc(). If requests for that same
-// host are made while the job is already outstanding, then they are attached
-// to the existing job rather than creating a new one. This avoids doing
-// parallel resolves for the same host.
+// HostResolverImpl::Job. When this job gets dispatched it creates a ProcJob
+// 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
+// resolves for the same host.
 //
 // The way these classes fit together is illustrated by:
 //
@@ -41,69 +44,73 @@ namespace net {
 //   Request ... Request  Request ... Request   Request ... Request
 //  (port1)     (port2)  (port3)      (port4)  (port5)      (portX)
 //
-//
-// When a HostResolverImpl::Job finishes its work in the threadpool, the
-// callbacks of each waiting request are run on the origin thread.
+// When a HostResolverImpl::Job finishes, the callbacks of each waiting request
+// are run on the origin thread.
 //
 // Thread safety: This class is not threadsafe, and must only be called
 // from one thread!
 //
-// The HostResolverImpl enforces |max_jobs_| as the maximum number of concurrent
-// threads.
+// The HostResolverImpl enforces limits on the maximum number of concurrent
+// threads using PrioritizedDispatcher::Limits.
 //
-// Requests are ordered in the queue based on their priority.
+// Jobs are ordered in the queue based on their priority and order of arrival.
 //
-// Whenever we try to resolve the host, we post a delayed task to check if host
-// resolution (OnLookupComplete) is completed or not. If the original attempt
-// hasn't completed, then we start another attempt for host resolution. We take
-// the results from the first attempt that finishes and ignore the results from
-// all other attempts.
-
 class NET_EXPORT HostResolverImpl
     : public HostResolver,
       NON_EXPORTED_BASE(public base::NonThreadSafe),
       public NetworkChangeNotifier::IPAddressObserver,
-      public NetworkChangeNotifier::DNSObserver {
+      public NetworkChangeNotifier::DNSObserver,
+      public base::SupportsWeakPtr<HostResolverImpl> {
  public:
-  // The index into |job_pools_| for the various job pools. Pools with a higher
-  // index have lower priority.
-  //
-  // Note: This is currently unused, since there is a single pool
-  //       for all requests.
-  enum JobPoolIndex {
-    POOL_NORMAL = 0,
-    POOL_COUNT,
-  };
-
-  // Creates a HostResolver that first uses the local cache |cache|, and then
-  // falls back to |resolver_proc|.
-  //
-  // If |cache| is NULL, then no caching is used. Otherwise we take
-  // ownership of the |cache| pointer, and will free it during destructor.
+  // Parameters for ProcTask which resolves hostnames using HostResolveProc.
   //
   // |resolver_proc| is used to perform the actual resolves; it must be
   // thread-safe since it is run from multiple worker threads. If
   // |resolver_proc| is NULL then the default host resolver procedure is
   // used (which is SystemHostResolverProc except if overridden).
-  // |max_jobs| specifies the maximum number of threads that the host resolver
-  // will use (not counting potential duplicate attempts). Use
-  // SetPoolConstraints() to specify finer-grain settings.
-  // |max_retry_attempts| is the maximum number of times we will retry for host
-  // resolution. Pass HostResolver::kDefaultRetryAttempts to choose a default
-  // value.
   //
   // For each attempt, we could start another attempt if host is not resolved
-  // within unresponsive_delay_ time. We keep attempting to resolve the host
-  // for max_retry_attempts. For every retry attempt, we grow the
-  // unresponsive_delay_ by the retry_factor_ amount (that is retry interval is
-  // multiplied by the retry factor each time). Once we have retried
-  // max_retry_attempts, we give up on additional attempts.
+  // within |unresponsive_delay| time. We keep attempting to resolve the host
+  // for |max_retry_attempts|. For every retry attempt, we grow the
+  // |unresponsive_delay| by the |retry_factor| amount (that is retry interval
+  // is multiplied by the retry factor each time). Once we have retried
+  // |max_retry_attempts|, we give up on additional attempts.
+  //
+  struct NET_EXPORT_PRIVATE ProcTaskParams {
+    // Sets up defaults.
+    ProcTaskParams(HostResolverProc* resolver_proc, size_t max_retry_attempts);
+
+    ~ProcTaskParams();
+
+    // The procedure to use for resolving host names. This will be NULL, except
+    // in the case of unit-tests which inject custom host resolving behaviors.
+    scoped_refptr<HostResolverProc> resolver_proc;
+
+    // Maximum number retry attempts to resolve the hostname.
+    // Pass HostResolver::kDefaultRetryAttempts to choose a default value.
+    size_t max_retry_attempts;
+
+    // This is the limit after which we make another attempt to resolve the host
+    // if the worker thread has not responded yet.
+    base::TimeDelta unresponsive_delay;
+
+    // Factor to grow |unresponsive_delay| when we re-re-try.
+    uint32 retry_factor;
+  };
+
+  // Creates a HostResolver that first uses the local cache |cache|, and then
+  // falls back to |proc_params.resolver_proc|.
+  //
+  // If |cache| is NULL, then no caching is used. Otherwise we take
+  // 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).
   //
   // |net_log| must remain valid for the life of the HostResolverImpl.
-  HostResolverImpl(HostResolverProc* resolver_proc,
-                   HostCache* cache,
-                   size_t max_jobs,
-                   size_t max_retry_attempts,
+  HostResolverImpl(HostCache* cache,
+                   const PrioritizedDispatcher::Limits& job_limits,
+                   const ProcTaskParams& proc_params,
                    NetLog* net_log);
 
   // If any completion callbacks are pending when the resolver is destroyed,
@@ -111,20 +118,9 @@ class NET_EXPORT HostResolverImpl
   // be called.
   virtual ~HostResolverImpl();
 
-  // Applies a set of constraints for requests that belong to the specified
-  // pool. NOTE: Don't call this after requests have been already been started.
-  //
-  //  |pool_index| -- Specifies which pool these constraints should be applied
-  //                  to.
-  //  |max_outstanding_jobs| -- How many concurrent jobs are allowed for this
-  //                            pool.
-  //  |max_pending_requests| -- How many requests can be enqueued for this pool
-  //                            before we start dropping requests. Dropped
-  //                            requests fail with
-  //                            ERR_HOST_RESOLVER_QUEUE_TOO_LARGE.
-  void SetPoolConstraints(JobPoolIndex pool_index,
-                          size_t max_outstanding_jobs,
-                          size_t max_pending_requests);
+  // Configures maximum number of Jobs in the queue. Exposed for testing.
+  // Only allowed when the queue is empty.
+  void SetMaxQueuedJobs(size_t value);
 
   // HostResolver methods:
   virtual int Resolve(const RequestInfo& info,
@@ -142,19 +138,13 @@ class NET_EXPORT HostResolverImpl
   virtual HostCache* GetHostCache() OVERRIDE;
 
  private:
-  // Allow tests to access our innards for testing purposes.
-  friend class LookupAttemptHostResolverProc;
-
-  // Allow tests to access our innards for testing purposes.
-  FRIEND_TEST_ALL_PREFIXES(HostResolverImplTest, MultipleAttempts);
-
   class Job;
-  class JobPool;
+  class ProcTask;
   class IPv6ProbeJob;
   class Request;
-  typedef std::vector<Request*> RequestsList;
   typedef HostCache::Key Key;
-  typedef std::map<Key, scoped_refptr<Job> > JobMap;
+  typedef std::map<Key, Job*> JobMap;
+  typedef std::vector<Request*> RequestsList;
 
   // Helper used by |Resolve()| and |ResolveFromCache()|.  Performs IP
   // literal and cache lookup, returns OK if successful,
@@ -181,153 +171,68 @@ class NET_EXPORT HostResolverImpl
                       int* net_error,
                       AddressList* addresses);
 
-  // Returns the HostResolverProc to use for this instance.
-  HostResolverProc* effective_resolver_proc() const {
-    return resolver_proc_ ?
-        resolver_proc_.get() : HostResolverProc::GetDefault();
-  }
-
-  // Adds a job to outstanding jobs list.
-  void AddOutstandingJob(Job* job);
-
-  // Returns the outstanding job for |key|, or NULL if there is none.
-  Job* FindOutstandingJob(const Key& key);
-
-  // Removes |job| from the outstanding jobs list.
-  void RemoveOutstandingJob(Job* job);
+  // The logging routines are defined here because some requests are resolved
+  // without a Request object.
 
-  // Callback for when |job| has completed with |net_error| and |addrlist|.
-  void OnJobComplete(Job* job, int net_error, int os_error,
-                     const AddressList& addrlist);
+  // Logs when a request has just been started.
+  static void LogStartRequest(const BoundNetLog& source_net_log,
+                              const BoundNetLog& request_net_log,
+                              const RequestInfo& info);
 
-  // Aborts |job|.  Same as OnJobComplete() except does not remove |job|
-  // from |jobs_| and does not cache the result (ERR_ABORTED).
-  void AbortJob(Job* job);
+  // Logs when a request has just completed (before its callback is run).
+  static void LogFinishRequest(const BoundNetLog& source_net_log,
+                               const BoundNetLog& request_net_log,
+                               const RequestInfo& info,
+                               int net_error,
+                               int os_error);
 
-  // Used by both OnJobComplete() and AbortJob();
-  void OnJobCompleteInternal(Job* job, int net_error, int os_error,
-                             const AddressList& addrlist);
+  // Logs when a request has been cancelled.
+  static void LogCancelRequest(const BoundNetLog& source_net_log,
+                               const BoundNetLog& request_net_log,
+                               const RequestInfo& info);
 
-  // Called when a request has just been started.
-  void OnStartRequest(const BoundNetLog& source_net_log,
-                      const BoundNetLog& request_net_log,
-                      const RequestInfo& info);
-
-  // Called when a request has just completed (before its callback is run).
-  void OnFinishRequest(const BoundNetLog& source_net_log,
-                       const BoundNetLog& request_net_log,
-                       const RequestInfo& info,
-                       int net_error,
-                       int os_error);
-
-  // Called when a request has been cancelled.
-  void OnCancelRequest(const BoundNetLog& source_net_log,
-                       const BoundNetLog& request_net_log,
-                       const RequestInfo& info);
-
-  // Notify IPv6ProbeJob not to call back, and discard reference to the job.
+  // Notifies IPv6ProbeJob not to call back, and discard reference to the job.
   void DiscardIPv6ProbeJob();
 
   // Callback from IPv6 probe activity.
   void IPv6ProbeSetDefaultAddressFamily(AddressFamily address_family);
 
-  // Returns true if the constraints for |pool| are met, and a new job can be
-  // created for this pool.
-  bool CanCreateJobForPool(const JobPool& pool) const;
-
-  // Returns the index of the pool that request |req| maps to.
-  static JobPoolIndex GetJobPoolIndexForRequest(const Request* req);
-
-  JobPool* GetPoolForRequest(const Request* req) {
-    return job_pools_[GetJobPoolIndexForRequest(req)];
-  }
-
-  // Starts up to 1 job given the current pool constraints. This job
-  // may have multiple requests attached to it.
-  void ProcessQueuedRequests();
-
   // 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) const;
 
-  // Attaches |req| to a new job, and starts it. Returns that job.
-  Job* CreateAndStartJob(Request* req);
+  // 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);
 
-  // Adds a pending request |req| to |pool|.
-  int EnqueueRequest(JobPool* pool, Request* req);
+  // Removes |job| from |jobs_|.
+  void RemoveJob(Job* job);
 
-  // Cancels all jobs.
-  void CancelAllJobs();
-
-  // Aborts all in progress jobs (but might start new ones).
+  // Aborts all in progress jobs and notifies their requests.
+  // Might start new jobs.
   void AbortAllInProgressJobs();
 
   // NetworkChangeNotifier::IPAddressObserver methods:
   virtual void OnIPAddressChanged() OVERRIDE;
 
-  // Helper methods to get and set max_retry_attempts_.
-  size_t max_retry_attempts() const {
-    return max_retry_attempts_;
-  }
-  void set_max_retry_attempts(const size_t max_retry_attempts) {
-    max_retry_attempts_ = max_retry_attempts;
-  }
-
-  // Helper methods for unit tests to get and set unresponsive_delay_.
-  base::TimeDelta unresponsive_delay() const { return unresponsive_delay_; }
-  void set_unresponsive_delay(const base::TimeDelta& unresponsive_delay) {
-    unresponsive_delay_ = unresponsive_delay;
-  }
-
-  // Helper methods to get and set retry_factor_.
-  uint32 retry_factor() const {
-    return retry_factor_;
-  }
-  void set_retry_factor(const uint32 retry_factor) {
-    retry_factor_ = retry_factor;
-  }
-
   // NetworkChangeNotifier::OnDNSChanged methods:
   virtual void OnDNSChanged() OVERRIDE;
 
   // Cache of host resolution results.
   scoped_ptr<HostCache> cache_;
 
-  // Map from hostname to outstanding job.
+  // Map from cache key to a job.

[cbentzel, 2012/01/09 16:05:18]

Perhaps HostCache::Key in comment.

   JobMap jobs_;
 
-  // Maximum number of concurrent jobs allowed, across all pools. Each job may
-  // create multiple concurrent resolve attempts for the hostname.
-  size_t max_jobs_;
-
-  // Maximum number retry attempts to resolve the hostname.
-  size_t max_retry_attempts_;
-
-  // This is the limit after which we make another attempt to resolve the host
-  // if the worker thread has not responded yet. Allow unit tests to change the
-  // value.
-  base::TimeDelta unresponsive_delay_;
-
-  // Factor to grow unresponsive_delay_ when we re-re-try. Allow unit tests to
-  // change the value.
-  uint32 retry_factor_;
-
-  // The information to track pending requests for a JobPool, as well as
-  // how many outstanding jobs the pool already has, and its constraints.
-  JobPool* job_pools_[POOL_COUNT];
-
-  // The job that OnJobComplete() is currently processing (needed in case
-  // HostResolver gets deleted from within the callback).
-  scoped_refptr<Job> cur_completing_job_;
+  // Starts Jobs according to their priority and the configured limits.
+  PrioritizedDispatcher dispatcher_;
 
-  // Monotonically increasing ID number to assign to the next job.
-  // The only consumer of this ID is the requests tracing code.
-  int next_job_id_;
+  // Limit on the maximum number of jobs queued in |dispatcher_|.
+  size_t max_queued_jobs_;
 
-  // The procedure to use for resolving host names. This will be NULL, except
-  // in the case of unit-tests which inject custom host resolving behaviors.
-  scoped_refptr<HostResolverProc> resolver_proc_;
+  // Parameters for ProcTask.
+  ProcTaskParams proc_params_;
 
   // Address family to use when the request doesn't specify one.
   AddressFamily default_address_family_;