Clean up dns prefetch code, and also port it.

chrome/browser/net/dns_master.cc

Index: chrome/browser/net/dns_master.cc
diff --git a/chrome/browser/net/dns_master.cc b/chrome/browser/net/dns_master.cc
index d58bb4bd251573b822f3ae4e70c6d85f976d8002..f2a584ec00b0eae3307610ced11a1d3c0e384d9f 100644
--- a/chrome/browser/net/dns_master.cc
+++ b/chrome/browser/net/dns_master.cc
@@ -2,61 +2,87 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// See header file for description of class
-
 #include "chrome/browser/net/dns_master.h"
 
+#include <algorithm>
 #include <set>
+#include <sstream>
 
+#include "base/compiler_specific.h"
 #include "base/histogram.h"
+#include "base/lock.h"
+#include "base/ref_counted.h"
 #include "base/stats_counters.h"
 #include "base/string_util.h"
-#include "base/thread.h"
-#include "base/win_util.h"
-#include "chrome/browser/net/dns_slave.h"
-
-using base::TimeDelta;
+#include "base/time.h"
+#include "net/base/address_list.h"
+#include "net/base/completion_callback.h"
+#include "net/base/host_resolver.h"
+#include "net/base/net_errors.h"
 
 namespace chrome_browser_net {
 
-DnsMaster::DnsMaster(TimeDelta shutdown_wait_time)
-  : slaves_have_work_(&lock_),
-    slave_count_(0),
-    running_slave_count_(0),
-    shutdown_(false),
-    kShutdownWaitTime_(shutdown_wait_time) {
-  for (size_t i = 0; i < kSlaveCountMax; i++) {
-    thread_ids_[i] = 0;
-    thread_handles_[i] = 0;
-    slaves_[i] = NULL;
+// static
+const size_t DnsMaster::kMaxConcurrentLookups = 8;
+
+class DnsMaster::LookupRequest {
+ public:
+  LookupRequest(DnsMaster* master, const std::string& hostname)
+      : ALLOW_THIS_IN_INITIALIZER_LIST(
+          net_callback_(this, &LookupRequest::OnLookupFinished)),
+        master_(master),
+        hostname_(hostname) {
   }
+
+  bool Start() {
+    const int result = resolver_.Resolve(hostname_, 80, &addresses_,
+                                         &net_callback_);
+    return (result == net::ERR_IO_PENDING);
+  }
+
+ private:
+  void OnLookupFinished(int result) {
+    master_->OnLookupFinished(this, hostname_, result == net::OK);
+  }
+
+  // HostResolver will call us using this callback when resolution is complete.
+  net::CompletionCallbackImpl<LookupRequest> net_callback_;
+
+  DnsMaster* master_;  // Master which started us.
+
+  const std::string hostname_;  // Hostname to resolve.
+  net::HostResolver resolver_;
+  net::AddressList addresses_;
+
+  DISALLOW_COPY_AND_ASSIGN(LookupRequest);
+};
+
+DnsMaster::DnsMaster() : peak_pending_lookups_(0), shutdown_(false) {
+}
+
+DnsMaster::~DnsMaster() {
+  DCHECK(shutdown_);
+}
+
+void DnsMaster::Shutdown() {
+  AutoLock auto_lock(lock_);
+
+  DCHECK(!shutdown_);
+  shutdown_ = true;
+
+  std::set<LookupRequest*>::iterator it;
+  for (it = pending_lookups_.begin(); it != pending_lookups_.end(); ++it)
+    delete *it;
 }
 
 // Overloaded Resolve() to take a vector of names.
 void DnsMaster::ResolveList(const NameList& hostnames,
                             DnsHostInfo::ResolutionMotivation motivation) {
-  bool need_to_signal = false;
-  {
-    AutoLock auto_lock(lock_);
-    if (shutdown_) return;
-    if (slave_count_ < kSlaveCountMin) {
-      for (int target_count = std::min(hostnames.size(), kSlaveCountMin);
-           target_count > 0;
-           target_count--)
-        PreLockedCreateNewSlaveIfNeeded();
-    } else {
-      PreLockedCreateNewSlaveIfNeeded();  // Allocate one per list call.
-    }
+  AutoLock auto_lock(lock_);
 
-    for (NameList::const_iterator it = hostnames.begin();
-      it < hostnames.end();
-         it++) {
-      if (PreLockedResolve(*it, motivation))
-        need_to_signal = true;
-    }
-  }
-  if (need_to_signal)
-    slaves_have_work_.Signal();
+  NameList::const_iterator it;
+  for (it = hostnames.begin(); it < hostnames.end(); ++it)
+    PreLockedResolve(*it, motivation);
 }
 
 // Basic Resolve() takes an invidual name, and adds it
@@ -65,16 +91,8 @@ void DnsMaster::Resolve(const std::string& hostname,
                         DnsHostInfo::ResolutionMotivation motivation) {
   if (0 == hostname.length())
     return;
-  bool need_to_signal = false;
-  {
-    AutoLock auto_lock(lock_);
-    if (shutdown_) return;
-    PreLockedCreateNewSlaveIfNeeded();  // Allocate one at a time.
-    if (PreLockedResolve(hostname, motivation))
-      need_to_signal = true;
-  }
-  if (need_to_signal)
-    slaves_have_work_.Signal();
+  AutoLock auto_lock(lock_);
+  PreLockedResolve(hostname, motivation);
 }
 
 bool DnsMaster::AccruePrefetchBenefits(const GURL& referrer,
@@ -147,26 +165,19 @@ void DnsMaster::NonlinkNavigation(const GURL& referrer,
 }
 
 void DnsMaster::NavigatingTo(const std::string& host_name) {
-  bool need_to_signal = false;
-  {
-    AutoLock auto_lock(lock_);
-    Referrers::iterator it = referrers_.find(host_name);
-    if (referrers_.end() == it)
-      return;
-    Referrer* referrer = &(it->second);
-    for (Referrer::iterator future_host = referrer->begin();
-         future_host != referrer->end(); ++future_host) {
-      DnsHostInfo* queued_info = PreLockedResolve(
-          future_host->first,
-          DnsHostInfo::LEARNED_REFERAL_MOTIVATED);
-      if (queued_info) {
-        need_to_signal = true;
-        queued_info->SetReferringHostname(host_name);
-      }
-    }
+  AutoLock auto_lock(lock_);
+  Referrers::iterator it = referrers_.find(host_name);
+  if (referrers_.end() == it)
+    return;
+  Referrer* referrer = &(it->second);
+  for (Referrer::iterator future_host = referrer->begin();
+       future_host != referrer->end(); ++future_host) {
+    DnsHostInfo* queued_info = PreLockedResolve(
+        future_host->first,
+        DnsHostInfo::LEARNED_REFERAL_MOTIVATED);
+    if (queued_info)
+      queued_info->SetReferringHostname(host_name);
   }
-  if (need_to_signal)
-    slaves_have_work_.Signal();
 }
 
 // Provide sort order so all .com's are together, etc.
@@ -297,7 +308,7 @@ void DnsMaster::GetHtmlInfo(std::string* output) {
     }
     if (!it->second.was_found())
       continue;  // Still being processed.
-    if (TimeDelta() != it->second.benefits_remaining()) {
+    if (base::TimeDelta() != it->second.benefits_remaining()) {
       network_hits.push_back(it->second);  // With no benefit yet.
       continue;
     }
@@ -332,9 +343,11 @@ DnsHostInfo* DnsMaster::PreLockedResolve(
     const std::string& hostname,
     DnsHostInfo::ResolutionMotivation motivation) {
   // DCHECK(We have the lock);
-  DCHECK(0 != slave_count_);
   DCHECK(0 != hostname.length());
 
+  if (shutdown_)
+    return NULL;
+
   DnsHostInfo* info = &results_[hostname];
   info->SetHostname(hostname);  // Initialize or DCHECK.
   // TODO(jar):  I need to discard names that have long since expired.
@@ -349,151 +362,52 @@ DnsHostInfo* DnsMaster::PreLockedResolve(
 
   info->SetQueuedState(motivation);
   name_buffer_.push(hostname);
+
+  PreLockedScheduleLookups();
+
   return info;
 }
 
-// GetNextAssignment() is executed on the thread associated with
-// with a prefetch slave instance.
-// Return value of false indicates slave thread termination is needed.
-// Return value of true means info argument was populated
-// with a pointer to the assigned DnsHostInfo instance.
-bool DnsMaster::GetNextAssignment(std::string* hostname) {
-  bool ask_for_help = false;
-  {
-    AutoLock auto_lock(lock_);  // For map and buffer access
-    while (0 == name_buffer_.size() && !shutdown_) {
-      // No work pending, so just wait.
-      // wait on condition variable while releasing lock temporarilly.
-      slaves_have_work_.Wait();
-    }
-    if (shutdown_)
-      return false;  // Tell slaves to terminate also.
-    *hostname = name_buffer_.front();
+void DnsMaster::PreLockedScheduleLookups() {
+  while (!name_buffer_.empty() &&
+         pending_lookups_.size() < kMaxConcurrentLookups) {
+    const std::string hostname(name_buffer_.front());
     name_buffer_.pop();
 
-    DnsHostInfo* info = &results_[*hostname];
-    DCHECK(info->HasHostname(*hostname));
+    DnsHostInfo* info = &results_[hostname];
+    DCHECK(info->HasHostname(hostname));
     info->SetAssignedState();
 
-    ask_for_help = name_buffer_.size() > 0;
-  }  // Release lock_
-  if (ask_for_help)
-    slaves_have_work_.Signal();
-  return true;
-}
-
-void DnsMaster::SetFoundState(const std::string hostname) {
-  AutoLock auto_lock(lock_);  // For map access (changing info values).
-  DnsHostInfo* info = &results_[hostname];
-  DCHECK(info->HasHostname(hostname));
-  if (info->is_marked_to_delete())
-    results_.erase(hostname);
-  else
-    info->SetFoundState();
+    LookupRequest* request = new LookupRequest(this, hostname);
+    if (request->Start()) {
+      pending_lookups_.insert(request);
+      peak_pending_lookups_ = std::max(peak_pending_lookups_,
+                                       pending_lookups_.size());
+    } else {
+      NOTREACHED();
+      delete request;
+    }
+  }
 }
 
-void DnsMaster::SetNoSuchNameState(const std::string hostname) {
+void DnsMaster::OnLookupFinished(LookupRequest* request,
+                                 const std::string& hostname, bool found) {
   AutoLock auto_lock(lock_);  // For map access (changing info values).
   DnsHostInfo* info = &results_[hostname];
   DCHECK(info->HasHostname(hostname));
   if (info->is_marked_to_delete())
     results_.erase(hostname);
-  else
-    info->SetNoSuchNameState();
-}
-
-bool DnsMaster::PreLockedCreateNewSlaveIfNeeded() {
-  // Don't create more than max.
-  if (kSlaveCountMax <= slave_count_  || shutdown_)
-    return false;
-
-  DnsSlave* slave_instance = new DnsSlave(this, slave_count_);
-  DWORD thread_id;
-  size_t stack_size = 0;
-  unsigned int flags = CREATE_SUSPENDED;
-  if (win_util::GetWinVersion() >= win_util::WINVERSION_XP) {
-    // 128kb stack size.
-    stack_size = 128*1024;
-    flags |= STACK_SIZE_PARAM_IS_A_RESERVATION;
+  else {
+    if (found)
+      info->SetFoundState();
+    else
+      info->SetNoSuchNameState();
   }
-  HANDLE handle = CreateThread(NULL,  // security
-                               stack_size,
-                               DnsSlave::ThreadStart,
-                               reinterpret_cast<void*>(slave_instance),
-                               flags,
-                               &thread_id);
-  DCHECK(NULL != handle);
-  if (NULL == handle)
-    return false;
-
-  // Carlos suggests it is not valuable to do a set priority.
-  //  BOOL WINAPI SetThreadPriority(handle,int nPriority);
 
-  thread_ids_[slave_count_] = thread_id;
-  thread_handles_[slave_count_] = handle;
-  slaves_[slave_count_] = slave_instance;
-  slave_count_++;
+  pending_lookups_.erase(request);
+  delete request;
 
-  ResumeThread(handle);  // WINAPI call.
-  running_slave_count_++;
-
-  return true;
-}
-
-void DnsMaster::SetSlaveHasTerminated(int slave_index) {
-  DCHECK_EQ(GetCurrentThreadId(), thread_ids_[slave_index]);
-  AutoLock auto_lock(lock_);
-  running_slave_count_--;
-  DCHECK(thread_ids_[slave_index]);
-  thread_ids_[slave_index] = 0;
-}
-
-bool DnsMaster::ShutdownSlaves() {
-  int running_slave_count;
-  {
-    AutoLock auto_lock(lock_);
-    shutdown_ = true;  // Block additional resolution requests.
-    // Empty the queue gracefully
-    while (name_buffer_.size() > 0) {
-      std::string hostname = name_buffer_.front();
-      name_buffer_.pop();
-      DnsHostInfo* info = &results_[hostname];
-      DCHECK(info->HasHostname(hostname));
-      // We should be in Queued state, so simulate to end of life.
-      info->SetAssignedState();  // Simulate slave assignment.
-      info->SetNoSuchNameState();  // Simulate failed lookup.
-      results_.erase(hostname);
-    }
-    running_slave_count = running_slave_count_;
-    // Release lock, so slaves can finish up.
-  }
-
-  if (running_slave_count) {
-    slaves_have_work_.Broadcast();  // Slaves need to check for termination.
-
-    DWORD result = WaitForMultipleObjects(
-                      slave_count_,
-                      thread_handles_,
-                      TRUE,  // Wait for all
-                      static_cast<DWORD>(kShutdownWaitTime_.InMilliseconds()));
-
-    DCHECK(result != WAIT_TIMEOUT) << "Some slaves didn't stop";
-    if (WAIT_TIMEOUT == result)
-      return false;
-  }
-  {
-    AutoLock auto_lock(lock_);
-    while (0 < slave_count_--) {
-      if (0 == thread_ids_[slave_count_]) {  // Thread terminated.
-        int result = CloseHandle(thread_handles_[slave_count_]);
-        CHECK(result);
-        thread_handles_[slave_count_] = 0;
-        delete slaves_[slave_count_];
-        slaves_[slave_count_] = NULL;
-      }
-    }
-  }
-  return true;
+  PreLockedScheduleLookups();
 }
 
 void DnsMaster::DiscardAllResults() {
@@ -514,10 +428,11 @@ void DnsMaster::DiscardAllResults() {
     info->SetAssignedState();
     info->SetNoSuchNameState();
   }
-  // Now every result_ is either resolved, or is being worked on by a slave.
+  // Now every result_ is either resolved, or is being resolved
+  // (see LookupRequest).
 
   // Step through result_, recording names of all hosts that can't be erased.
-  // We can't erase anything being worked on by a slave.
+  // We can't erase anything being worked on.
   Results assignees;
   for (Results::iterator it = results_.begin(); results_.end() != it; ++it) {
     std::string hostname = it->first;
@@ -528,9 +443,9 @@ void DnsMaster::DiscardAllResults() {
       assignees[hostname] = *info;
     }
   }
-  DCHECK(kSlaveCountMax >= assignees.size());
+  DCHECK(assignees.size() <= kMaxConcurrentLookups);
   results_.clear();
-  // Put back in the names being worked on by slaves.
+  // Put back in the names being worked on.
   for (Results::iterator it = assignees.begin(); assignees.end() != it; ++it) {
     DCHECK(it->second.is_marked_to_delete());
     results_[it->first] = it->second;
@@ -584,4 +499,3 @@ void DnsMaster::DeserializeReferrers(const ListValue& referral_list) {
 }
 
 }  // namespace chrome_browser_net
-