Revert "Clean up dns prefetch code, and also port it."

chrome/browser/net/dns_master.h

Index: chrome/browser/net/dns_master.h
diff --git a/chrome/browser/net/dns_master.h b/chrome/browser/net/dns_master.h
index 971f080b881ad79a51ffb2dfa1913f158da0c5f7..f406b320d3c84220c23c37ec0f38f82e85336bb2 100644
--- a/chrome/browser/net/dns_master.h
+++ b/chrome/browser/net/dns_master.h
@@ -3,42 +3,58 @@
 // found in the LICENSE file.
 
 // A DnsMaster object is instantiated once in the browser
-// process, and manages asynchronous resolution of DNS hostnames.
+// process, and delivers DNS prefetch assignments (hostnames)
+// to any of several DnsSlave objects.
 // Most hostname lists are sent out by renderer processes, and
 // involve lists of hostnames that *might* be used in the near
 // future by the browsing user.  The goal of this class is to
 // cause the underlying DNS structure to lookup a hostname before
 // it is really needed, and hence reduce latency in the standard
-// lookup paths.
+// lookup paths.  Since some DNS lookups may take a LONG time, we
+// use several DnsSlave threads to concurrently perform the
+// lookups.
 
 #ifndef CHROME_BROWSER_NET_DNS_MASTER_H_
 #define CHROME_BROWSER_NET_DNS_MASTER_H_
 
 #include <map>
 #include <queue>
-#include <set>
 #include <string>
 
-#include "base/lock.h"
+#include "base/condition_variable.h"
+#include "base/scoped_ptr.h"
 #include "chrome/browser/net/dns_host_info.h"
 #include "chrome/browser/net/referrer.h"
 #include "chrome/common/net/dns.h"
 #include "googleurl/src/url_canon.h"
-#include "testing/gtest/include/gtest/gtest_prod.h"
 
 namespace chrome_browser_net {
 
+class DnsSlave;
+
 typedef chrome_common_net::NameList NameList;
 typedef std::map<std::string, DnsHostInfo> Results;
 
 class DnsMaster {
  public:
-  // Too many concurrent lookups negate benefits of prefetching
-  // by trashing the OS cache.
-  static const size_t kMaxConcurrentLookups;
+  // The number of slave processes that will do DNS prefetching
+  static const size_t kSlaveCountMax = 8;
+
+  explicit DnsMaster(base::TimeDelta shutdown_wait_time);
+
+  ~DnsMaster() {
+    if (!shutdown_)
+      ShutdownSlaves();  // Ensure we did our cleanup.
+  }
 
-  DnsMaster();
-  ~DnsMaster();
+  // ShutdownSlaves() gets all spawned threads to terminate, closes
+  // their handles, and deletes their DnsSlave instances.
+  // Return value of true means all operations succeeded.
+  // Return value of false means that the threads wouldn't terminate,
+  // and that resources may leak.  If this returns false, it is best
+  // to NOT delete this DnsMaster, as slave threads may still call into
+  // this object.
+  bool ShutdownSlaves();
 
   // In some circumstances, for privacy reasons, all results should be
   // discarded.  This method gracefully handles that activity.
@@ -47,7 +63,7 @@ class DnsMaster {
   // (cache hits etc.).
   void DiscardAllResults();
 
-  // Add hostname(s) to the queue for processing.
+  // Add hostname(s) to the queue for processing by slaves
   void ResolveList(const NameList& hostnames,
                    DnsHostInfo::ResolutionMotivation motivation);
   void Resolve(const std::string& hostname,
@@ -72,44 +88,59 @@ class DnsMaster {
   // domains.
   void GetHtmlInfo(std::string* output);
 
- private:
-  FRIEND_TEST(DnsMasterTest, BenefitLookupTest);
-  FRIEND_TEST(DnsMasterTest, ShutdownWhenResolutionIsPendingTest);
-  FRIEND_TEST(DnsMasterTest, SingleLookupTest);
-  FRIEND_TEST(DnsMasterTest, ConcurrentLookupTest);
-  FRIEND_TEST(DnsMasterTest, MassiveConcurrentLookupTest);
-  friend class WaitForResolutionHelper;  // For testing.
-
-  class LookupRequest;
-
-  // A map that is keyed with the hostnames that we've learned were the cause
-  // of loading additional hostnames.  The list of additional hostnames in held
-  // in a Referrer instance, which is found in this type.
-  typedef std::map<std::string, Referrer> Referrers;
-
-  // Only for testing. Returns true if hostname has been successfully resolved
-  // (name found).
+  // For testing only...
+  // Currently testing only provides a crude measure of success.
   bool WasFound(const std::string& hostname) {
     AutoLock auto_lock(lock_);
     return (results_.find(hostname) != results_.end()) &&
             results_[hostname].was_found();
   }
 
-  // Only for testing. Return how long was the resolution
-  // or DnsHostInfo::kNullDuration if it hasn't been resolved yet.
-  base::TimeDelta GetResolutionDuration(const std::string& hostname) {
+  // Accessor methods, used mostly for testing.
+  // Both functions return DnsHostInfo::kNullDuration if name was not yet
+  // processed enough.
+  base::TimeDelta GetResolutionDuration(const std::string hostname) {
     AutoLock auto_lock(lock_);
     if (results_.find(hostname) == results_.end())
       return DnsHostInfo::kNullDuration;
     return results_[hostname].resolve_duration();
   }
 
-  // Only for testing;
-  size_t peak_pending_lookups() const { return peak_pending_lookups_; }
+  base::TimeDelta GetQueueDuration(const std::string hostname) {
+    AutoLock auto_lock(lock_);
+    if (results_.find(hostname) == results_.end())
+      return DnsHostInfo::kNullDuration;
+    return results_[hostname].queue_duration();
+  }
+
+  size_t running_slave_count() {
+    AutoLock auto_lock(lock_);
+    return running_slave_count_;
+  }
+
+  //----------------------------------------------------------------------------
+  // Methods below this line should only be called by slave processes.
+
+  // GetNextAssignment() gets the next hostname from queue for processing
+  // It is not meant to be public, and should only be used by the slave.
+  // GetNextAssignment() waits on a condition variable if there are no more
+  // names in queue.
+  // Return false if slave thread should terminate.
+  // Return true if slave thread should process the value.
+  bool GetNextAssignment(std::string* hostname);
+
+  // Access methods for use by slave threads to callback with state updates.
+  void SetFoundState(const std::string hostname);
+  void SetNoSuchNameState(const std::string hostname);
 
-  // Access method for use by lookup request to pass resolution result.
-  void OnLookupFinished(LookupRequest* request,
-                        const std::string& hostname, bool found);
+  // Notification during ShutdownSlaves.
+  void SetSlaveHasTerminated(int slave_index);
+
+ private:
+  // A map that is keyed with the hostnames that we've learned were the cause
+  // of loading additional hostnames.  The list of additional hostnames in held
+  // in a Referrer instance, which is found in this type.
+  typedef std::map<std::string, Referrer> Referrers;
 
   // "PreLocked" means that the caller has already Acquired lock_ in the
   // following method names.
@@ -117,17 +148,12 @@ class DnsMaster {
   // to the queued instance, otherwise return NULL.
   DnsHostInfo* PreLockedResolve(const std::string& hostname,
                                 DnsHostInfo::ResolutionMotivation motivation);
+  bool PreLockedCreateNewSlaveIfNeeded();  // Lazy slave processes creation.
 
-  // Take lookup requests from name_buffer_ and tell HostResolver
-  // to look them up asynchronously, provided we don't exceed
-  // concurrent resolution limit.
-  void PreLockedScheduleLookups();
+  // Number of slave processes started early (to help with startup prefetch).
+  static const size_t kSlaveCountMin = 4;
 
-  // Synchronize access to results_, referrers_, pending_lookups_.
-  // TODO(phajdan.jr): Re-evaluate usage of this lock. After restructuring
-  // the code to run only on the IO thread and using PostTask from other threads
-  // the lock should not be needed. If that's not possible, then at least its
-  // usage can be reduced.
+  // Synchronize access to results_, referrers_, and slave control data.
   Lock lock_;
 
   // name_buffer_ holds a list of names we need to look up.
@@ -141,10 +167,30 @@ class DnsMaster {
   // pre-resolve when there is a navigation to the orginial hostname.
   Referrers referrers_;
 
-  std::set<LookupRequest*> pending_lookups_;
-
-  // For testing, to verify that we don't exceed the limit.
-  size_t peak_pending_lookups_;
+  // Signaling slaves to process elements in the queue, or to terminate,
+  // is done using ConditionVariables.
+  ConditionVariable slaves_have_work_;
+
+  size_t slave_count_;  // Count of slave processes started.
+  size_t running_slave_count_;  // Count of slaves process still running.
+
+  // TODO(jrg): wait for CL 15076 from _ph to come in which resolves
+  // this.  In the short term this file is hacked to be happy when
+  // included in render_process.h.
+#if defined(OS_WIN)
+  // The following arrays are only initialized as
+  // slave_count_ grows (up to the indicated max).
+  DWORD thread_ids_[kSlaveCountMax];
+  HANDLE thread_handles_[kSlaveCountMax];
+  DnsSlave* slaves_[kSlaveCountMax];
+#endif
+  
+  // shutdown_ is set to tell the slaves to terminate.
+  bool shutdown_;
+
+  // The following is the maximum time the ShutdownSlaves method
+  // will wait for all the slave processes to terminate.
+  const base::TimeDelta kShutdownWaitTime_;
 
   // A list of successful events resulting from pre-fetching.
   DnsHostInfo::DnsInfoTable cache_hits_;