diff options
| -rw-r--r-- | chrome/browser/browser.scons | 4 | ||||
| -rw-r--r-- | chrome/browser/browser.vcproj | 8 | ||||
| -rw-r--r-- | chrome/browser/net/dns_global.cc | 14 | ||||
| -rw-r--r-- | chrome/browser/net/dns_host_info.cc | 4 | ||||
| -rw-r--r-- | chrome/browser/net/dns_host_info.h | 11 | ||||
| -rw-r--r-- | chrome/browser/net/dns_host_info_unittest.cc | 6 | ||||
| -rw-r--r-- | chrome/browser/net/dns_master.cc | 291 | ||||
| -rw-r--r-- | chrome/browser/net/dns_master.h | 134 | ||||
| -rw-r--r-- | chrome/browser/net/dns_master_unittest.cc | 408 | ||||
| -rw-r--r-- | chrome/browser/net/dns_slave.cc | 96 | ||||
| -rw-r--r-- | chrome/browser/net/dns_slave.h | 72 | ||||
| -rw-r--r-- | chrome/test/unit/unit_tests.scons | 2 | ||||
| -rw-r--r-- | net/base/host_resolver.h | 9 | ||||
| -rw-r--r-- | net/base/scoped_host_mapper.h | 25 |
14 files changed, 745 insertions, 339 deletions
diff --git a/chrome/browser/browser.scons b/chrome/browser/browser.scons index 0fb23ae..225d6fa 100644 --- a/chrome/browser/browser.scons +++ b/chrome/browser/browser.scons @@ -426,6 +426,8 @@ input_files = ChromeFileList([ 'net/dns_host_info.h', 'net/dns_master.cc', 'net/dns_master.h', + 'net/dns_slave.cc', + 'net/dns_slave.h', 'net/referrer.cc', 'net/referrer.h', 'net/sdch_dictionary_fetcher.cc', @@ -746,6 +748,8 @@ if not env.Bit('windows'): 'metrics/metrics_service.cc', 'modal_html_dialog_delegate.cc', 'net/dns_global.cc', + 'net/dns_master.cc', + 'net/dns_slave.cc', 'google_update.cc', 'password_manager/encryptor.cc', 'password_manager/ie7_password.cc', diff --git a/chrome/browser/browser.vcproj b/chrome/browser/browser.vcproj index f6bba92..c1cbed3 100644 --- a/chrome/browser/browser.vcproj +++ b/chrome/browser/browser.vcproj @@ -1614,6 +1614,14 @@ > </File> <File + RelativePath=".\net\dns_slave.cc" + > + </File> + <File + RelativePath=".\net\dns_slave.h" + > + </File> + <File RelativePath=".\net\referrer.cc" > </File> diff --git a/chrome/browser/net/dns_global.cc b/chrome/browser/net/dns_global.cc index 693a2cd..3ea79ff 100644 --- a/chrome/browser/net/dns_global.cc +++ b/chrome/browser/net/dns_global.cc @@ -377,9 +377,13 @@ void DnsPrefetchGetHtmlInfo(std::string* output) { static PrefetchObserver dns_resolution_observer; void InitDnsPrefetch(PrefService* user_prefs) { + // Use a large shutdown time so that UI tests (that instigate lookups, and + // then try to shutdown the browser) don't instigate the CHECK about + // "some slaves have not finished" + const TimeDelta kAllowableShutdownTime(TimeDelta::FromSeconds(10)); DCHECK(NULL == dns_master); if (!dns_master) { - dns_master = new DnsMaster(); + dns_master = new DnsMaster(kAllowableShutdownTime); // We did the initialization, so we should prime the pump, and set up // the DNS resolution system to run. off_the_record_observer.Register(); @@ -398,8 +402,14 @@ void InitDnsPrefetch(PrefService* user_prefs) { void ShutdownDnsPrefetch() { DCHECK(NULL != dns_master); - delete dns_master; + DnsMaster* master = dns_master; dns_master = NULL; + if (master->ShutdownSlaves()) { + delete master; + } else { + // Leak instance if shutdown problem. + DCHECK(0); + } } static void DiscardAllPrefetchState() { diff --git a/chrome/browser/net/dns_host_info.cc b/chrome/browser/net/dns_host_info.cc index a219420..9be4b2d 100644 --- a/chrome/browser/net/dns_host_info.cc +++ b/chrome/browser/net/dns_host_info.cc @@ -39,8 +39,8 @@ bool DnsHostInfo::NeedsDnsUpdate(const std::string& hostname) { return true; case QUEUED: // In queue. - case ASSIGNED: // It's being resolved. - case ASSIGNED_BUT_MARKED: // It's being resolved. + case ASSIGNED: // Slave is working on it. + case ASSIGNED_BUT_MARKED: // Slave is working on it. return false; // We're already working on it case NO_SUCH_NAME: // Lookup failed. diff --git a/chrome/browser/net/dns_host_info.h b/chrome/browser/net/dns_host_info.h index b9ea157..50c44f3 100644 --- a/chrome/browser/net/dns_host_info.h +++ b/chrome/browser/net/dns_host_info.h @@ -4,8 +4,9 @@ // A DnsHostInfo object is used to store status of a Dns lookup of a specific // hostname. -// It includes progress, from placement in the DnsMaster's queue, to resolution -// by the DNS service as either FOUND or NO_SUCH_NAME. +// It includes progress, from placement in the DnsMaster's queue, to assignment +// to a slave, to resolution by the (blocking) DNS service as either FOUND or +// NO_SUCH_NAME. #ifndef CHROME_BROWSER_NET_DNS_HOST_INFO_H_ #define CHROME_BROWSER_NET_DNS_HOST_INFO_H_ @@ -53,9 +54,9 @@ class DnsHostInfo { enum DnsProcessingState { // When processed by our prefetching system, the states are: PENDING, // Constructor has completed. - QUEUED, // In prefetch queue but not yet being resolved. - ASSIGNED, // Currently being processed. - ASSIGNED_BUT_MARKED, // Needs to be deleted as soon as it's resolved. + QUEUED, // In prefetch queue but not yet assigned to a slave. + ASSIGNED, // Currently being processed by a slave. + ASSIGNED_BUT_MARKED, // Needs to be deleted as soon as slave is done. FOUND, // DNS prefetch search completed. NO_SUCH_NAME, // DNS prefetch search completed. // When processed by the network stack during navigation, the states are: diff --git a/chrome/browser/net/dns_host_info_unittest.cc b/chrome/browser/net/dns_host_info_unittest.cc index 9d2fb21..79cca93 100644 --- a/chrome/browser/net/dns_host_info_unittest.cc +++ b/chrome/browser/net/dns_host_info_unittest.cc @@ -41,7 +41,8 @@ TEST(DnsHostInfoTest, StateChangeTest) { EXPECT_FALSE(info.NeedsDnsUpdate(hostname1)) << "update needed after being queued"; info.SetAssignedState(); - EXPECT_FALSE(info.NeedsDnsUpdate(hostname1)); + EXPECT_FALSE(info.NeedsDnsUpdate(hostname1)) + << "update needed while assigned to slave"; info.SetFoundState(); EXPECT_FALSE(info.NeedsDnsUpdate(hostname1)) << "default expiration time is TOOOOO short"; @@ -68,7 +69,8 @@ TEST(DnsHostInfoTest, StateChangeTest) { // be found. We'll sleep for a while, and then come back with not-found. info.SetQueuedState(DnsHostInfo::UNIT_TEST_MOTIVATED); info.SetAssignedState(); - EXPECT_FALSE(info.NeedsDnsUpdate(hostname1)); + EXPECT_FALSE(info.NeedsDnsUpdate(hostname1)) + << "update needed while assigned to slave"; // Greater than minimal expected network latency on DNS lookup. PlatformThread::Sleep(25); info.SetNoSuchNameState(); diff --git a/chrome/browser/net/dns_master.cc b/chrome/browser/net/dns_master.cc index 7a88b138..4d477b0 100644 --- a/chrome/browser/net/dns_master.cc +++ b/chrome/browser/net/dns_master.cc @@ -2,81 +2,61 @@ // 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 <sstream> -#include "base/compiler_specific.h" #include "base/histogram.h" -#include "base/lock.h" -#include "base/revocable_store.h" #include "base/stats_counters.h" #include "base/string_util.h" -#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 { - -// static -const size_t DnsMaster::kMaxConcurrentLookups = 8; - -class DnsMaster::LookupRequest : RevocableStore::Revocable { - public: - LookupRequest(DnsMaster* master, const std::string& hostname) - : RevocableStore::Revocable(&master->pending_callbacks_), - ALLOW_THIS_IN_INITIALIZER_LIST(callback_( - this, &LookupRequest::OnLookupFinished)), - hostname_(hostname), - master_(master) { - } +#include "base/thread.h" +#include "base/win_util.h" +#include "chrome/browser/net/dns_slave.h" - bool Start() { - const int result = resolver_.Resolve(hostname_, 80, &addresses_, - &callback_); - return (result == net::ERR_IO_PENDING); - } +using base::TimeDelta; - private: - void OnLookupFinished(int result) { - if (revoked()) { - delete this; - return; - } - - if (result == net::OK) - master_->SetFoundState(hostname_); - else - master_->SetNoSuchNameState(hostname_); +namespace chrome_browser_net { - delete this; +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; } - - net::CompletionCallbackImpl<LookupRequest> callback_; - const std::string hostname_; - net::HostResolver resolver_; - net::AddressList addresses_; - DnsMaster* master_; -}; - -DnsMaster::DnsMaster() : pending_lookups_(0), peak_pending_lookups_(0) { -} - -DnsMaster::~DnsMaster() { - pending_callbacks_.RevokeAll(); } // Overloaded Resolve() to take a vector of names. void DnsMaster::ResolveList(const NameList& hostnames, DnsHostInfo::ResolutionMotivation motivation) { - AutoLock auto_lock(lock_); + 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. + } - NameList::const_iterator it; - for (it = hostnames.begin(); it < hostnames.end(); ++it) - PreLockedResolve(*it, motivation); + 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(); } // Basic Resolve() takes an invidual name, and adds it @@ -85,8 +65,16 @@ void DnsMaster::Resolve(const std::string& hostname, DnsHostInfo::ResolutionMotivation motivation) { if (0 == hostname.length()) return; - AutoLock auto_lock(lock_); - PreLockedResolve(hostname, motivation); + 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(); } bool DnsMaster::AccruePrefetchBenefits(const GURL& referrer, @@ -159,19 +147,26 @@ void DnsMaster::NonlinkNavigation(const GURL& referrer, } void DnsMaster::NavigatingTo(const std::string& 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); + 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); + } + } } + if (need_to_signal) + slaves_have_work_.Signal(); } // Provide sort order so all .com's are together, etc. @@ -302,7 +297,7 @@ void DnsMaster::GetHtmlInfo(std::string* output) { } if (!it->second.was_found()) continue; // Still being processed. - if (base::TimeDelta() != it->second.benefits_remaining()) { + if (TimeDelta() != it->second.benefits_remaining()) { network_hits.push_back(it->second); // With no benefit yet. continue; } @@ -337,6 +332,7 @@ DnsHostInfo* DnsMaster::PreLockedResolve( const std::string& hostname, DnsHostInfo::ResolutionMotivation motivation) { // DCHECK(We have the lock); + DCHECK(0 != slave_count_); DCHECK(0 != hostname.length()); DnsHostInfo* info = &results_[hostname]; @@ -353,30 +349,37 @@ DnsHostInfo* DnsMaster::PreLockedResolve( info->SetQueuedState(motivation); name_buffer_.push(hostname); - - PreLockedScheduleLookups(); - return info; } -void DnsMaster::PreLockedScheduleLookups() { - while (!name_buffer_.empty() && pending_lookups_ < kMaxConcurrentLookups) { - const std::string hostname(name_buffer_.front()); +// 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(); name_buffer_.pop(); - DnsHostInfo* info = &results_[hostname]; - DCHECK(info->HasHostname(hostname)); + DnsHostInfo* info = &results_[*hostname]; + DCHECK(info->HasHostname(*hostname)); info->SetAssignedState(); - LookupRequest* request = new LookupRequest(this, hostname); - if (request->Start()) { - pending_lookups_++; - peak_pending_lookups_ = std::max(peak_pending_lookups_, pending_lookups_); - } else { - NOTREACHED(); - delete request; - } - } + 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) { @@ -387,9 +390,6 @@ void DnsMaster::SetFoundState(const std::string hostname) { results_.erase(hostname); else info->SetFoundState(); - - pending_lookups_--; - PreLockedScheduleLookups(); } void DnsMaster::SetNoSuchNameState(const std::string hostname) { @@ -400,9 +400,100 @@ void DnsMaster::SetNoSuchNameState(const std::string hostname) { results_.erase(hostname); else info->SetNoSuchNameState(); +} - pending_lookups_--; - PreLockedScheduleLookups(); +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; + } + 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_++; + + 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; } void DnsMaster::DiscardAllResults() { @@ -423,11 +514,10 @@ void DnsMaster::DiscardAllResults() { info->SetAssignedState(); info->SetNoSuchNameState(); } - // Now every result_ is either resolved, or is being resolved - // (see LookupRequest). + // Now every result_ is either resolved, or is being worked on by a slave. // Step through result_, recording names of all hosts that can't be erased. - // We can't erase anything being worked on. + // We can't erase anything being worked on by a slave. Results assignees; for (Results::iterator it = results_.begin(); results_.end() != it; ++it) { std::string hostname = it->first; @@ -438,9 +528,9 @@ void DnsMaster::DiscardAllResults() { assignees[hostname] = *info; } } - DCHECK(assignees.size() <= kMaxConcurrentLookups); + DCHECK(kSlaveCountMax >= assignees.size()); results_.clear(); - // Put back in the names being worked on. + // Put back in the names being worked on by slaves. for (Results::iterator it = assignees.begin(); assignees.end() != it; ++it) { DCHECK(it->second.is_marked_to_delete()); results_[it->first] = it->second; @@ -448,3 +538,4 @@ void DnsMaster::DiscardAllResults() { } } // namespace chrome_browser_net + diff --git a/chrome/browser/net/dns_master.h b/chrome/browser/net/dns_master.h index 78f5540..7620d7c 100644 --- a/chrome/browser/net/dns_master.h +++ b/chrome/browser/net/dns_master.h @@ -3,13 +3,16 @@ // 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_ @@ -18,27 +21,40 @@ #include <queue> #include <string> -#include "base/lock.h" -#include "base/revocable_store.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,62 +88,72 @@ 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 lookup callbacks to pass resolution result. + // Access methods for use by slave threads to callback with state updates. void SetFoundState(const std::string hostname); void SetNoSuchNameState(const std::string hostname); + // 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. // Queue hostname for resolution. If queueing was done, return the pointer // 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,13 +167,25 @@ class DnsMaster { // pre-resolve when there is a navigation to the orginial hostname. Referrers referrers_; - // Count of started, but not yet finished dns lookups - size_t 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. + + // 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]; - // For testing, to verify that we don't exceed the limit; - size_t peak_pending_lookups_; + // shutdown_ is set to tell the slaves to terminate. + bool shutdown_; - RevocableStore pending_callbacks_; // We'll cancel them on 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_; diff --git a/chrome/browser/net/dns_master_unittest.cc b/chrome/browser/net/dns_master_unittest.cc index 70c263c2..650a8ff 100644 --- a/chrome/browser/net/dns_master_unittest.cc +++ b/chrome/browser/net/dns_master_unittest.cc @@ -2,89 +2,104 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +// Multi-threaded tests of DnsMaster and DnsPrefetch slave functionality. + #include <time.h> +#include <ws2tcpip.h> +#include <Wspiapi.h> // Needed for win2k compatibility #include <algorithm> #include <map> #include <sstream> #include <string> -#include <vector> -#include "base/message_loop.h" #include "base/platform_thread.h" #include "base/spin_wait.h" -#include "base/timer.h" #include "chrome/browser/net/dns_global.h" #include "chrome/browser/net/dns_host_info.h" -#include "chrome/common/net/dns.h" -#include "net/base/address_list.h" -#include "net/base/host_resolver.h" -#include "net/base/scoped_host_mapper.h" +#include "chrome/browser/net/dns_slave.h" #include "net/base/winsock_init.h" #include "testing/gtest/include/gtest/gtest.h" + using base::Time; using base::TimeDelta; -namespace chrome_browser_net { - -class WaitForResolutionHelper; +namespace { -typedef base::RepeatingTimer<WaitForResolutionHelper> HelperTimer; - -class WaitForResolutionHelper { - public: - WaitForResolutionHelper(DnsMaster* master, const NameList& hosts, - HelperTimer* timer) - : master_(master), - hosts_(hosts), - timer_(timer) { - } +class DnsMasterTest : public testing::Test { +}; - void Run() { - for (NameList::const_iterator i = hosts_.begin(); i != hosts_.end(); ++i) - if (master_->GetResolutionDuration(*i) == DnsHostInfo::kNullDuration) - return; // We don't have resolution for that host. +typedef chrome_browser_net::DnsMaster DnsMaster; +typedef chrome_browser_net::DnsPrefetcherInit DnsPrefetcherInit; +typedef chrome_browser_net::DnsHostInfo DnsHostInfo; +typedef chrome_browser_net::NameList NameList; - // When all hostnames have been resolved, exit the loop. - timer_->Stop(); - MessageLoop::current()->Quit(); - delete timer_; - delete this; - } - private: - DnsMaster* master_; - const NameList hosts_; - HelperTimer* timer_; -}; +//------------------------------------------------------------------------------ +// Provide network function stubs to run tests offline (and avoid the variance +// of real DNS lookups. +//------------------------------------------------------------------------------ -class DnsMasterTest : public testing::Test { - protected: - virtual void SetUp() { -#if defined(OS_WIN) - net::EnsureWinsockInit(); -#endif - mapper_.AddRuleWithLatency("www.google.com", "127.0.0.1", 50); - mapper_.AddRuleWithLatency("gmail.google.com.com", "127.0.0.1", 70); - mapper_.AddRuleWithLatency("mail.google.com", "127.0.0.1", 44); - mapper_.AddRuleWithLatency("gmail.com", "127.0.0.1", 63); - - mapper_.AddRuleWithLatency("veryslow.net", "127.0.0.1", 10000); - } +static void __stdcall fake_free_addr_info(struct addrinfo* ai) { + // Kill off the dummy results. + EXPECT_TRUE(NULL != ai); + delete ai; +} - void WaitForResolution(DnsMaster* master, const NameList& hosts) { - HelperTimer* timer = new HelperTimer(); - timer->Start(TimeDelta::FromMilliseconds(100), - new WaitForResolutionHelper(master, hosts, timer), - &WaitForResolutionHelper::Run); - MessageLoop::current()->Run(); - } +static int __stdcall fake_get_addr_info(const char* nodename, + const char* servname, + const struct addrinfo* hints, + struct addrinfo** result) { + static Lock lock; + int duration; + bool was_found; + std::string hostname(nodename); + // Dummy up *some* return results to pass along. + *result = new addrinfo; + EXPECT_TRUE(NULL != *result); + { + AutoLock autolock(lock); + + static bool initialized = false; + typedef std::map<std::string, int> Latency; + static Latency latency; + static std::map<std::string, bool> found; + if (!initialized) { + initialized = true; + // List all known hostnames + latency["www.google.com"] = 50; + latency["gmail.google.com.com"] = 70; + latency["mail.google.com"] = 44; + latency["gmail.com"] = 63; + + for (Latency::iterator it = latency.begin(); latency.end() != it; it++) { + found[it->first] = true; + } + } // End static initialization + + was_found = found[hostname]; + + if (latency.end() != latency.find(hostname)) { + duration = latency[hostname]; + } else { + duration = 500; + } + // Change latency to simulate cache warming (next latency will be short). + latency[hostname] = 1; + } // Release lock. + + PlatformThread::Sleep(duration); + + return was_found ? 0 : WSAHOST_NOT_FOUND; +} - private: - MessageLoop loop_; - net::ScopedHostMapper mapper_; -}; +static void SetupNetworkInfrastructure() { + bool kUseFakeNetwork = true; + if (kUseFakeNetwork) + chrome_browser_net::SetAddrinfoCallbacks(fake_get_addr_info, + fake_free_addr_info); +} //------------------------------------------------------------------------------ // Provide a function to create unique (nonexistant) domains at *every* call. @@ -105,63 +120,53 @@ static std::string GetNonexistantDomain() { // Use a blocking function to contrast results we get via async services. //------------------------------------------------------------------------------ TimeDelta BlockingDnsLookup(const std::string& hostname) { + char* port = "80"; // I may need to get the real port + struct addrinfo* result = NULL; Time start = Time::Now(); - net::HostResolver resolver; - net::AddressList addresses; - resolver.Resolve(hostname, 80, &addresses, NULL); + // Use the same underlying methods as dns_prefetch_slave does + chrome_browser_net::get_getaddrinfo()(hostname.c_str(), port, + NULL, &result); + + TimeDelta duration = Time::Now() - start; + + if (result) { + chrome_browser_net::get_freeaddrinfo()(result); + result = NULL; + } - return Time::Now() - start; + return duration; } //------------------------------------------------------------------------------ // First test to be sure the OS is caching lookups, which is the whole premise // of DNS prefetching. -TEST_F(DnsMasterTest, OsCachesLookupsTest) { - const Time start = Time::Now(); - int all_lookups = 0; - int lookups_with_improvement = 0; - // This test can be really flaky on Linux. It should run in much shorter time, - // but sometimes it won't and we don't like bogus failures. - while (Time::Now() - start < TimeDelta::FromMinutes(1)) { +TEST(DnsMasterTest, OsCachesLookupsTest) { + SetupNetworkInfrastructure(); + net::EnsureWinsockInit(); + + for (int i = 0; i < 5; i++) { std::string badname; badname = GetNonexistantDomain(); - TimeDelta duration = BlockingDnsLookup(badname); - - // Produce more than one result and remove the largest one - // to reduce flakiness. - std::vector<TimeDelta> cached_results; - for (int j = 0; j < 3; j++) - cached_results.push_back(BlockingDnsLookup(badname)); - std::sort(cached_results.begin(), cached_results.end()); - cached_results.pop_back(); - - TimeDelta cached_sum = TimeDelta::FromSeconds(0); - for (std::vector<TimeDelta>::const_iterator j = cached_results.begin(); - j != cached_results.end(); ++j) - cached_sum += *j; - TimeDelta cached_duration = cached_sum / cached_results.size(); - - all_lookups++; - if (cached_duration < duration) - lookups_with_improvement++; - if (all_lookups >= 10) - if (lookups_with_improvement * 100 > all_lookups * 75) - // Okay, we see the improvement for more than 75% of all lookups. - return; + TimeDelta cached_duration = BlockingDnsLookup(badname); + EXPECT_TRUE(duration > cached_duration); } - FAIL() << "No substantial improvement in lookup time."; } -TEST_F(DnsMasterTest, StartupShutdownTest) { - DnsMaster testing_master; - // We do shutdown on destruction. +TEST(DnsMasterTest, StartupShutdownTest) { + DnsMaster testing_master(TimeDelta::FromMilliseconds(5000)); + + // With no threads, we should have no problem doing a shutdown. + EXPECT_TRUE(testing_master.ShutdownSlaves()); } -TEST_F(DnsMasterTest, BenefitLookupTest) { - DnsMaster testing_master; +TEST(DnsMasterTest, BenefitLookupTest) { + SetupNetworkInfrastructure(); + net::EnsureWinsockInit(); + DnsPrefetcherInit dns_init(NULL); // Creates global service . + DnsMaster testing_master(TimeDelta::FromMilliseconds(5000)); std::string goog("www.google.com"), goog2("gmail.google.com.com"), @@ -191,9 +196,23 @@ TEST_F(DnsMasterTest, BenefitLookupTest) { names.insert(names.end(), goog3); names.insert(names.end(), goog4); + // First only cause a minimal set of threads to start up. + // Currently we actually start 4 threads when we get called with an array testing_master.ResolveList(names, DnsHostInfo::PAGE_SCAN_MOTIVATED); - WaitForResolution(&testing_master, names); + // Wait for some resoultion for each google. + SPIN_FOR_1_SECOND_OR_UNTIL_TRUE(0 <= + testing_master.GetResolutionDuration(goog).InMilliseconds()); + SPIN_FOR_1_SECOND_OR_UNTIL_TRUE(0 <= + testing_master.GetResolutionDuration(goog2).InMilliseconds()); + SPIN_FOR_1_SECOND_OR_UNTIL_TRUE(0 <= + testing_master.GetResolutionDuration(goog3).InMilliseconds()); + SPIN_FOR_1_SECOND_OR_UNTIL_TRUE(0 <= + testing_master.GetResolutionDuration(goog4).InMilliseconds()); + + EXPECT_EQ(std::min(names.size(), + 4u /* chrome_browser_net::DnsMaster::kSlaveCountMin */ ), + testing_master.running_slave_count()); EXPECT_TRUE(testing_master.WasFound(goog)); EXPECT_TRUE(testing_master.WasFound(goog2)); @@ -215,51 +234,68 @@ TEST_F(DnsMasterTest, BenefitLookupTest) { EXPECT_FALSE(testing_master.AccruePrefetchBenefits(GURL(), &goog2_info)); EXPECT_FALSE(testing_master.AccruePrefetchBenefits(GURL(), &goog3_info)); EXPECT_FALSE(testing_master.AccruePrefetchBenefits(GURL(), &goog4_info)); -} - -TEST_F(DnsMasterTest, ShutdownWhenResolutionIsPendingTest) { - DnsMaster testing_master; - std::string slow("veryslow.net"); - NameList names; - names.insert(names.end(), slow); - - testing_master.ResolveList(names, DnsHostInfo::PAGE_SCAN_MOTIVATED); - - MessageLoop::current()->PostDelayedTask(FROM_HERE, - new MessageLoop::QuitTask(), 500); - MessageLoop::current()->Run(); - MessageLoop::current()->RunAllPending(); - - EXPECT_FALSE(testing_master.WasFound(slow)); + // Ensure a clean shutdown. + EXPECT_TRUE(testing_master.ShutdownSlaves()); } -TEST_F(DnsMasterTest, SingleLookupTest) { - DnsMaster testing_master; +TEST(DnsMasterTest, DISABLED_SingleSlaveLookupTest) { + SetupNetworkInfrastructure(); + net::EnsureWinsockInit(); + DnsPrefetcherInit dns_init(NULL); // Creates global service. + DnsMaster testing_master(TimeDelta::FromMilliseconds(5000)); - std::string goog("www.google.com"); + std::string goog("www.google.com"), + goog2("gmail.google.com.com"), + goog3("mail.google.com"), + goog4("gmail.com"); + std::string bad1(GetNonexistantDomain()), + bad2(GetNonexistantDomain()); + + // Warm up local OS cache. + BlockingDnsLookup(goog); NameList names; names.insert(names.end(), goog); + names.insert(names.end(), bad1); + names.insert(names.end(), bad2); - // Try to flood the master with many concurrent requests. - for (int i = 0; i < 10; i++) - testing_master.ResolveList(names, DnsHostInfo::PAGE_SCAN_MOTIVATED); + // First only cause a single thread to start up + testing_master.ResolveList(names, DnsHostInfo::PAGE_SCAN_MOTIVATED); - WaitForResolution(&testing_master, names); + // Wait for some resoultion for google. + SPIN_FOR_1_SECOND_OR_UNTIL_TRUE(0 <= + testing_master.GetResolutionDuration(goog).InMilliseconds()); EXPECT_TRUE(testing_master.WasFound(goog)); + EXPECT_FALSE(testing_master.WasFound(bad1)); + EXPECT_FALSE(testing_master.WasFound(bad2)); + // Verify the reason it is not found is that it is still being proceessed. + // Negative time mean no resolution yet. + EXPECT_GT(0, testing_master.GetResolutionDuration(bad2).InMilliseconds()); + + // Spin long enough that we *do* find the resolution of bad2. + SPIN_FOR_1_SECOND_OR_UNTIL_TRUE(0 <= + testing_master.GetResolutionDuration(bad2).InMilliseconds()); + + // Verify both fictitious names are resolved by now. + // Typical random name takes about 20-30 ms + EXPECT_LT(0, testing_master.GetResolutionDuration(bad1).InMilliseconds()); + EXPECT_LT(0, testing_master.GetResolutionDuration(bad2).InMilliseconds()); + EXPECT_FALSE(testing_master.WasFound(bad1)); + EXPECT_FALSE(testing_master.WasFound(bad2)); - MessageLoop::current()->RunAllPending(); + EXPECT_EQ(1U, testing_master.running_slave_count()); - EXPECT_GT(testing_master.peak_pending_lookups(), names.size() / 2); - EXPECT_LE(testing_master.peak_pending_lookups(), names.size()); - EXPECT_LE(testing_master.peak_pending_lookups(), - DnsMaster::kMaxConcurrentLookups); + // With just one thread (doing nothing now), ensure a clean shutdown. + EXPECT_TRUE(testing_master.ShutdownSlaves()); } -TEST_F(DnsMasterTest, ConcurrentLookupTest) { - DnsMaster testing_master; +TEST(DnsMasterTest, DISABLED_MultiThreadedLookupTest) { + SetupNetworkInfrastructure(); + net::EnsureWinsockInit(); + DnsMaster testing_master(TimeDelta::FromSeconds(30)); + DnsPrefetcherInit dns_init(NULL); std::string goog("www.google.com"), goog2("gmail.google.com.com"), @@ -283,11 +319,12 @@ TEST_F(DnsMasterTest, ConcurrentLookupTest) { BlockingDnsLookup(goog3); BlockingDnsLookup(goog4); - // Try to flood the master with many concurrent requests. - for (int i = 0; i < 10; i++) + // Get all 8 threads running by calling many times before queue is handled. + for (int i = 0; i < 10; i++) { testing_master.ResolveList(names, DnsHostInfo::PAGE_SCAN_MOTIVATED); + } - WaitForResolution(&testing_master, names); + Sleep(10); // Allow time for async DNS to get answers. EXPECT_TRUE(testing_master.WasFound(goog)); EXPECT_TRUE(testing_master.WasFound(goog3)); @@ -296,35 +333,100 @@ TEST_F(DnsMasterTest, ConcurrentLookupTest) { EXPECT_FALSE(testing_master.WasFound(bad1)); EXPECT_FALSE(testing_master.WasFound(bad2)); - MessageLoop::current()->RunAllPending(); - - EXPECT_FALSE(testing_master.WasFound(bad1)); - EXPECT_FALSE(testing_master.WasFound(bad2)); + EXPECT_EQ(8U, testing_master.running_slave_count()); - EXPECT_GT(testing_master.peak_pending_lookups(), names.size() / 2); - EXPECT_LE(testing_master.peak_pending_lookups(), names.size()); - EXPECT_LE(testing_master.peak_pending_lookups(), - DnsMaster::kMaxConcurrentLookups); + EXPECT_TRUE(testing_master.ShutdownSlaves()); } -TEST_F(DnsMasterTest, MassiveConcurrentLookupTest) { - DnsMaster testing_master; +TEST(DnsMasterTest, DISABLED_MultiThreadedSpeedupTest) { + SetupNetworkInfrastructure(); + net::EnsureWinsockInit(); + DnsMaster testing_master(TimeDelta::FromSeconds(30)); + DnsPrefetcherInit dns_init(NULL); + + std::string goog("www.google.com"), + goog2("gmail.google.com.com"), + goog3("mail.google.com"), + goog4("gmail.com"); + std::string bad1(GetNonexistantDomain()), + bad2(GetNonexistantDomain()), + bad3(GetNonexistantDomain()), + bad4(GetNonexistantDomain()); NameList names; - for (int i = 0; i < 100; i++) - names.push_back(GetNonexistantDomain()); + names.insert(names.end(), goog); + names.insert(names.end(), bad1); + names.insert(names.end(), bad2); + names.insert(names.end(), goog3); + names.insert(names.end(), goog2); + names.insert(names.end(), bad3); + names.insert(names.end(), bad4); + names.insert(names.end(), goog4); + + // First cause a lookup using a single thread. + testing_master.ResolveList(names, DnsHostInfo::PAGE_SCAN_MOTIVATED); - // Try to flood the master with many concurrent requests. + // Wait for some resoultion for google. + SPIN_FOR_1_SECOND_OR_UNTIL_TRUE(0 <= + testing_master.GetResolutionDuration(goog).InMilliseconds()); + + EXPECT_TRUE(testing_master.WasFound(goog)); + EXPECT_FALSE(testing_master.WasFound(bad1)); + EXPECT_FALSE(testing_master.WasFound(bad2)); + // ...and due to delay in geting resolution of bad names, the single slave + // thread won't have time to finish the list. + EXPECT_FALSE(testing_master.WasFound(goog3)); + EXPECT_FALSE(testing_master.WasFound(goog2)); + EXPECT_FALSE(testing_master.WasFound(goog4)); + + EXPECT_EQ(1U, testing_master.running_slave_count()); + + // Get all 8 threads running by calling many times before queue is handled. + names.clear(); for (int i = 0; i < 10; i++) - testing_master.ResolveList(names, DnsHostInfo::PAGE_SCAN_MOTIVATED); + testing_master.Resolve(GetNonexistantDomain(), + DnsHostInfo::PAGE_SCAN_MOTIVATED); + + // Wait long enough for all the goog's to be resolved. + // They should all take about the same time, and run in parallel. + SPIN_FOR_1_SECOND_OR_UNTIL_TRUE(0 <= + testing_master.GetResolutionDuration(goog2).InMilliseconds()); + SPIN_FOR_1_SECOND_OR_UNTIL_TRUE(0 <= + testing_master.GetResolutionDuration(goog3).InMilliseconds()); + SPIN_FOR_1_SECOND_OR_UNTIL_TRUE(0 <= + testing_master.GetResolutionDuration(goog4).InMilliseconds()); - WaitForResolution(&testing_master, names); + EXPECT_TRUE(testing_master.WasFound(goog3)); + EXPECT_TRUE(testing_master.WasFound(goog2)); + EXPECT_TRUE(testing_master.WasFound(goog4)); + + EXPECT_FALSE(testing_master.WasFound(bad1)); + EXPECT_FALSE(testing_master.WasFound(bad2)); // Perhaps not even decided. + + // Queue durations should be distinct from when 1 slave was working. + EXPECT_GT(testing_master.GetQueueDuration(goog3).InMilliseconds(), + testing_master.GetQueueDuration(goog).InMilliseconds()); + EXPECT_GT(testing_master.GetQueueDuration(goog4).InMilliseconds(), + testing_master.GetQueueDuration(goog).InMilliseconds()); - MessageLoop::current()->RunAllPending(); + // Give bad names a chance to be determined as unresolved. + SPIN_FOR_1_SECOND_OR_UNTIL_TRUE(0 <= + testing_master.GetResolutionDuration(bad1).InMilliseconds()); + SPIN_FOR_1_SECOND_OR_UNTIL_TRUE(0 <= + testing_master.GetResolutionDuration(bad2).InMilliseconds()); - EXPECT_LE(testing_master.peak_pending_lookups(), names.size()); - EXPECT_LE(testing_master.peak_pending_lookups(), - DnsMaster::kMaxConcurrentLookups); + + // Well known names should resolve faster than bad names. + EXPECT_GE(testing_master.GetResolutionDuration(bad1).InMilliseconds(), + testing_master.GetResolutionDuration(goog).InMilliseconds()); + + EXPECT_GE(testing_master.GetResolutionDuration(bad2).InMilliseconds(), + testing_master.GetResolutionDuration(goog4).InMilliseconds()); + + EXPECT_EQ(8U, testing_master.running_slave_count()); + + EXPECT_TRUE(testing_master.ShutdownSlaves()); } -} // namespace chrome_browser_net +} // namespace + diff --git a/chrome/browser/net/dns_slave.cc b/chrome/browser/net/dns_slave.cc new file mode 100644 index 0000000..2ffb78d --- /dev/null +++ b/chrome/browser/net/dns_slave.cc @@ -0,0 +1,96 @@ +// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// 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 <ws2tcpip.h> +#include <Wspiapi.h> // Needed for win2k compatibility + +#include "chrome/browser/net/dns_slave.h" + +#include "base/logging.h" +#include "base/platform_thread.h" +#include "base/string_util.h" +#include "chrome/browser/net/dns_host_info.h" +#include "chrome/browser/net/dns_master.h" + + +namespace chrome_browser_net { + +//------------------------------------------------------------------------------ +// We supply a functions for stubbing network callbacks, so that offline testing +// can be performed. +//------------------------------------------------------------------------------ +static GetAddrInfoFunction get_addr = getaddrinfo; +static FreeAddrInfoFunction free_addr = freeaddrinfo; +void SetAddrinfoCallbacks(GetAddrInfoFunction getaddrinfo, + FreeAddrInfoFunction freeaddrinfo) { + get_addr = getaddrinfo; + free_addr = freeaddrinfo; +} + +GetAddrInfoFunction get_getaddrinfo() {return get_addr;} +FreeAddrInfoFunction get_freeaddrinfo() {return free_addr;} + + +//------------------------------------------------------------------------------ +// This is the entry method used by DnsMaster to start our thread. +//------------------------------------------------------------------------------ + +DWORD __stdcall DnsSlave::ThreadStart(void* pThis) { + DnsSlave* slave_instance = reinterpret_cast<DnsSlave*>(pThis); + return slave_instance->Run(); +} + +//------------------------------------------------------------------------------ +// The following are methods on the DnsPrefetch class. +//------------------------------------------------------------------------------ + +unsigned DnsSlave::Run() { + DCHECK(slave_index_ >= 0 && slave_index_ < DnsMaster::kSlaveCountMax); + + std::string name = StringPrintf( + "dns_prefetcher_%d_of_%d", slave_index_ + 1, DnsMaster::kSlaveCountMax); + DLOG(INFO) << "Now Running " << name; + PlatformThread::SetName(name.c_str()); + + while (master_->GetNextAssignment(&hostname_)) { + BlockingDnsLookup(); + } + // GetNextAssignment() fails when we are told to terminate. + master_->SetSlaveHasTerminated(slave_index_); + return 0; +} + +void DnsSlave::BlockingDnsLookup() { + const char* port = "80"; // I may need to get the real port + addrinfo* result = NULL; + + int error_code = get_addr(hostname_.c_str(), port, NULL, &result); + + // Note that since info has value semantics, I need to ask + // master_ to set the new states atomically in its map. + switch (error_code) { + case 0: + master_->SetFoundState(hostname_); + break; + + default: + DCHECK_EQ(0, error_code) << "surprising output" ; + // fall through + + case WSAHOST_NOT_FOUND: + master_->SetNoSuchNameState(hostname_); + break; + } + + // We don't save results, so lets free them... + if (result) { + free_addr(result); + result = NULL; + } +} + +} // namespace chrome_browser_net + diff --git a/chrome/browser/net/dns_slave.h b/chrome/browser/net/dns_slave.h new file mode 100644 index 0000000..6bbc17f --- /dev/null +++ b/chrome/browser/net/dns_slave.h @@ -0,0 +1,72 @@ +// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// A DnsSlave object processes hostname lookups +// via DNS on a single thread, waiting for that blocking +// call to complete, and then getting its next hostname +// from its associated DnsMaster object. +// Since this class only is concerned with prefetching +// to warm the underlying DNS cache, the actual IP +// does not need to be recorded. It is necessary to record +// when the lookup finished, so that the associated DnsMaster +// won't (wastefully) ask for the same name in "too short a +// period of time." +// This class does no "de-duping," and merely slavishly services +// items supplied by its DnsMaster. + +#ifndef CHROME_BROWSER_NET_DNS_SLAVE_H_ +#define CHROME_BROWSER_NET_DNS_SLAVE_H_ + +#include <windows.h> +#include <string> + +#include "base/basictypes.h" + +namespace chrome_browser_net { + +class DnsMaster; +class DnsSlave; + +// Support testing infrastructure, and allow off-line testing. +typedef void (__stdcall *FreeAddrInfoFunction)(struct addrinfo* ai); +typedef int (__stdcall *GetAddrInfoFunction)(const char* nodename, + const char* servname, + const struct addrinfo* hints, + struct addrinfo** result); +void SetAddrinfoCallbacks(GetAddrInfoFunction getaddrinfo, + FreeAddrInfoFunction freeaddrinfo); + +GetAddrInfoFunction get_getaddrinfo(); +FreeAddrInfoFunction get_freeaddrinfo(); + +class DnsSlave { + public: + DnsSlave(DnsMaster* master, size_t slave_index) + : master_(master), + slave_index_(slave_index) { + } + + ~DnsSlave() { + master_ = NULL; + } + + static DWORD WINAPI ThreadStart(void* pThis); + + unsigned Run(); + + private: + std::string hostname_; // Name being looked up. + + DnsMaster* master_; // Master that started us. + size_t slave_index_; // Our index into DnsMaster's array. + + void BlockingDnsLookup(); + + DISALLOW_COPY_AND_ASSIGN(DnsSlave); +}; + +} // namespace chrome_browser_net + +#endif // CHROME_BROWSER_NET_DNS_SLAVE_H_ + diff --git a/chrome/test/unit/unit_tests.scons b/chrome/test/unit/unit_tests.scons index d3c418a..21763ce 100644 --- a/chrome/test/unit/unit_tests.scons +++ b/chrome/test/unit/unit_tests.scons @@ -322,7 +322,6 @@ if env.Bit('mac'): '$CHROME_DIR/browser/history/visit_tracker_unittest.cc', '$CHROME_DIR/browser/metrics/metrics_response_unittest.cc', '$CHROME_DIR/browser/net/dns_host_info_unittest.cc', - '$CHROME_DIR/browser/net/dns_master_unittest.cc', '$CHROME_DIR/browser/net/url_fetcher_unittest.cc', '$CHROME_DIR/browser/printing/page_range_unittest.cc', '$CHROME_DIR/browser/printing/page_setup_unittest.cc', @@ -411,6 +410,7 @@ if not env.Bit('windows'): '$CHROME_DIR/browser/renderer_host/mock_render_process_host.cc', '$CHROME_DIR/browser/renderer_host/mock_render_process_host.h', '$CHROME_DIR/browser/navigation_controller_unittest.cc', + '$CHROME_DIR/browser/net/dns_master_unittest.cc', '$CHROME_DIR/browser/password_manager/encryptor_unittest.cc', '$CHROME_DIR/browser/password_manager/password_form_manager_unittest.cc', '$CHROME_DIR/browser/printing/page_number_unittest.cc', diff --git a/net/base/host_resolver.h b/net/base/host_resolver.h index fa07708..1f649f8 100644 --- a/net/base/host_resolver.h +++ b/net/base/host_resolver.h @@ -41,9 +41,10 @@ class HostResolver { // // When callback is null, the operation completes synchronously. // - // When callback is non-null, the operation will be performed asynchronously. - // ERR_IO_PENDING is returned if it has been scheduled successfully. Real - // result code will be passed to the completion callback. + // When callback is non-null, ERR_IO_PENDING is returned if the operation + // could not be completed synchronously, in which case the result code will + // be passed to the callback when available. + // int Resolve(const std::string& hostname, int port, AddressList* addresses, CompletionCallback* callback); @@ -73,7 +74,7 @@ class HostMapper { // function while there are no outstanding HostResolver instances. // // NOTE: In most cases, you should use ScopedHostMapper instead, which is -// defined in scoped_host_resolver.h. +// defined in host_resolver_unittest.h // HostMapper* SetHostMapper(HostMapper* host_mapper); #endif diff --git a/net/base/scoped_host_mapper.h b/net/base/scoped_host_mapper.h index 8b623b9..836cb34 100644 --- a/net/base/scoped_host_mapper.h +++ b/net/base/scoped_host_mapper.h @@ -39,7 +39,6 @@ #include <list> #include "base/string_util.h" -#include "base/platform_thread.h" #include "net/base/host_resolver.h" #include "net/base/net_errors.h" @@ -64,22 +63,12 @@ class ScopedHostMapper : public HostMapper { rules_.push_back(Rule(host_pattern, replacement)); } - void AddRuleWithLatency(const char* host_pattern, const char* replacement, - int latency) { - rules_.push_back(Rule(host_pattern, replacement, latency)); - } - private: std::string Map(const std::string& host) { - RuleList::iterator r; + RuleList::const_iterator r; for (r = rules_.begin(); r != rules_.end(); ++r) { - if (MatchPattern(host, r->host_pattern)) { - if (r->latency != 0) { - PlatformThread::Sleep(r->latency); - r->latency = 1; // Simulate cache warmup. - } + if (MatchPattern(host, r->host_pattern)) return r->replacement; - } } return previous_host_mapper_ ? previous_host_mapper_->Map(host) : host; } @@ -87,15 +76,7 @@ class ScopedHostMapper : public HostMapper { struct Rule { std::string host_pattern; std::string replacement; - int latency; // in milliseconds - Rule(const char* h, const char* r) - : host_pattern(h), - replacement(r), - latency(0) {} - Rule(const char* h, const char* r, const int l) - : host_pattern(h), - replacement(r), - latency(l) {} + Rule(const char* h, const char* r) : host_pattern(h), replacement(r) {} }; typedef std::list<Rule> RuleList; |