diff options
| -rw-r--r-- | chrome/browser/net/dns_probe_test_util.cc | 6 | ||||
| -rw-r--r-- | net/base/prioritized_dispatcher.cc | 44 | ||||
| -rw-r--r-- | net/base/prioritized_dispatcher.h | 13 | ||||
| -rw-r--r-- | net/base/prioritized_dispatcher_unittest.cc | 127 | ||||
| -rw-r--r-- | net/base/priority_queue.h | 21 | ||||
| -rw-r--r-- | net/base/priority_queue_unittest.cc | 17 | ||||
| -rw-r--r-- | net/dns/dns_test_util.cc | 111 | ||||
| -rw-r--r-- | net/dns/dns_test_util.h | 35 | ||||
| -rw-r--r-- | net/dns/host_resolver_impl.cc | 312 | ||||
| -rw-r--r-- | net/dns/host_resolver_impl.h | 6 | ||||
| -rw-r--r-- | net/dns/host_resolver_impl_unittest.cc | 310 |
11 files changed, 225 insertions, 777 deletions
diff --git a/chrome/browser/net/dns_probe_test_util.cc b/chrome/browser/net/dns_probe_test_util.cc index fcc5b02..88d481d 100644 --- a/chrome/browser/net/dns_probe_test_util.cc +++ b/chrome/browser/net/dns_probe_test_util.cc @@ -28,10 +28,10 @@ scoped_ptr<DnsClient> CreateMockDnsClientForProbes( const uint16 kTypeA = net::dns_protocol::kTypeA; MockDnsClientRuleList rules; - rules.push_back(MockDnsClientRule(DnsProbeRunner::kKnownGoodHostname, kTypeA, - result, false)); + rules.push_back( + MockDnsClientRule(DnsProbeRunner::kKnownGoodHostname, kTypeA, result)); - return scoped_ptr<DnsClient>(new net::MockDnsClient(config, rules)); + return CreateMockDnsClient(config, rules).Pass(); } } // namespace chrome_browser_net diff --git a/net/base/prioritized_dispatcher.cc b/net/base/prioritized_dispatcher.cc index 55e8dd0..44348e6 100644 --- a/net/base/prioritized_dispatcher.cc +++ b/net/base/prioritized_dispatcher.cc @@ -18,7 +18,17 @@ PrioritizedDispatcher::PrioritizedDispatcher(const Limits& limits) : queue_(limits.reserved_slots.size()), max_running_jobs_(limits.reserved_slots.size()), num_running_jobs_(0) { - SetLimits(limits); + size_t total = 0; + for (size_t i = 0; i < limits.reserved_slots.size(); ++i) { + total += limits.reserved_slots[i]; + max_running_jobs_[i] = total; + } + // Unreserved slots are available for all priorities. + DCHECK_LE(total, limits.total_jobs) << "sum(reserved_slots) <= total_jobs"; + size_t spare = limits.total_jobs - total; + for (size_t i = limits.reserved_slots.size(); i > 0; --i) { + max_running_jobs_[i - 1] += spare; + } } PrioritizedDispatcher::~PrioritizedDispatcher() {} @@ -35,18 +45,6 @@ PrioritizedDispatcher::Handle PrioritizedDispatcher::Add( return queue_.Insert(job, priority); } -PrioritizedDispatcher::Handle PrioritizedDispatcher::AddAtHead( - Job* job, Priority priority) { - DCHECK(job); - DCHECK_LT(priority, num_priorities()); - if (num_running_jobs_ < max_running_jobs_[priority]) { - ++num_running_jobs_; - job->Start(); - return Handle(); - } - return queue_.InsertAtFront(job, priority); -} - void PrioritizedDispatcher::Cancel(const Handle& handle) { queue_.Erase(handle); } @@ -88,11 +86,6 @@ void PrioritizedDispatcher::OnJobFinished() { MaybeDispatchJob(handle, handle.priority()); } -void PrioritizedDispatcher::Disable() { - // Set limits to allow no new jobs to start. - SetLimits(Limits(queue_.num_priorities(), 0)); -} - bool PrioritizedDispatcher::MaybeDispatchJob(const Handle& handle, Priority job_priority) { DCHECK_LT(job_priority, num_priorities()); @@ -105,19 +98,4 @@ bool PrioritizedDispatcher::MaybeDispatchJob(const Handle& handle, return true; } -void PrioritizedDispatcher::SetLimits(const Limits& limits) { - DCHECK_EQ(queue_.num_priorities(), limits.reserved_slots.size()); - size_t total = 0; - for (size_t i = 0; i < limits.reserved_slots.size(); ++i) { - total += limits.reserved_slots[i]; - max_running_jobs_[i] = total; - } - // Unreserved slots are available for all priorities. - DCHECK_LE(total, limits.total_jobs) << "sum(reserved_slots) <= total_jobs"; - size_t spare = limits.total_jobs - total; - for (size_t i = limits.reserved_slots.size(); i > 0; --i) { - max_running_jobs_[i - 1] += spare; - } -} - } // namespace net diff --git a/net/base/prioritized_dispatcher.h b/net/base/prioritized_dispatcher.h index 0146706..708f9d6 100644 --- a/net/base/prioritized_dispatcher.h +++ b/net/base/prioritized_dispatcher.h @@ -78,10 +78,6 @@ class NET_EXPORT_PRIVATE PrioritizedDispatcher { // it is queued in the dispatcher. Handle Add(Job* job, Priority priority); - // Just like Add, except that it adds Job at the font of queue of jobs with - // priorities of |priority|. - Handle AddAtHead(Job* job, Priority priority); - // Removes the job with |handle| from the queue. Invalidates |handle|. // Note: a Handle is valid iff the job is in the queue, i.e. has not Started. void Cancel(const Handle& handle); @@ -98,21 +94,12 @@ class NET_EXPORT_PRIVATE PrioritizedDispatcher { // Notifies the dispatcher that a running job has finished. Could start a job. void OnJobFinished(); - // Tells the dispatcher not to start new jobs when old jobs complete or are - // cancelled. Can't be undone. Can be used to avoid starting new jobs during - // cleanup. - void Disable(); - private: // Attempts to dispatch the job with |handle| at priority |priority| (might be // different than |handle.priority()|. Returns true if successful. If so // the |handle| becomes invalid. bool MaybeDispatchJob(const Handle& handle, Priority priority); - // Updates |max_running_jobs_| to match |limits|. Does not start any new - // new jobs, even if the new limits would allow queued jobs to start. - void SetLimits(const Limits& limits); - // Queue for jobs that need to wait for a spare slot. PriorityQueue<Job*> queue_; // Maximum total number of running jobs allowed after a job at a particular diff --git a/net/base/prioritized_dispatcher_unittest.cc b/net/base/prioritized_dispatcher_unittest.cc index dc86879d..41a09c5 100644 --- a/net/base/prioritized_dispatcher_unittest.cc +++ b/net/base/prioritized_dispatcher_unittest.cc @@ -52,17 +52,13 @@ class PrioritizedDispatcherTest : public testing::Test { return handle_; } - void Add(bool at_head) { + void Add() { CHECK(handle_.is_null()); CHECK(!running_); size_t num_queued = dispatcher_->num_queued_jobs(); size_t num_running = dispatcher_->num_running_jobs(); - if (!at_head) { - handle_ = dispatcher_->Add(this, priority_); - } else { - handle_ = dispatcher_->AddAtHead(this, priority_); - } + handle_ = dispatcher_->Add(this, priority_); if (handle_.is_null()) { EXPECT_EQ(num_queued, dispatcher_->num_queued_jobs()); @@ -144,14 +140,7 @@ class PrioritizedDispatcherTest : public testing::Test { TestJob* AddJob(char data, Priority priority) { TestJob* job = new TestJob(dispatcher_.get(), data, priority, &log_); jobs_.push_back(job); - job->Add(false); - return job; - } - - TestJob* AddJobAtHead(char data, Priority priority) { - TestJob* job = new TestJob(dispatcher_.get(), data, priority, &log_); - jobs_.push_back(job); - job->Add(true); + job->Add(); return job; } @@ -213,33 +202,6 @@ TEST_F(PrioritizedDispatcherTest, AddPriority) { Expect("a.c.d.b.e."); } -TEST_F(PrioritizedDispatcherTest, AddAtHead) { - PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, 1); - Prepare(limits); - - TestJob* job_a = AddJob('a', MEDIUM); - TestJob* job_b = AddJobAtHead('b', MEDIUM); - TestJob* job_c = AddJobAtHead('c', HIGHEST); - TestJob* job_d = AddJobAtHead('d', HIGHEST); - TestJob* job_e = AddJobAtHead('e', MEDIUM); - TestJob* job_f = AddJob('f', MEDIUM); - - ASSERT_TRUE(job_a->running()); - job_a->Finish(); - ASSERT_TRUE(job_d->running()); - job_d->Finish(); - ASSERT_TRUE(job_c->running()); - job_c->Finish(); - ASSERT_TRUE(job_e->running()); - job_e->Finish(); - ASSERT_TRUE(job_b->running()); - job_b->Finish(); - ASSERT_TRUE(job_f->running()); - job_f->Finish(); - - Expect("a.d.c.e.b.f."); -} - TEST_F(PrioritizedDispatcherTest, EnforceLimits) { // Reserve 2 for HIGHEST and 1 for LOW or higher. // This leaves 2 for LOWEST or lower. @@ -283,40 +245,29 @@ TEST_F(PrioritizedDispatcherTest, EnforceLimits) { } TEST_F(PrioritizedDispatcherTest, ChangePriority) { - PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, 2); - // Reserve one slot only for HIGHEST priority requests. - limits.reserved_slots[HIGHEST] = 1; + PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, 1); Prepare(limits); TestJob* job_a = AddJob('a', IDLE); - TestJob* job_b = AddJob('b', LOW); - TestJob* job_c = AddJob('c', MEDIUM); - TestJob* job_d = AddJob('d', MEDIUM); - TestJob* job_e = AddJob('e', IDLE); + TestJob* job_b = AddJob('b', MEDIUM); + TestJob* job_c = AddJob('c', HIGHEST); + TestJob* job_d = AddJob('d', HIGHEST); ASSERT_FALSE(job_b->running()); ASSERT_FALSE(job_c->running()); - job_b->ChangePriority(MEDIUM); - job_c->ChangePriority(LOW); + job_b->ChangePriority(HIGHEST); + job_c->ChangePriority(MEDIUM); ASSERT_TRUE(job_a->running()); job_a->Finish(); ASSERT_TRUE(job_d->running()); job_d->Finish(); - - EXPECT_FALSE(job_e->running()); - // Increasing |job_e|'s priority to HIGHEST should result in it being - // started immediately. - job_e->ChangePriority(HIGHEST); - ASSERT_TRUE(job_e->running()); - job_e->Finish(); - ASSERT_TRUE(job_b->running()); job_b->Finish(); ASSERT_TRUE(job_c->running()); job_c->Finish(); - Expect("a.d.be..c."); + Expect("a.d.b.c."); } TEST_F(PrioritizedDispatcherTest, Cancel) { @@ -373,64 +324,6 @@ TEST_F(PrioritizedDispatcherTest, EvictFromEmpty) { EXPECT_TRUE(dispatcher_->EvictOldestLowest() == NULL); } -TEST_F(PrioritizedDispatcherTest, AddWhileDisabled) { - PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, 1); - Prepare(limits); - - dispatcher_->Disable(); - TestJob* job_a = AddJob('a', MEDIUM); - TestJob* job_b = AddJobAtHead('b', MEDIUM); - - EXPECT_FALSE(job_a->running()); - EXPECT_FALSE(job_b->running()); - EXPECT_EQ(0u, dispatcher_->num_running_jobs()); - EXPECT_EQ(2u, dispatcher_->num_queued_jobs()); -} - -TEST_F(PrioritizedDispatcherTest, DisableThenCancel) { - PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, 1); - Prepare(limits); - - TestJob* job_a = AddJob('a', IDLE); - TestJob* job_b = AddJob('b', IDLE); - TestJob* job_c = AddJob('c', IDLE); - dispatcher_->Disable(); - - EXPECT_TRUE(job_a->running()); - EXPECT_FALSE(job_b->running()); - EXPECT_FALSE(job_c->running()); - job_a->Finish(); - - EXPECT_FALSE(job_b->running()); - EXPECT_FALSE(job_c->running()); - - job_b->Cancel(); - EXPECT_FALSE(job_c->running()); - job_c->Cancel(); - - Expect("a."); -} - -TEST_F(PrioritizedDispatcherTest, DisableThenIncreatePriority) { - PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, 2); - limits.reserved_slots[HIGHEST] = 1; - Prepare(limits); - - TestJob* job_a = AddJob('a', IDLE); - TestJob* job_b = AddJob('b', IDLE); - EXPECT_TRUE(job_a->running()); - EXPECT_FALSE(job_b->running()); - dispatcher_->Disable(); - - job_b->ChangePriority(HIGHEST); - EXPECT_FALSE(job_b->running()); - job_a->Finish(); - EXPECT_FALSE(job_b->running()); - - job_b->Cancel(); - Expect("a."); -} - #if GTEST_HAS_DEATH_TEST && !defined(NDEBUG) TEST_F(PrioritizedDispatcherTest, CancelNull) { PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, 1); diff --git a/net/base/priority_queue.h b/net/base/priority_queue.h index c684580..b758ca4 100644 --- a/net/base/priority_queue.h +++ b/net/base/priority_queue.h @@ -139,24 +139,6 @@ class PriorityQueue : public base::NonThreadSafe { #endif } - // Adds |value| with |priority| to the queue. Returns a pointer to the - // created element. - Pointer InsertAtFront(const T& value, Priority priority) { - DCHECK(CalledOnValidThread()); - DCHECK_LT(priority, lists_.size()); - ++size_; - List& list = lists_[priority]; -#if !defined(NDEBUG) - unsigned id = next_id_; - valid_ids_.insert(id); - ++next_id_; - return Pointer(priority, list.insert(list.begin(), - std::make_pair(id, value))); -#else - return Pointer(priority, list.insert(list.begin(), value)); -#endif - } - // Removes the value pointed by |pointer| from the queue. All pointers to this // value including |pointer| become invalid. void Erase(const Pointer& pointer) { @@ -231,9 +213,6 @@ class PriorityQueue : public base::NonThreadSafe { size_ = 0u; } - // Returns the number of priorities the queue supports. - size_t num_priorities() const { return lists_.size(); } - // Returns number of queued values. size_t size() const { DCHECK(CalledOnValidThread()); diff --git a/net/base/priority_queue_unittest.cc b/net/base/priority_queue_unittest.cc index 7e3e045..c8449bb 100644 --- a/net/base/priority_queue_unittest.cc +++ b/net/base/priority_queue_unittest.cc @@ -91,22 +91,7 @@ TEST_F(PriorityQueueTest, EraseFromMiddle) { queue_.Erase(pointers_[2]); queue_.Erase(pointers_[3]); - const int expected_order[] = { 8, 1, 6, 0, 5, 4, 7 }; - - for (size_t i = 0; i < arraysize(expected_order); ++i) { - EXPECT_EQ(expected_order[i], queue_.FirstMin().value()); - queue_.Erase(queue_.FirstMin()); - } - CheckEmpty(); -} - -TEST_F(PriorityQueueTest, InsertAtFront) { - queue_.InsertAtFront(9, 2); - queue_.InsertAtFront(10, 0); - queue_.InsertAtFront(11, 1); - queue_.InsertAtFront(12, 1); - - const int expected_order[] = { 10, 3, 8, 12, 11, 1, 6, 9, 0, 2, 5, 4, 7 }; + int expected_order[] = { 8, 1, 6, 0, 5, 4, 7 }; for (size_t i = 0; i < arraysize(expected_order); ++i) { EXPECT_EQ(expected_order[i], queue_.FirstMin().value()); diff --git a/net/dns/dns_test_util.cc b/net/dns/dns_test_util.cc index 6301421..37bf855 100644 --- a/net/dns/dns_test_util.cc +++ b/net/dns/dns_test_util.cc @@ -15,6 +15,9 @@ #include "net/base/io_buffer.h" #include "net/base/net_errors.h" #include "net/dns/address_sorter.h" +#include "net/dns/dns_client.h" +#include "net/dns/dns_config_service.h" +#include "net/dns/dns_protocol.h" #include "net/dns/dns_query.h" #include "net/dns/dns_response.h" #include "net/dns/dns_transaction.h" @@ -23,16 +26,6 @@ namespace net { namespace { -class MockAddressSorter : public AddressSorter { - public: - virtual ~MockAddressSorter() {} - virtual void Sort(const AddressList& list, - const CallbackType& callback) const OVERRIDE { - // Do nothing. - callback.Run(true, list); - } -}; - // A DnsTransaction which uses MockDnsClientRuleList to determine the response. class MockTransaction : public DnsTransaction, public base::SupportsWeakPtr<MockTransaction> { @@ -45,8 +38,7 @@ class MockTransaction : public DnsTransaction, hostname_(hostname), qtype_(qtype), callback_(callback), - started_(false), - delayed_(false) { + started_(false) { // Find the relevant rule which matches |qtype| and prefix of |hostname|. for (size_t i = 0; i < rules.size(); ++i) { const std::string& prefix = rules[i].prefix; @@ -54,7 +46,6 @@ class MockTransaction : public DnsTransaction, (hostname.size() >= prefix.size()) && (hostname.compare(0, prefix.size(), prefix) == 0)) { result_ = rules[i].result; - delayed_ = rules[i].delay; break; } } @@ -71,21 +62,11 @@ class MockTransaction : public DnsTransaction, virtual void Start() OVERRIDE { EXPECT_FALSE(started_); started_ = true; - if (delayed_) - return; // Using WeakPtr to cleanly cancel when transaction is destroyed. base::MessageLoop::current()->PostTask( FROM_HERE, base::Bind(&MockTransaction::Finish, AsWeakPtr())); } - void FinishDelayedTransaction() { - EXPECT_TRUE(delayed_); - delayed_ = false; - Finish(); - } - - bool delayed() const { return delayed_; } - private: void Finish() { switch (result_) { @@ -155,17 +136,14 @@ class MockTransaction : public DnsTransaction, const uint16 qtype_; DnsTransactionFactory::CallbackType callback_; bool started_; - bool delayed_; }; -} // namespace // A DnsTransactionFactory which creates MockTransaction. class MockTransactionFactory : public DnsTransactionFactory { public: explicit MockTransactionFactory(const MockDnsClientRuleList& rules) : rules_(rules) {} - virtual ~MockTransactionFactory() {} virtual scoped_ptr<DnsTransaction> CreateTransaction( @@ -173,57 +151,60 @@ class MockTransactionFactory : public DnsTransactionFactory { uint16 qtype, const DnsTransactionFactory::CallbackType& callback, const BoundNetLog&) OVERRIDE { - MockTransaction* transaction = - new MockTransaction(rules_, hostname, qtype, callback); - if (transaction->delayed()) - delayed_transactions_.push_back(transaction->AsWeakPtr()); - return scoped_ptr<DnsTransaction>(transaction); - } - - void CompleteDelayedTransactions() { - DelayedTransactionList old_delayed_transactions; - old_delayed_transactions.swap(delayed_transactions_); - for (DelayedTransactionList::iterator it = old_delayed_transactions.begin(); - it != old_delayed_transactions.end(); ++it) { - if (it->get()) - (*it)->FinishDelayedTransaction(); - } + return scoped_ptr<DnsTransaction>( + new MockTransaction(rules_, hostname, qtype, callback)); } private: - typedef std::vector<base::WeakPtr<MockTransaction> > DelayedTransactionList; - MockDnsClientRuleList rules_; - DelayedTransactionList delayed_transactions_; }; -MockDnsClient::MockDnsClient(const DnsConfig& config, - const MockDnsClientRuleList& rules) - : config_(config), - factory_(new MockTransactionFactory(rules)), - address_sorter_(new MockAddressSorter()) { -} +class MockAddressSorter : public AddressSorter { + public: + virtual ~MockAddressSorter() {} + virtual void Sort(const AddressList& list, + const CallbackType& callback) const OVERRIDE { + // Do nothing. + callback.Run(true, list); + } +}; -MockDnsClient::~MockDnsClient() {} +// MockDnsClient provides MockTransactionFactory. +class MockDnsClient : public DnsClient { + public: + MockDnsClient(const DnsConfig& config, + const MockDnsClientRuleList& rules) + : config_(config), factory_(rules) {} + virtual ~MockDnsClient() {} -void MockDnsClient::SetConfig(const DnsConfig& config) { - config_ = config; -} + virtual void SetConfig(const DnsConfig& config) OVERRIDE { + config_ = config; + } -const DnsConfig* MockDnsClient::GetConfig() const { - return config_.IsValid() ? &config_ : NULL; -} + virtual const DnsConfig* GetConfig() const OVERRIDE { + return config_.IsValid() ? &config_ : NULL; + } -DnsTransactionFactory* MockDnsClient::GetTransactionFactory() { - return config_.IsValid() ? factory_.get() : NULL; -} + virtual DnsTransactionFactory* GetTransactionFactory() OVERRIDE { + return config_.IsValid() ? &factory_ : NULL; + } -AddressSorter* MockDnsClient::GetAddressSorter() { - return address_sorter_.get(); -} + virtual AddressSorter* GetAddressSorter() OVERRIDE { + return &address_sorter_; + } + + private: + DnsConfig config_; + MockTransactionFactory factory_; + MockAddressSorter address_sorter_; +}; + +} // namespace -void MockDnsClient::CompleteDelayedTransactions() { - factory_->CompleteDelayedTransactions(); +// static +scoped_ptr<DnsClient> CreateMockDnsClient(const DnsConfig& config, + const MockDnsClientRuleList& rules) { + return scoped_ptr<DnsClient>(new MockDnsClient(config, rules)); } } // namespace net diff --git a/net/dns/dns_test_util.h b/net/dns/dns_test_util.h index d0b8e81..d447b29 100644 --- a/net/dns/dns_test_util.h +++ b/net/dns/dns_test_util.h @@ -10,7 +10,6 @@ #include "base/basictypes.h" #include "base/memory/scoped_ptr.h" -#include "net/dns/dns_client.h" #include "net/dns/dns_config_service.h" #include "net/dns/dns_protocol.h" @@ -175,9 +174,7 @@ static const int kT3TTL = 0x00000015; // +2 for the CNAME records, +1 for TXT record. static const unsigned kT3RecordCount = arraysize(kT3IpAddresses) + 3; -class AddressSorter; class DnsClient; -class MockTransactionFactory; struct MockDnsClientRule { enum Result { @@ -187,43 +184,21 @@ struct MockDnsClientRule { OK, // Return a response with loopback address. }; - // If |delay| is true, matching transactions will be delayed until triggered - // by the consumer. MockDnsClientRule(const std::string& prefix_arg, uint16 qtype_arg, - Result result_arg, - bool delay) - : result(result_arg), prefix(prefix_arg), qtype(qtype_arg), - delay(delay) {} + Result result_arg) + : result(result_arg), prefix(prefix_arg), qtype(qtype_arg) { } Result result; std::string prefix; uint16 qtype; - bool delay; }; typedef std::vector<MockDnsClientRule> MockDnsClientRuleList; -// MockDnsClient provides MockTransactionFactory. -class MockDnsClient : public DnsClient { - public: - MockDnsClient(const DnsConfig& config, const MockDnsClientRuleList& rules); - virtual ~MockDnsClient(); - - // DnsClient interface: - virtual void SetConfig(const DnsConfig& config) OVERRIDE; - virtual const DnsConfig* GetConfig() const OVERRIDE; - virtual DnsTransactionFactory* GetTransactionFactory() OVERRIDE; - virtual AddressSorter* GetAddressSorter() OVERRIDE; - - // Completes all DnsTransactions that were delayed by a rule. - void CompleteDelayedTransactions(); - - private: - DnsConfig config_; - scoped_ptr<MockTransactionFactory> factory_; - scoped_ptr<AddressSorter> address_sorter_; -}; +// Creates mock DnsClient for testing HostResolverImpl. +scoped_ptr<DnsClient> CreateMockDnsClient(const DnsConfig& config, + const MockDnsClientRuleList& rules); } // namespace net diff --git a/net/dns/host_resolver_impl.cc b/net/dns/host_resolver_impl.cc index 010f987..d10949f 100644 --- a/net/dns/host_resolver_impl.cc +++ b/net/dns/host_resolver_impl.cc @@ -970,98 +970,54 @@ class HostResolverImpl::LoopbackProbeJob { // TODO(szym): This could be moved to separate source file as well. class HostResolverImpl::DnsTask : public base::SupportsWeakPtr<DnsTask> { public: - class Delegate { - public: - virtual void OnDnsTaskComplete(base::TimeTicks start_time, - int net_error, - const AddressList& addr_list, - base::TimeDelta ttl) = 0; - - // Called when the first of two jobs succeeds. If the first completed - // transaction fails, this is not called. Also not called when the DnsTask - // only needs to run one transaction. - virtual void OnFirstDnsTransactionComplete() = 0; - - protected: - Delegate() {} - virtual ~Delegate() {} - }; + typedef base::Callback<void(int net_error, + const AddressList& addr_list, + base::TimeDelta ttl)> Callback; DnsTask(DnsClient* client, const Key& key, - Delegate* delegate, + const Callback& callback, const BoundNetLog& job_net_log) : client_(client), - key_(key), - delegate_(delegate), - net_log_(job_net_log), - num_completed_transactions_(0), - task_start_time_(base::TimeTicks::Now()) { + family_(key.address_family), + callback_(callback), + net_log_(job_net_log) { DCHECK(client); - DCHECK(delegate_); - } - - bool needs_two_transactions() const { - return key_.address_family == ADDRESS_FAMILY_UNSPECIFIED; - } - - bool needs_another_transaction() const { - return needs_two_transactions() && !transaction_aaaa_; + DCHECK(!callback.is_null()); + + // If unspecified, do IPv4 first, because suffix search will be faster. + uint16 qtype = (family_ == ADDRESS_FAMILY_IPV6) ? + dns_protocol::kTypeAAAA : + dns_protocol::kTypeA; + transaction_ = client_->GetTransactionFactory()->CreateTransaction( + key.hostname, + qtype, + base::Bind(&DnsTask::OnTransactionComplete, base::Unretained(this), + true /* first_query */, base::TimeTicks::Now()), + net_log_); } - void StartFirstTransaction() { - DCHECK_EQ(0u, num_completed_transactions_); + void Start() { net_log_.BeginEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK); - if (key_.address_family == ADDRESS_FAMILY_IPV6) { - StartAAAA(); - } else { - StartA(); - } - } - - void StartSecondTransaction() { - DCHECK(needs_two_transactions()); - StartAAAA(); + transaction_->Start(); } private: - void StartA() { - DCHECK(!transaction_a_); - DCHECK_NE(ADDRESS_FAMILY_IPV6, key_.address_family); - transaction_a_ = CreateTransaction(ADDRESS_FAMILY_IPV4); - transaction_a_->Start(); - } - - void StartAAAA() { - DCHECK(!transaction_aaaa_); - DCHECK_NE(ADDRESS_FAMILY_IPV4, key_.address_family); - transaction_aaaa_ = CreateTransaction(ADDRESS_FAMILY_IPV6); - transaction_aaaa_->Start(); - } - - scoped_ptr<DnsTransaction> CreateTransaction(AddressFamily family) { - DCHECK_NE(ADDRESS_FAMILY_UNSPECIFIED, family); - return client_->GetTransactionFactory()->CreateTransaction( - key_.hostname, - family == ADDRESS_FAMILY_IPV6 ? dns_protocol::kTypeAAAA : - dns_protocol::kTypeA, - base::Bind(&DnsTask::OnTransactionComplete, base::Unretained(this), - base::TimeTicks::Now()), - net_log_); - } - - void OnTransactionComplete(const base::TimeTicks& start_time, + void OnTransactionComplete(bool first_query, + const base::TimeTicks& start_time, DnsTransaction* transaction, int net_error, const DnsResponse* response) { DCHECK(transaction); base::TimeDelta duration = base::TimeTicks::Now() - start_time; + // Run |callback_| last since the owning Job will then delete this DnsTask. if (net_error != OK) { DNS_HISTOGRAM("AsyncDNS.TransactionFailure", duration); OnFailure(net_error, DnsResponse::DNS_PARSE_OK); return; } + CHECK(response); DNS_HISTOGRAM("AsyncDNS.TransactionSuccess", duration); switch (transaction->GetType()) { case dns_protocol::kTypeA: @@ -1071,7 +1027,6 @@ class HostResolverImpl::DnsTask : public base::SupportsWeakPtr<DnsTask> { DNS_HISTOGRAM("AsyncDNS.TransactionSuccess_AAAA", duration); break; } - AddressList addr_list; base::TimeDelta ttl; DnsResponse::Result result = response->ParseToAddressList(&addr_list, &ttl); @@ -1084,47 +1039,58 @@ class HostResolverImpl::DnsTask : public base::SupportsWeakPtr<DnsTask> { return; } - ++num_completed_transactions_; - if (num_completed_transactions_ == 1) { - ttl_ = ttl; - } else { - ttl_ = std::min(ttl_, ttl); - } - - if (transaction->GetType() == dns_protocol::kTypeA) { - DCHECK_EQ(transaction_a_.get(), transaction); - // Place IPv4 addresses after IPv6. - addr_list_.insert(addr_list_.end(), addr_list.begin(), addr_list.end()); + bool needs_sort = false; + if (first_query) { + DCHECK(client_->GetConfig()) << + "Transaction should have been aborted when config changed!"; + if (family_ == ADDRESS_FAMILY_IPV6) { + needs_sort = (addr_list.size() > 1); + } else if (family_ == ADDRESS_FAMILY_UNSPECIFIED) { + first_addr_list_ = addr_list; + first_ttl_ = ttl; + // Use fully-qualified domain name to avoid search. + transaction_ = client_->GetTransactionFactory()->CreateTransaction( + response->GetDottedName() + ".", + dns_protocol::kTypeAAAA, + base::Bind(&DnsTask::OnTransactionComplete, base::Unretained(this), + false /* first_query */, base::TimeTicks::Now()), + net_log_); + transaction_->Start(); + return; + } } else { - DCHECK_EQ(transaction_aaaa_.get(), transaction); - // Place IPv6 addresses before IPv4. - addr_list_.insert(addr_list_.begin(), addr_list.begin(), addr_list.end()); + DCHECK_EQ(ADDRESS_FAMILY_UNSPECIFIED, family_); + bool has_ipv6_addresses = !addr_list.empty(); + if (!first_addr_list_.empty()) { + ttl = std::min(ttl, first_ttl_); + // Place IPv4 addresses after IPv6. + addr_list.insert(addr_list.end(), first_addr_list_.begin(), + first_addr_list_.end()); + } + needs_sort = (has_ipv6_addresses && addr_list.size() > 1); } - if (needs_two_transactions() && num_completed_transactions_ == 1) { - // No need to repeat the suffix search. - key_.hostname = transaction->GetHostname(); - delegate_->OnFirstDnsTransactionComplete(); + if (addr_list.empty()) { + // TODO(szym): Don't fallback to ProcTask in this case. + OnFailure(ERR_NAME_NOT_RESOLVED, DnsResponse::DNS_PARSE_OK); return; } - // If there are multiple addresses, and at least one is IPv6, need to sort - // them. Note that IPv6 addresses are always put before IPv4 ones, so it's - // sufficient to just check the family of the first address. - if (addr_list_.size() > 1 && - addr_list_[0].GetFamily() == ADDRESS_FAMILY_IPV6) { - // Sort addresses if needed. Sort could complete synchronously. + if (needs_sort) { + // Sort could complete synchronously. client_->GetAddressSorter()->Sort( - addr_list_, + addr_list, base::Bind(&DnsTask::OnSortComplete, AsWeakPtr(), - base::TimeTicks::Now())); + base::TimeTicks::Now(), + ttl)); } else { - OnSuccess(addr_list_); + OnSuccess(addr_list, ttl); } } void OnSortComplete(base::TimeTicks start_time, + base::TimeDelta ttl, bool success, const AddressList& addr_list) { if (!success) { @@ -1144,7 +1110,7 @@ class HostResolverImpl::DnsTask : public base::SupportsWeakPtr<DnsTask> { return; } - OnSuccess(addr_list); + OnSuccess(addr_list, ttl); } void OnFailure(int net_error, DnsResponse::Result result) { @@ -1152,34 +1118,26 @@ class HostResolverImpl::DnsTask : public base::SupportsWeakPtr<DnsTask> { net_log_.EndEvent( NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK, base::Bind(&NetLogDnsTaskFailedCallback, net_error, result)); - delegate_->OnDnsTaskComplete(task_start_time_, net_error, AddressList(), - base::TimeDelta()); + callback_.Run(net_error, AddressList(), base::TimeDelta()); } - void OnSuccess(const AddressList& addr_list) { + void OnSuccess(const AddressList& addr_list, base::TimeDelta ttl) { net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK, addr_list.CreateNetLogCallback()); - delegate_->OnDnsTaskComplete(task_start_time_, OK, addr_list, ttl_); + callback_.Run(OK, addr_list, ttl); } DnsClient* client_; - Key key_; - + AddressFamily family_; // The listener to the results of this DnsTask. - Delegate* delegate_; + Callback callback_; const BoundNetLog net_log_; - scoped_ptr<DnsTransaction> transaction_a_; - scoped_ptr<DnsTransaction> transaction_aaaa_; - - unsigned num_completed_transactions_; + scoped_ptr<DnsTransaction> transaction_; - // These are updated as each transaction completes. - base::TimeDelta ttl_; - // IPv6 addresses must appear first in the list. - AddressList addr_list_; - - base::TimeTicks task_start_time_; + // Results from the first transaction. Used only if |family_| is unspecified. + AddressList first_addr_list_; + base::TimeDelta first_ttl_; DISALLOW_COPY_AND_ASSIGN(DnsTask); }; @@ -1187,8 +1145,7 @@ class HostResolverImpl::DnsTask : public base::SupportsWeakPtr<DnsTask> { //----------------------------------------------------------------------------- // Aggregates all Requests for the same Key. Dispatched via PriorityDispatch. -class HostResolverImpl::Job : public PrioritizedDispatcher::Job, - public HostResolverImpl::DnsTask::Delegate { +class HostResolverImpl::Job : public PrioritizedDispatcher::Job { public: // Creates new job for |key| where |request_net_log| is bound to the // request that spawned it. @@ -1201,7 +1158,6 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job, priority_tracker_(priority), had_non_speculative_request_(false), had_dns_config_(false), - num_occupied_job_slots_(0), dns_task_error_(OK), creation_time_(base::TimeTicks::Now()), priority_change_time_(creation_time_), @@ -1225,7 +1181,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job, proc_task_ = NULL; } // Clean up now for nice NetLog. - KillDnsTask(); + dns_task_.reset(NULL); net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB, ERR_ABORTED); } else if (is_queued()) { @@ -1248,23 +1204,9 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job, } } - // Add this job to the dispatcher. If "at_head" is true, adds at the front - // of the queue. - void Schedule(bool at_head) { - DCHECK(!is_queued()); - PrioritizedDispatcher::Handle handle; - if (!at_head) { - handle = resolver_->dispatcher_.Add(this, priority()); - } else { - handle = resolver_->dispatcher_.AddAtHead(this, priority()); - } - // The dispatcher could have started |this| in the above call to Add, which - // could have called Schedule again. In that case |handle| will be null, - // but |handle_| may have been set by the other nested call to Schedule. - if (!handle.is_null()) { - DCHECK(handle_.is_null()); - handle_ = handle; - } + // Add this job to the dispatcher. + void Schedule() { + handle_ = resolver_->dispatcher_.Add(this, priority()); } void AddRequest(scoped_ptr<Request> req) { @@ -1332,7 +1274,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job, // If DnsTask present, abort it and fall back to ProcTask. void AbortDnsTask() { if (dns_task_) { - KillDnsTask(); + dns_task_.reset(); dns_task_error_ = OK; StartProcTask(); } @@ -1381,29 +1323,6 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job, } private: - void KillDnsTask() { - if (dns_task_) { - ReduceToOneJobSlot(); - dns_task_.reset(); - } - } - - // Reduce the number of job slots occupied and queued in the dispatcher - // to one. If the second Job slot is queued in the dispatcher, cancels the - // queued job. Otherwise, the second Job has been started by the - // PrioritizedDispatcher, so signals it is complete. - void ReduceToOneJobSlot() { - DCHECK_GE(num_occupied_job_slots_, 1u); - if (is_queued()) { - resolver_->dispatcher_.Cancel(handle_); - handle_.Reset(); - } else if (num_occupied_job_slots_ > 1) { - resolver_->dispatcher_.OnJobFinished(); - --num_occupied_job_slots_; - } - DCHECK_EQ(1u, num_occupied_job_slots_); - } - void UpdatePriority() { if (is_queued()) { if (priority() != static_cast<RequestPriority>(handle_.priority())) @@ -1420,17 +1339,8 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job, // PriorityDispatch::Job: virtual void Start() OVERRIDE { - DCHECK_LE(num_occupied_job_slots_, 1u); - - handle_.Reset(); - ++num_occupied_job_slots_; - - if (num_occupied_job_slots_ == 2) { - StartSecondDnsTransaction(); - return; - } - DCHECK(!is_running()); + handle_.Reset(); net_log_.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_STARTED); @@ -1534,18 +1444,14 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job, void StartDnsTask() { DCHECK(resolver_->HaveDnsConfig()); - dns_task_.reset(new DnsTask(resolver_->dns_client_.get(), key_, this, - net_log_)); - - dns_task_->StartFirstTransaction(); - // Schedule a second transaction, if needed. - if (dns_task_->needs_two_transactions()) - Schedule(true); - } + base::TimeTicks start_time = base::TimeTicks::Now(); + dns_task_.reset(new DnsTask( + resolver_->dns_client_.get(), + key_, + base::Bind(&Job::OnDnsTaskComplete, base::Unretained(this), start_time), + net_log_)); - void StartSecondDnsTransaction() { - DCHECK(dns_task_->needs_two_transactions()); - dns_task_->StartSecondTransaction(); + dns_task_->Start(); } // Called if DnsTask fails. It is posted from StartDnsTask, so Job may be @@ -1567,7 +1473,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job, // TODO(szym): Some net errors indicate lack of connectivity. Starting // ProcTask in that case is a waste of time. if (resolver_->fallback_to_proctask_) { - KillDnsTask(); + dns_task_.reset(); StartProcTask(); } else { UmaAsyncDnsResolveStatus(RESOLVE_STATUS_FAIL); @@ -1575,13 +1481,11 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job, } } - - // HostResolverImpl::DnsTask::Delegate implementation: - - virtual void OnDnsTaskComplete(base::TimeTicks start_time, - int net_error, - const AddressList& addr_list, - base::TimeDelta ttl) OVERRIDE { + // Called by DnsTask when it completes. + void OnDnsTaskComplete(base::TimeTicks start_time, + int net_error, + const AddressList& addr_list, + base::TimeDelta ttl) { DCHECK(is_dns_running()); base::TimeDelta duration = base::TimeTicks::Now() - start_time; @@ -1616,19 +1520,6 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job, bounded_ttl); } - virtual void OnFirstDnsTransactionComplete() OVERRIDE { - DCHECK(dns_task_->needs_two_transactions()); - DCHECK_EQ(dns_task_->needs_another_transaction(), is_queued()); - // No longer need to occupy two dispatcher slots. - ReduceToOneJobSlot(); - - // We already have a job slot at the dispatcher, so if the second - // transaction hasn't started, reuse it now instead of waiting in the queue - // for the second slot. - if (dns_task_->needs_another_transaction()) - dns_task_->StartSecondTransaction(); - } - // Performs Job's last rites. Completes all Requests. Deletes this. void CompleteRequests(const HostCache::Entry& entry, base::TimeDelta ttl) { @@ -1643,12 +1534,12 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job, resolver_->RemoveJob(this); if (is_running()) { + DCHECK(!is_queued()); if (is_proc_running()) { - DCHECK(!is_queued()); proc_task_->Cancel(); proc_task_ = NULL; } - KillDnsTask(); + dns_task_.reset(); // Signal dispatcher that a slot has opened. resolver_->dispatcher_.OnJobFinished(); @@ -1742,9 +1633,6 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job, // Distinguishes measurements taken while DnsClient was fully configured. bool had_dns_config_; - // Number of slots occupied by this Job in resolver's PrioritizedDispatcher. - unsigned num_occupied_job_slots_; - // Result of DnsTask. int dns_task_error_; @@ -1832,10 +1720,7 @@ HostResolverImpl::HostResolverImpl( } HostResolverImpl::~HostResolverImpl() { - // Prevent the dispatcher from starting new jobs. - dispatcher_.Disable(); - // It's now safe for Jobs to call KillDsnTask on destruction, because - // OnJobComplete will not start any new jobs. + // This will also cancel all outstanding requests. STLDeleteValues(&jobs_); NetworkChangeNotifier::RemoveIPAddressObserver(this); @@ -1888,7 +1773,7 @@ int HostResolverImpl::Resolve(const RequestInfo& info, if (jobit == jobs_.end()) { job = new Job(weak_ptr_factory_.GetWeakPtr(), key, priority, request_net_log); - job->Schedule(false); + job->Schedule(); // Check for queue overflow. if (dispatcher_.num_queued_jobs() > max_queued_jobs_) { @@ -2185,6 +2070,9 @@ void HostResolverImpl::AbortAllInProgressJobs() { } } + // Check if no dispatcher slots leaked out. + DCHECK_EQ(dispatcher_.num_running_jobs(), jobs_to_abort.size()); + // Life check to bail once |this| is deleted. base::WeakPtr<HostResolverImpl> self = weak_ptr_factory_.GetWeakPtr(); diff --git a/net/dns/host_resolver_impl.h b/net/dns/host_resolver_impl.h index c25adb5..aa6421c 100644 --- a/net/dns/host_resolver_impl.h +++ b/net/dns/host_resolver_impl.h @@ -225,10 +225,8 @@ class NET_EXPORT HostResolverImpl // and resulted in |net_error|. void OnDnsTaskResolve(int net_error); - // Allows the tests to catch slots leaking out of the dispatcher. One - // HostResolverImpl::Job could occupy multiple PrioritizedDispatcher job - // slots. - size_t num_running_dispatcher_jobs_for_tests() const { + // Allows the tests to catch slots leaking out of the dispatcher. + size_t num_running_jobs_for_tests() const { return dispatcher_.num_running_jobs(); } diff --git a/net/dns/host_resolver_impl_unittest.cc b/net/dns/host_resolver_impl_unittest.cc index 987ad68..d604b64 100644 --- a/net/dns/host_resolver_impl_unittest.cc +++ b/net/dns/host_resolver_impl_unittest.cc @@ -12,7 +12,6 @@ #include "base/memory/ref_counted.h" #include "base/memory/scoped_vector.h" #include "base/message_loop/message_loop.h" -#include "base/run_loop.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "base/synchronization/condition_variable.h" @@ -421,20 +420,6 @@ class HostResolverImplTest : public testing::Test { HostResolverImplTest() : proc_(new MockHostResolverProc()) {} - void CreateResolver() { - CreateResolverWithLimitsAndParams(DefaultLimits(), - DefaultParams(proc_.get())); - } - - // This HostResolverImpl will only allow 1 outstanding resolve at a time and - // perform no retries. - void CreateSerialResolver() { - HostResolverImpl::ProcTaskParams params = DefaultParams(proc_.get()); - params.max_retry_attempts = 0u; - PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, 1); - CreateResolverWithLimitsAndParams(limits, params); - } - protected: // A Request::Handler which is a proxy to the HostResolverImplTest fixture. struct Handler : public Request::Handler { @@ -458,22 +443,24 @@ class HostResolverImplTest : public testing::Test { HostResolverImplTest* test; }; - // testing::Test implementation: - virtual void SetUp() OVERRIDE { - CreateResolver(); - } - - virtual void TearDown() OVERRIDE { - if (resolver_.get()) - EXPECT_EQ(0u, resolver_->num_running_dispatcher_jobs_for_tests()); - EXPECT_FALSE(proc_->HasBlockedRequests()); + void CreateResolver() { + resolver_.reset(new HostResolverImpl(HostCache::CreateDefaultCache(), + DefaultLimits(), + DefaultParams(proc_.get()), + NULL)); } - virtual void CreateResolverWithLimitsAndParams( - const PrioritizedDispatcher::Limits& limits, - const HostResolverImpl::ProcTaskParams& params) { - resolver_.reset(new HostResolverImpl(HostCache::CreateDefaultCache(), - limits, params, NULL)); + // This HostResolverImpl will only allow 1 outstanding resolve at a time and + // perform no retries. + void CreateSerialResolver() { + HostResolverImpl::ProcTaskParams params = DefaultParams(proc_.get()); + params.max_retry_attempts = 0u; + PrioritizedDispatcher::Limits limits(NUM_PRIORITIES, 1); + resolver_.reset(new HostResolverImpl( + HostCache::CreateDefaultCache(), + limits, + params, + NULL)); } // The Request will not be made until a call to |Resolve()|, and the Job will @@ -509,15 +496,25 @@ class HostResolverImplTest : public testing::Test { return CreateRequest(hostname, kDefaultPort); } + virtual void SetUp() OVERRIDE { + CreateResolver(); + } + + virtual void TearDown() OVERRIDE { + if (resolver_.get()) + EXPECT_EQ(0u, resolver_->num_running_jobs_for_tests()); + EXPECT_FALSE(proc_->HasBlockedRequests()); + } + void set_handler(Handler* handler) { handler_.reset(handler); handler_->test = this; } // Friendship is not inherited, so use proxies to access those. - size_t num_running_dispatcher_jobs() const { + size_t num_running_jobs() const { DCHECK(resolver_.get()); - return resolver_->num_running_dispatcher_jobs_for_tests(); + return resolver_->num_running_jobs_for_tests(); } void set_fallback_to_proctask(bool fallback_to_proctask) { @@ -901,7 +898,7 @@ TEST_F(HostResolverImplTest, ObeyPoolConstraintsAfterIPAddressChange) { EXPECT_EQ(ERR_NETWORK_CHANGED, requests_[0]->WaitForResult()); - EXPECT_EQ(1u, num_running_dispatcher_jobs()); + EXPECT_EQ(1u, num_running_jobs()); EXPECT_FALSE(requests_[1]->completed()); EXPECT_FALSE(requests_[2]->completed()); @@ -1268,63 +1265,36 @@ DnsConfig CreateValidDnsConfig() { // Specialized fixture for tests of DnsTask. class HostResolverImplDnsTest : public HostResolverImplTest { - public: - HostResolverImplDnsTest() : dns_client_(NULL) {} - protected: - // testing::Test implementation: virtual void SetUp() OVERRIDE { - AddDnsRule("nx", dns_protocol::kTypeA, MockDnsClientRule::FAIL, false); - AddDnsRule("nx", dns_protocol::kTypeAAAA, MockDnsClientRule::FAIL, false); - AddDnsRule("ok", dns_protocol::kTypeA, MockDnsClientRule::OK, false); - AddDnsRule("ok", dns_protocol::kTypeAAAA, MockDnsClientRule::OK, false); - AddDnsRule("4ok", dns_protocol::kTypeA, MockDnsClientRule::OK, false); - AddDnsRule("4ok", dns_protocol::kTypeAAAA, MockDnsClientRule::EMPTY, false); - AddDnsRule("6ok", dns_protocol::kTypeA, MockDnsClientRule::EMPTY, false); - AddDnsRule("6ok", dns_protocol::kTypeAAAA, MockDnsClientRule::OK, false); - AddDnsRule("4nx", dns_protocol::kTypeA, MockDnsClientRule::OK, false); - AddDnsRule("4nx", dns_protocol::kTypeAAAA, MockDnsClientRule::FAIL, false); - - AddDnsRule("4slow_ok", dns_protocol::kTypeA, MockDnsClientRule::OK, true); - AddDnsRule("4slow_ok", dns_protocol::kTypeAAAA, MockDnsClientRule::OK, - false); - AddDnsRule("6slow_ok", dns_protocol::kTypeA, MockDnsClientRule::OK, false); - AddDnsRule("6slow_ok", dns_protocol::kTypeAAAA, MockDnsClientRule::OK, - true); - AddDnsRule("4slow_4ok", dns_protocol::kTypeA, MockDnsClientRule::OK, true); - AddDnsRule("4slow_4ok", dns_protocol::kTypeAAAA, MockDnsClientRule::EMPTY, - false); - AddDnsRule("4slow_4timeout", dns_protocol::kTypeA, - MockDnsClientRule::TIMEOUT, true); - AddDnsRule("4slow_4timeout", dns_protocol::kTypeAAAA, MockDnsClientRule::OK, - false); - AddDnsRule("4slow_6timeout", dns_protocol::kTypeA, - MockDnsClientRule::OK, true); - AddDnsRule("4slow_6timeout", dns_protocol::kTypeAAAA, - MockDnsClientRule::TIMEOUT, false); + AddDnsRule("nx", dns_protocol::kTypeA, MockDnsClientRule::FAIL); + AddDnsRule("nx", dns_protocol::kTypeAAAA, MockDnsClientRule::FAIL); + AddDnsRule("ok", dns_protocol::kTypeA, MockDnsClientRule::OK); + AddDnsRule("ok", dns_protocol::kTypeAAAA, MockDnsClientRule::OK); + AddDnsRule("4ok", dns_protocol::kTypeA, MockDnsClientRule::OK); + AddDnsRule("4ok", dns_protocol::kTypeAAAA, MockDnsClientRule::EMPTY); + AddDnsRule("6ok", dns_protocol::kTypeA, MockDnsClientRule::EMPTY); + AddDnsRule("6ok", dns_protocol::kTypeAAAA, MockDnsClientRule::OK); + AddDnsRule("4nx", dns_protocol::kTypeA, MockDnsClientRule::OK); + AddDnsRule("4nx", dns_protocol::kTypeAAAA, MockDnsClientRule::FAIL); CreateResolver(); } - // HostResolverImplTest implementation: - virtual void CreateResolverWithLimitsAndParams( - const PrioritizedDispatcher::Limits& limits, - const HostResolverImpl::ProcTaskParams& params) OVERRIDE { + void CreateResolver() { resolver_.reset(new HostResolverImpl(HostCache::CreateDefaultCache(), - limits, - params, + DefaultLimits(), + DefaultParams(proc_.get()), NULL)); // Disable IPv6 support probing. resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED); - dns_client_ = new MockDnsClient(DnsConfig(), dns_rules_); - resolver_->SetDnsClient(scoped_ptr<DnsClient>(dns_client_)); + resolver_->SetDnsClient(CreateMockDnsClient(DnsConfig(), dns_rules_)); } // Adds a rule to |dns_rules_|. Must be followed by |CreateResolver| to apply. void AddDnsRule(const std::string& prefix, uint16 qtype, - MockDnsClientRule::Result result, - bool delay) { - dns_rules_.push_back(MockDnsClientRule(prefix, qtype, result, delay)); + MockDnsClientRule::Result result) { + dns_rules_.push_back(MockDnsClientRule(prefix, qtype, result)); } void ChangeDnsConfig(const DnsConfig& config) { @@ -1334,8 +1304,6 @@ class HostResolverImplDnsTest : public HostResolverImplTest { } MockDnsClientRuleList dns_rules_; - // Owned by |resolver_|. - MockDnsClient* dns_client_; }; // TODO(szym): Test AbortAllInProgressJobs due to DnsConfig change. @@ -1403,8 +1371,7 @@ TEST_F(HostResolverImplDnsTest, NoFallbackToProcTask) { // Simulate the case when the preference or policy has disabled the DNS client // causing AbortDnsTasks. - resolver_->SetDnsClient( - scoped_ptr<DnsClient>(new MockDnsClient(DnsConfig(), dns_rules_))); + resolver_->SetDnsClient(CreateMockDnsClient(DnsConfig(), dns_rules_)); ChangeDnsConfig(CreateValidDnsConfig()); // First request is resolved by MockDnsClient, others should fail due to @@ -1628,8 +1595,7 @@ TEST_F(HostResolverImplDnsTest, DualFamilyLocalhost) { DefaultLimits(), DefaultParams(proc.get()), NULL)); - resolver_->SetDnsClient( - scoped_ptr<DnsClient>(new MockDnsClient(DnsConfig(), dns_rules_))); + resolver_->SetDnsClient(CreateMockDnsClient(DnsConfig(), dns_rules_)); resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_IPV4); // Get the expected output. @@ -1682,186 +1648,4 @@ TEST_F(HostResolverImplDnsTest, DualFamilyLocalhost) { EXPECT_EQ(saw_ipv6, req->HasAddress("::1", 80)); } -// Cancel a request with a single DNS transaction active. -TEST_F(HostResolverImplDnsTest, CancelWithOneTransactionActive) { - resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_IPV4); - ChangeDnsConfig(CreateValidDnsConfig()); - - EXPECT_EQ(ERR_IO_PENDING, CreateRequest("ok", 80)->Resolve()); - EXPECT_EQ(1u, num_running_dispatcher_jobs()); - requests_[0]->Cancel(); - - // Dispatcher state checked in TearDown. -} - -// Cancel a request with a single DNS transaction active and another pending. -TEST_F(HostResolverImplDnsTest, CancelWithOneTransactionActiveOnePending) { - CreateSerialResolver(); - resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED); - ChangeDnsConfig(CreateValidDnsConfig()); - - EXPECT_EQ(ERR_IO_PENDING, CreateRequest("ok", 80)->Resolve()); - EXPECT_EQ(1u, num_running_dispatcher_jobs()); - requests_[0]->Cancel(); - - // Dispatcher state checked in TearDown. -} - -// Cancel a request with two DNS transactions active. -TEST_F(HostResolverImplDnsTest, CancelWithTwoTransactionsActive) { - resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED); - ChangeDnsConfig(CreateValidDnsConfig()); - - EXPECT_EQ(ERR_IO_PENDING, CreateRequest("ok", 80)->Resolve()); - EXPECT_EQ(2u, num_running_dispatcher_jobs()); - requests_[0]->Cancel(); - - // Dispatcher state checked in TearDown. -} - -// Delete a resolver with some active requests and some queued requests. -TEST_F(HostResolverImplDnsTest, DeleteWithActiveTransactions) { - // At most 10 Jobs active at once. - CreateResolverWithLimitsAndParams( - PrioritizedDispatcher::Limits(NUM_PRIORITIES, 10u), - DefaultParams(proc_.get())); - - resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED); - ChangeDnsConfig(CreateValidDnsConfig()); - - // First active job is an IPv4 request. - EXPECT_EQ(ERR_IO_PENDING, CreateRequest("ok", 80, MEDIUM, - ADDRESS_FAMILY_IPV4)->Resolve()); - - // Add 10 more DNS lookups for different hostnames. First 4 should have two - // active jobs, next one has a single active job, and one pending. Others - // should all be queued. - for (int i = 0; i < 10; ++i) { - EXPECT_EQ(ERR_IO_PENDING, CreateRequest( - base::StringPrintf("ok%i", i))->Resolve()); - } - EXPECT_EQ(10u, num_running_dispatcher_jobs()); - - resolver_.reset(); -} - -// Cancel a request with only the IPv6 transaction active. -TEST_F(HostResolverImplDnsTest, CancelWithIPv6TransactionActive) { - resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED); - ChangeDnsConfig(CreateValidDnsConfig()); - - EXPECT_EQ(ERR_IO_PENDING, CreateRequest("6slow_ok", 80)->Resolve()); - EXPECT_EQ(2u, num_running_dispatcher_jobs()); - - // The IPv4 request should complete, the IPv6 request is still pending. - base::RunLoop().RunUntilIdle(); - EXPECT_EQ(1u, num_running_dispatcher_jobs()); - requests_[0]->Cancel(); - - // Dispatcher state checked in TearDown. -} - -// Cancel a request with only the IPv4 transaction pending. -TEST_F(HostResolverImplDnsTest, CancelWithIPv4TransactionPending) { - set_fallback_to_proctask(false); - resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED); - ChangeDnsConfig(CreateValidDnsConfig()); - - EXPECT_EQ(ERR_IO_PENDING, CreateRequest("4slow_ok", 80)->Resolve()); - EXPECT_EQ(2u, num_running_dispatcher_jobs()); - - // The IPv6 request should complete, the IPv4 request is still pending. - base::RunLoop().RunUntilIdle(); - EXPECT_EQ(1u, num_running_dispatcher_jobs()); - - requests_[0]->Cancel(); -} - -// Test cases where AAAA completes first. -TEST_F(HostResolverImplDnsTest, AAAACompletesFirst) { - set_fallback_to_proctask(false); - resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED); - ChangeDnsConfig(CreateValidDnsConfig()); - - EXPECT_EQ(ERR_IO_PENDING, CreateRequest("4slow_ok", 80)->Resolve()); - EXPECT_EQ(ERR_IO_PENDING, CreateRequest("4slow_4ok", 80)->Resolve()); - EXPECT_EQ(ERR_IO_PENDING, CreateRequest("4slow_4timeout", 80)->Resolve()); - EXPECT_EQ(ERR_IO_PENDING, CreateRequest("4slow_6timeout", 80)->Resolve()); - - base::RunLoop().RunUntilIdle(); - EXPECT_FALSE(requests_[0]->completed()); - EXPECT_FALSE(requests_[1]->completed()); - EXPECT_FALSE(requests_[2]->completed()); - // The IPv6 of the third request should have failed and resulted in cancelling - // the IPv4 request. - EXPECT_TRUE(requests_[3]->completed()); - EXPECT_EQ(ERR_DNS_TIMED_OUT, requests_[3]->result()); - EXPECT_EQ(3u, num_running_dispatcher_jobs()); - - dns_client_->CompleteDelayedTransactions(); - EXPECT_TRUE(requests_[0]->completed()); - EXPECT_EQ(OK, requests_[0]->result()); - EXPECT_EQ(2u, requests_[0]->NumberOfAddresses()); - EXPECT_TRUE(requests_[0]->HasAddress("127.0.0.1", 80)); - EXPECT_TRUE(requests_[0]->HasAddress("::1", 80)); - - EXPECT_TRUE(requests_[1]->completed()); - EXPECT_EQ(OK, requests_[1]->result()); - EXPECT_EQ(1u, requests_[1]->NumberOfAddresses()); - EXPECT_TRUE(requests_[1]->HasAddress("127.0.0.1", 80)); - - EXPECT_TRUE(requests_[2]->completed()); - EXPECT_EQ(ERR_DNS_TIMED_OUT, requests_[2]->result()); -} - -// Test the case where only a single transaction slot is available. -TEST_F(HostResolverImplDnsTest, SerialResolver) { - CreateSerialResolver(); - set_fallback_to_proctask(false); - resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED); - ChangeDnsConfig(CreateValidDnsConfig()); - - EXPECT_EQ(ERR_IO_PENDING, CreateRequest("ok", 80)->Resolve()); - EXPECT_EQ(1u, num_running_dispatcher_jobs()); - - base::RunLoop().RunUntilIdle(); - EXPECT_TRUE(requests_[0]->completed()); - EXPECT_EQ(OK, requests_[0]->result()); - EXPECT_EQ(2u, requests_[0]->NumberOfAddresses()); - EXPECT_TRUE(requests_[0]->HasAddress("127.0.0.1", 80)); - EXPECT_TRUE(requests_[0]->HasAddress("::1", 80)); -} - -// Test the case where the AAAA query is started when another transaction -// completes. -TEST_F(HostResolverImplDnsTest, AAAAStartsAfterOtherJobFinishes) { - CreateResolverWithLimitsAndParams( - PrioritizedDispatcher::Limits(NUM_PRIORITIES, 2), - DefaultParams(proc_.get())); - set_fallback_to_proctask(false); - resolver_->SetDefaultAddressFamily(ADDRESS_FAMILY_UNSPECIFIED); - ChangeDnsConfig(CreateValidDnsConfig()); - - EXPECT_EQ(ERR_IO_PENDING, CreateRequest("ok", 80, MEDIUM, - ADDRESS_FAMILY_IPV4)->Resolve()); - EXPECT_EQ(ERR_IO_PENDING, - CreateRequest("4slow_ok", 80, MEDIUM)->Resolve()); - // An IPv4 request should have been started pending for each job. - EXPECT_EQ(2u, num_running_dispatcher_jobs()); - - // Request 0's IPv4 request should complete, starting Request 1's IPv6 - // request, which should also complete. - base::RunLoop().RunUntilIdle(); - EXPECT_EQ(1u, num_running_dispatcher_jobs()); - EXPECT_TRUE(requests_[0]->completed()); - EXPECT_FALSE(requests_[1]->completed()); - - dns_client_->CompleteDelayedTransactions(); - EXPECT_TRUE(requests_[1]->completed()); - EXPECT_EQ(OK, requests_[1]->result()); - EXPECT_EQ(2u, requests_[1]->NumberOfAddresses()); - EXPECT_TRUE(requests_[1]->HasAddress("127.0.0.1", 80)); - EXPECT_TRUE(requests_[1]->HasAddress("::1", 80)); -} - } // namespace net |