Refactorings surrounding HostResolver:...

net/base/host_resolver.h

Index: net/base/host_resolver.h
===================================================================
--- net/base/host_resolver.h	(revision 20760)
+++ net/base/host_resolver.h	(working copy)
@@ -6,57 +6,26 @@
 #define NET_BASE_HOST_RESOLVER_H_
 
 #include <string>
-#include <vector>
 
-#include "base/basictypes.h"
-#include "base/lock.h"
 #include "base/ref_counted.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/completion_callback.h"
-#include "net/base/host_cache.h"
 
 class MessageLoop;
 
 namespace net {
 
 class AddressList;
-class HostMapper;
 
 // This class represents the task of resolving hostnames (or IP address
 // literal) to an AddressList object.
 //
-// HostResolver handles multiple requests at a time, so when cancelling a
-// request the Request* handle that was returned by Resolve() needs to be
+// HostResolver can handle multiple requests at a time, so when cancelling a
+// request the RequestHandle that was returned by Resolve() needs to be
 // given.  A simpler alternative for consumers that only have 1 outstanding
 // request at a time is to create a SingleRequestHostResolver wrapper around
 // HostResolver (which will automatically cancel the single request when it
 // goes out of scope).
-//
-// For each hostname that is requested, HostResolver creates a
-// HostResolver::Job. This job gets dispatched to a thread in the global
-// WorkerPool, where it runs "getaddrinfo(hostname)". 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.
-//
-// The way these classes fit together is illustrated by:
-//
-//
-//            +------------- HostResolver ---------------+
-//            |                    |                     |
-//           Job                  Job                   Job
-//       (for host1)          (for host2)           (for hostX)
-//       /    |   |            /   |   |             /   |   |
-//   Request ... Request  Request ... Request   Request ... Request
-//  (port1)     (port2)  (port3)      (port4)  (port5)      (portX)
-//
-//
-// When a HostResolver::Job finishes its work in the threadpool, 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!
-//
 class HostResolver : public base::RefCounted<HostResolver> {
  public:
   // The parameters for doing a Resolve(). |hostname| and |port| are required,
@@ -120,21 +89,16 @@
     virtual void OnCancelResolution(int id, const RequestInfo& info) = 0;
   };
 
-  // Creates a HostResolver that caches up to |max_cache_entries| for
-  // |cache_duration_ms| milliseconds.
-  //
-  // TODO(eroman): Get rid of the default parameters as it violate google
-  // style. This is temporary to help with refactoring.
-  HostResolver(int max_cache_entries = 100, int cache_duration_ms = 60000);
+  // Opaque type used to cancel a request.
+  typedef void* RequestHandle;
 
+  HostResolver() {}
+
   // If any completion callbacks are pending when the resolver is destroyed,
   // the host resolutions are cancelled, and the completion callbacks will not
   // be called.
-  ~HostResolver();
+  virtual ~HostResolver() {}
 
-  // Opaque type used to cancel a request.
-  class Request;
-
   // 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
@@ -147,75 +111,24 @@
   // result code will be passed to the completion callback. If |req| is
   // non-NULL, then |*req| will be filled with a handle to the async request.
   // This handle is not valid after the request has completed.
-  int Resolve(const RequestInfo& info, AddressList* addresses,
-              CompletionCallback* callback, Request** req);
+  virtual int Resolve(const RequestInfo& info, AddressList* addresses,
+                      CompletionCallback* callback, RequestHandle* out_req) = 0;
 
   // Cancels the specified request. |req| is the handle returned by Resolve().
   // After a request is cancelled, its completion callback will not be called.
-  void CancelRequest(Request* req);
+  virtual void CancelRequest(RequestHandle req) = 0;
 
   // Adds an observer to this resolver. The observer will be notified of the
   // start and completion of all requests (excluding cancellation). |observer|
   // must remain valid for the duration of this HostResolver's lifetime.
-  void AddObserver(Observer* observer);
+  virtual void AddObserver(Observer* observer) = 0;
 
   // Unregisters an observer previously added by AddObserver().
-  void RemoveObserver(Observer* observer);
+  virtual void RemoveObserver(Observer* observer) = 0;
 
   // TODO(eroman): temp hack for http://crbug.com/15513
-  void Shutdown();
+  virtual void Shutdown() = 0;
 
- private:
-  class Job;
-  typedef std::vector<Request*> RequestsList;
-  typedef base::hash_map<std::string, scoped_refptr<Job> > JobMap;
-  typedef std::vector<Observer*> ObserversList;
-
-  // Adds a job to outstanding jobs list.
-  void AddOutstandingJob(Job* job);
-
-  // Returns the outstanding job for |hostname|, or NULL if there is none.
-  Job* FindOutstandingJob(const std::string& hostname);
-
-  // Removes |job| from the outstanding jobs list.
-  void RemoveOutstandingJob(Job* job);
-
-  // Callback for when |job| has completed with |error| and |addrlist|.
-  void OnJobComplete(Job* job, int error, const AddressList& addrlist);
-
-  // Notify all observers of the start of a resolve request.
-  void NotifyObserversStartRequest(int request_id,
-                                   const RequestInfo& info);
-
-  // Notify all observers of the completion of a resolve request.
-  void NotifyObserversFinishRequest(int request_id,
-                                    const RequestInfo& info,
-                                    int error);
-
-  // Notify all observers of the cancellation of a resolve request.
-  void NotifyObserversCancelRequest(int request_id,
-                                    const RequestInfo& info);
-
-  // Cache of host resolution results.
-  HostCache cache_;
-
-  // Map from hostname to outstanding job.
-  JobMap jobs_;
-
-  // The job that OnJobComplete() is currently processing (needed in case
-  // HostResolver gets deleted from within the callback).
-  scoped_refptr<Job> cur_completing_job_;
-
-  // The observers to notify when a request starts/ends.
-  ObserversList observers_;
-
-  // Monotonically increasing ID number to assign to the next request.
-  // Observers are the only consumers of this ID number.
-  int next_request_id_;
-
-  // TODO(eroman): temp hack for http://crbug.com/15513
-  bool shutdown_;
-
   DISALLOW_COPY_AND_ASSIGN(HostResolver);
 };
 
@@ -245,7 +158,7 @@
   scoped_refptr<HostResolver> resolver_;
 
   // The current request (if any).
-  HostResolver::Request* cur_request_;
+  HostResolver::RequestHandle cur_request_;
   CompletionCallback* cur_request_callback_;
 
   // Completion callback for when request to |resolver_| completes.
@@ -254,47 +167,11 @@
   DISALLOW_COPY_AND_ASSIGN(SingleRequestHostResolver);
 };
 
-// A helper class used in unit tests to alter hostname mappings.  See
-// SetHostMapper for details.
-class HostMapper : public base::RefCountedThreadSafe<HostMapper> {
- public:
-  virtual ~HostMapper() {}
+// Creates a HostResolver implementation that queries the underlying system.
+// (Except if a unit-test has changed the global HostResolverProc using
+// ScopedHostResolverProc to intercept requests to the system).
+HostResolver* CreateSystemHostResolver();
 
-  // Returns possibly altered hostname, or empty string to simulate
-  // a failed lookup.
-  virtual std::string Map(const std::string& host) = 0;
-
- protected:
-  // Ask previous host mapper (if set) for mapping of given host.
-  std::string MapUsingPrevious(const std::string& host);
-
- private:
-  friend class ScopedHostMapper;
-
-  // Set mapper to ask when this mapper doesn't want to modify the result.
-  void set_previous_mapper(HostMapper* mapper) {
-    previous_mapper_ = mapper;
-  }
-
-  scoped_refptr<HostMapper> previous_mapper_;
-};
-
-#ifdef UNIT_TEST
-// This function is designed to allow unit tests to override the behavior of
-// HostResolver.  For example, a HostMapper instance can force all hostnames
-// to map to a fixed IP address such as 127.0.0.1.
-//
-// The previously set HostMapper (or NULL if there was none) is returned.
-//
-// NOTE: This function is not thread-safe, so take care to only call this
-// function while there are no outstanding HostResolver instances.
-//
-// NOTE: In most cases, you should use ScopedHostMapper instead, which is
-// defined in host_resolver_unittest.h
-//
-HostMapper* SetHostMapper(HostMapper* host_mapper);
-#endif
-
 }  // namespace net
 
 #endif  // NET_BASE_HOST_RESOLVER_H_