[net/dns] Perform A/AAAA queries for AF_UNSPEC resolutions in parallel.

net/dns/host_resolver_impl.cc

 // Copyright (c) 2012 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.
 
 #include "net/dns/host_resolver_impl.h"
 
 #if defined(OS_WIN)
 #include <Winsock2.h>
 #elif defined(OS_POSIX)
 #include <netdb.h>
@@ skipping 952 matching lines @@
 
   DISALLOW_COPY_AND_ASSIGN(LoopbackProbeJob);
 };
 
 //-----------------------------------------------------------------------------
 
 // Resolves the hostname using DnsTransaction.
 // TODO(szym): This could be moved to separate source file as well.
 class HostResolverImpl::DnsTask : public base::SupportsWeakPtr<DnsTask> {
  public:
-  typedef base::Callback<void(int net_error,
-                              const AddressList& addr_list,
-                              base::TimeDelta ttl)> Callback;
+  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() {}
+  };
 
   DnsTask(DnsClient* client,
           const Key& key,
-          const Callback& callback,
+          Delegate* delegate,
           const BoundNetLog& job_net_log)
       : client_(client),
-        family_(key.address_family),
-        callback_(callback),
-        net_log_(job_net_log) {
+        key_(key),
+        delegate_(delegate),
+        net_log_(job_net_log),
+        num_completed_transactions_(0),
+        task_start_time_(base::TimeTicks::Now()) {
     DCHECK(client);
-    DCHECK(!callback.is_null());
+    DCHECK(delegate_);
+  }
 
-    // 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,
+  bool needs_two_transactions() const {
+    return key_.address_family == ADDRESS_FAMILY_UNSPECIFIED;
+  }
+
+  bool needs_another_transaction() const {
+    return needs_two_transactions() && !transaction_aaaa_;
+  }
+
+  void StartFirstTransaction() {
+    DCHECK_EQ(0u, num_completed_transactions_);
+    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();
+  }
+
+ 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),
-                   true /* first_query */, base::TimeTicks::Now()),
+                   base::TimeTicks::Now()),
         net_log_);
   }
 
-  void Start() {
-    net_log_.BeginEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK);
-    transaction_->Start();
-  }
-
- private:
-  void OnTransactionComplete(bool first_query,
-                             const base::TimeTicks& start_time,
+  void OnTransactionComplete(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:
         DNS_HISTOGRAM("AsyncDNS.TransactionSuccess_A", duration);
         break;
       case dns_protocol::kTypeAAAA:
         DNS_HISTOGRAM("AsyncDNS.TransactionSuccess_AAAA", duration);
         break;
     }
+
     AddressList addr_list;
     base::TimeDelta ttl;
     DnsResponse::Result result = response->ParseToAddressList(&addr_list, &ttl);
     UMA_HISTOGRAM_ENUMERATION("AsyncDNS.ParseToAddressList",
                               result,
                               DnsResponse::DNS_PARSE_RESULT_MAX);
     if (result != DnsResponse::DNS_PARSE_OK) {
       // Fail even if the other query succeeds.
       OnFailure(ERR_DNS_MALFORMED_RESPONSE, result);
       return;
     }
 
-    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;
-      }
+    ++num_completed_transactions_;
+    if (num_completed_transactions_ == 1) {
+      ttl_ = ttl;
     } else {
-      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);
+      ttl_ = std::min(ttl_, ttl);
     }
 
-    if (addr_list.empty()) {
-      // TODO(szym): Don't fallback to ProcTask in this case.
-      OnFailure(ERR_NAME_NOT_RESOLVED, DnsResponse::DNS_PARSE_OK);

[szym, 2013/08/22 15:50:07]

Oh oh... Looks like we missed handling of this case leading to
http://crbug.com/277625
I thought we had a test covering this case.

How about:
proc_->AddRuleForAllFamilies("6ok_fallback", "192.168.0.1");
CreateRequest("6ok_fallback", 80, MEDIUM, ADDRESS_FAMILY_IPV4)->Resolve();
EXPECT_EQ(OK, requests_[0]->WaitForResult());
EXPECT_TRUE(requests_[0]->HasOneAddress("192.168.0.1", 80));

Could also consider adding a MockDnsRule for "empty" which just returns an empty
response.

+    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());
+    } else {
+      DCHECK_EQ(transaction_aaaa_.get(), transaction);
+      // Place IPv6 addresses before IPv4.
+      addr_list_.insert(addr_list_.begin(), addr_list.begin(), addr_list.end());
+    }
+
+    if (needs_two_transactions() && num_completed_transactions_ == 1) {
+      // No need to repeat the suffix search.
+      key_.hostname = transaction->GetHostname();
+      delegate_->OnFirstDnsTransactionComplete();
       return;
     }
 

[szym, 2013/08/22 15:50:07]

if (addr_list.empty()) {
  OnFailure(ERR_NAME_NOT_RESOLVED, DnsResponse::DNS_PARSE_OK);
  return;
}

-    if (needs_sort) {
-      // Sort could complete synchronously.
+    // 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.
       client_->GetAddressSorter()->Sort(
-          addr_list,
+          addr_list_,
           base::Bind(&DnsTask::OnSortComplete,
                      AsWeakPtr(),
-                     base::TimeTicks::Now(),
-                     ttl));
+                     base::TimeTicks::Now()));
     } else {
-      OnSuccess(addr_list, ttl);
+      OnSuccess(addr_list_);
     }
   }
 
   void OnSortComplete(base::TimeTicks start_time,
-                      base::TimeDelta ttl,
                       bool success,
                       const AddressList& addr_list) {
     if (!success) {
       DNS_HISTOGRAM("AsyncDNS.SortFailure",
                     base::TimeTicks::Now() - start_time);
       OnFailure(ERR_DNS_SORT_ERROR, DnsResponse::DNS_PARSE_OK);
       return;
     }
 
     DNS_HISTOGRAM("AsyncDNS.SortSuccess",
                   base::TimeTicks::Now() - start_time);
 
     // AddressSorter prunes unusable destinations.
     if (addr_list.empty()) {
       LOG(WARNING) << "Address list empty after RFC3484 sort";
       OnFailure(ERR_NAME_NOT_RESOLVED, DnsResponse::DNS_PARSE_OK);
       return;
     }
 
-    OnSuccess(addr_list, ttl);
+    OnSuccess(addr_list);
   }
 
   void OnFailure(int net_error, DnsResponse::Result result) {
     DCHECK_NE(OK, net_error);
     net_log_.EndEvent(
         NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK,
         base::Bind(&NetLogDnsTaskFailedCallback, net_error, result));
-    callback_.Run(net_error, AddressList(), base::TimeDelta());
+    delegate_->OnDnsTaskComplete(task_start_time_, net_error, AddressList(),
+                                 base::TimeDelta());
   }
 
-  void OnSuccess(const AddressList& addr_list, base::TimeDelta ttl) {
+  void OnSuccess(const AddressList& addr_list) {
     net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK,
                       addr_list.CreateNetLogCallback());
-    callback_.Run(OK, addr_list, ttl);
+    delegate_->OnDnsTaskComplete(task_start_time_, OK, addr_list, ttl_);
   }
 
   DnsClient* client_;
-  AddressFamily family_;
+  Key key_;
+
   // The listener to the results of this DnsTask.
-  Callback callback_;
+  Delegate* delegate_;
   const BoundNetLog net_log_;
 
-  scoped_ptr<DnsTransaction> transaction_;
+  scoped_ptr<DnsTransaction> transaction_a_;
+  scoped_ptr<DnsTransaction> transaction_aaaa_;
 
-  // Results from the first transaction. Used only if |family_| is unspecified.
-  AddressList first_addr_list_;
-  base::TimeDelta first_ttl_;
+  unsigned num_completed_transactions_;
+
+  // 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_;
 
   DISALLOW_COPY_AND_ASSIGN(DnsTask);
 };
 
 //-----------------------------------------------------------------------------
 
 // Aggregates all Requests for the same Key. Dispatched via PriorityDispatch.
-class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
+class HostResolverImpl::Job : public PrioritizedDispatcher::Job,
+                              public HostResolverImpl::DnsTask::Delegate {
  public:
   // Creates new job for |key| where |request_net_log| is bound to the
   // request that spawned it.
   Job(const base::WeakPtr<HostResolverImpl>& resolver,
       const Key& key,
       RequestPriority priority,
       const BoundNetLog& request_net_log)
       : resolver_(resolver),
         key_(key),
         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_),
         net_log_(BoundNetLog::Make(request_net_log.net_log(),
                                    NetLog::SOURCE_HOST_RESOLVER_IMPL_JOB)) {
     request_net_log.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_CREATE_JOB);
 
     net_log_.BeginEvent(
         NetLog::TYPE_HOST_RESOLVER_IMPL_JOB,
         base::Bind(&NetLogJobCreationCallback,
                    request_net_log.source(),
                    &key_.hostname));
   }
 
   virtual ~Job() {
     if (is_running()) {
       // |resolver_| was destroyed with this Job still in flight.
       // Clean-up, record in the log, but don't run any callbacks.
       if (is_proc_running()) {
         proc_task_->Cancel();
         proc_task_ = NULL;
       }
       // Clean up now for nice NetLog.
-      dns_task_.reset(NULL);
+      KillDnsTask();
       net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB,
                                         ERR_ABORTED);
     } else if (is_queued()) {
       // |resolver_| was destroyed without running this Job.
       // TODO(szym): is there any benefit in having this distinction?
       net_log_.AddEvent(NetLog::TYPE_CANCELLED);
       net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB);
     }
     // else CompleteRequests logged EndEvent.
 
     // Log any remaining Requests as cancelled.
     for (RequestsList::const_iterator it = requests_.begin();
          it != requests_.end(); ++it) {
       Request* req = *it;
       if (req->was_canceled())
         continue;
       DCHECK_EQ(this, req->job());
       LogCancelRequest(req->source_net_log(), req->request_net_log(),
                        req->info());
     }
   }
 
-  // Add this job to the dispatcher.
-  void Schedule() {
-    handle_ = resolver_->dispatcher_.Add(this, priority());
+  // 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;
+    }
   }
 
   void AddRequest(scoped_ptr<Request> req) {
     DCHECK_EQ(key_.hostname, req->info().hostname());
 
     req->set_job(this);
     priority_tracker_.Add(req->priority());
 
     req->request_net_log().AddEvent(
         NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_ATTACH,
@@ skipping 47 matching lines @@
   // Called from AbortAllInProgressJobs. Completes all requests and destroys
   // the job. This currently assumes the abort is due to a network change.
   void Abort() {
     DCHECK(is_running());
     CompleteRequestsWithError(ERR_NETWORK_CHANGED);
   }
 
   // If DnsTask present, abort it and fall back to ProcTask.
   void AbortDnsTask() {
     if (dns_task_) {
-      dns_task_.reset();
+      KillDnsTask();
       dns_task_error_ = OK;
       StartProcTask();
     }
   }
 
   // Called by HostResolverImpl when this job is evicted due to queue overflow.
   // Completes all requests and destroys the job.
   void OnEvicted() {
     DCHECK(!is_running());
     DCHECK(is_queued());
@@ skipping 28 matching lines @@
 
   bool is_queued() const {
     return !handle_.is_null();
   }
 
   bool is_running() const {
     return is_dns_running() || is_proc_running();
   }
 
  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()))
         priority_change_time_ = base::TimeTicks::Now();
       handle_ = resolver_->dispatcher_.ChangePriority(handle_, priority());
     }
   }
 
   AddressList MakeAddressListForRequest(const AddressList& list) const {
     if (requests_.empty())
       return list;
     return AddressList::CopyWithPort(list, requests_.front()->info().port());
   }
 
   // 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);
 
     had_dns_config_ = resolver_->HaveDnsConfig();
 
     base::TimeTicks now = base::TimeTicks::Now();
     base::TimeDelta queue_time = now - creation_time_;
     base::TimeDelta queue_time_after_change = now - priority_change_time_;
 
     if (had_dns_config_) {
@@ skipping 83 matching lines @@
       ttl = base::TimeDelta::FromSeconds(kCacheEntryTTLSeconds);
 
     // Don't store the |ttl| in cache since it's not obtained from the server.
     CompleteRequests(
         HostCache::Entry(net_error, MakeAddressListForRequest(addr_list)),
         ttl);
   }
 
   void StartDnsTask() {
     DCHECK(resolver_->HaveDnsConfig());
-    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_));
+    dns_task_.reset(new DnsTask(resolver_->dns_client_.get(), key_, this,
+                                net_log_));
 
-    dns_task_->Start();
+    dns_task_->StartFirstTransaction();
+    // Schedule a second transaction, if needed.
+    if (dns_task_->needs_two_transactions())
+      Schedule(true);
+  }
+
+  void StartSecondDnsTransaction() {
+    DCHECK(dns_task_->needs_two_transactions());
+    dns_task_->StartSecondTransaction();
   }
 
   // Called if DnsTask fails. It is posted from StartDnsTask, so Job may be
   // deleted before this callback. In this case dns_task is deleted as well,
   // so we use it as indicator whether Job is still valid.
   void OnDnsTaskFailure(const base::WeakPtr<DnsTask>& dns_task,
                         base::TimeDelta duration,
                         int net_error) {
     DNS_HISTOGRAM("AsyncDNS.ResolveFail", duration);
 
     if (dns_task == NULL)
       return;
 
     dns_task_error_ = net_error;
 
     // TODO(szym): Run ServeFromHosts now if nsswitch.conf says so.
     // http://crbug.com/117655
 
     // TODO(szym): Some net errors indicate lack of connectivity. Starting
     // ProcTask in that case is a waste of time.
     if (resolver_->fallback_to_proctask_) {
-      dns_task_.reset();
+      KillDnsTask();
       StartProcTask();
     } else {
       UmaAsyncDnsResolveStatus(RESOLVE_STATUS_FAIL);
       CompleteRequestsWithError(net_error);
     }
   }
 
-  // Called by DnsTask when it completes.
-  void OnDnsTaskComplete(base::TimeTicks start_time,
-                         int net_error,
-                         const AddressList& addr_list,
-                         base::TimeDelta ttl) {
+
+  // HostResolverImpl::DnsTask::Delegate implementation:
+
+  virtual void OnDnsTaskComplete(base::TimeTicks start_time,
+                                 int net_error,
+                                 const AddressList& addr_list,
+                                 base::TimeDelta ttl) OVERRIDE {
     DCHECK(is_dns_running());
 
     base::TimeDelta duration = base::TimeTicks::Now() - start_time;
     if (net_error != OK) {
       OnDnsTaskFailure(dns_task_->AsWeakPtr(), duration, net_error);
       return;
     }
     DNS_HISTOGRAM("AsyncDNS.ResolveSuccess", duration);
     // Log DNS lookups based on |address_family|.
     switch(key_.address_family) {
@@ skipping 14 matching lines @@
     resolver_->OnDnsTaskResolve(OK);
 
     base::TimeDelta bounded_ttl =
         std::max(ttl, base::TimeDelta::FromSeconds(kMinimumTTLSeconds));
 
     CompleteRequests(
         HostCache::Entry(net_error, MakeAddressListForRequest(addr_list), ttl),
         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) {
     CHECK(resolver_.get());
 
     // This job must be removed from resolver's |jobs_| now to make room for a
     // new job with the same key in case one of the OnComplete callbacks decides
     // to spawn one. Consequently, the job deletes itself when CompleteRequests
     // is done.
     scoped_ptr<Job> self_deleter(this);
 
     resolver_->RemoveJob(this);
 
     if (is_running()) {
-      DCHECK(!is_queued());
       if (is_proc_running()) {
+        DCHECK(!is_queued());
         proc_task_->Cancel();
         proc_task_ = NULL;
       }
-      dns_task_.reset();
+      KillDnsTask();
 
       // Signal dispatcher that a slot has opened.
       resolver_->dispatcher_.OnJobFinished();
     } else if (is_queued()) {
       resolver_->dispatcher_.Cancel(handle_);
       handle_.Reset();
     }
 
     if (num_active_requests() == 0) {
       net_log_.AddEvent(NetLog::TYPE_CANCELLED);
@@ skipping 73 matching lines @@
   Key key_;
 
   // Tracks the highest priority across |requests_|.
   PriorityTracker priority_tracker_;
 
   bool had_non_speculative_request_;
 
   // 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_;
 
   const base::TimeTicks creation_time_;
   base::TimeTicks priority_change_time_;
 
   BoundNetLog net_log_;
 
   // Resolves the host using a HostResolverProc.
   scoped_refptr<ProcTask> proc_task_;
@@ skipping 67 matching lines @@
   {
     DnsConfig dns_config;
     NetworkChangeNotifier::GetDnsConfig(&dns_config);
     received_dns_config_ = dns_config.IsValid();
   }
 
   fallback_to_proctask_ = !ConfigureAsyncDnsNoFallbackFieldTrial();
 }
 
 HostResolverImpl::~HostResolverImpl() {
-  // This will also cancel all outstanding requests.
+  // 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.
   STLDeleteValues(&jobs_);
 
   NetworkChangeNotifier::RemoveIPAddressObserver(this);
   NetworkChangeNotifier::RemoveDNSObserver(this);
 }
 
 void HostResolverImpl::SetMaxQueuedJobs(size_t value) {
   DCHECK_EQ(0u, dispatcher_.num_queued_jobs());
   DCHECK_GT(value, 0u);
   max_queued_jobs_ = value;
@@ skipping 32 matching lines @@
   }
 
   // Next we need to attach our request to a "job". This job is responsible for
   // calling "getaddrinfo(hostname)" on a worker thread.
 
   JobMap::iterator jobit = jobs_.find(key);
   Job* job;
   if (jobit == jobs_.end()) {
     job =
         new Job(weak_ptr_factory_.GetWeakPtr(), key, priority, request_net_log);
-    job->Schedule();
+    job->Schedule(false);
 
     // Check for queue overflow.
     if (dispatcher_.num_queued_jobs() > max_queued_jobs_) {
       Job* evicted = static_cast<Job*>(dispatcher_.EvictOldestLowest());
       DCHECK(evicted);
       evicted->OnEvicted();  // Deletes |evicted|.
       if (evicted == job) {
         rv = ERR_HOST_RESOLVER_QUEUE_TOO_LARGE;
         LogFinishRequest(source_net_log, request_net_log, info, rv);
         return rv;
@@ skipping 276 matching lines @@
     Job* job = it->second;
     if (job->is_running()) {
       jobs_to_abort.push_back(job);
       jobs_.erase(it++);
     } else {
       DCHECK(job->is_queued());
       ++it;
     }
   }
 
-  // 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();
 
   // Then Abort them.
   for (size_t i = 0; self.get() && i < jobs_to_abort.size(); ++i) {
     jobs_to_abort[i]->Abort();
     jobs_to_abort[i] = NULL;
   }
 }
 
@@ skipping 111 matching lines @@
   }
   DnsConfig dns_config;
   NetworkChangeNotifier::GetDnsConfig(&dns_config);
   dns_client_->SetConfig(dns_config);
   num_dns_failures_ = 0;
   if (dns_config.IsValid())
     UMA_HISTOGRAM_BOOLEAN("AsyncDNS.DnsClientEnabled", true);
 }
 
 }  // namespace net