Change HostResolver::Resolve() to take an std::unique_ptr<Request>* rather than a RequestHandle*

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>
 #endif
 
 #include <cmath>
 #include <memory>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
+#include "base/callback_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/debug/debugger.h"
 #include "base/debug/leak_annotations.h"
 #include "base/debug/stack_trace.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/profiler/scoped_tracker.h"
@@ skipping 487 matching lines @@
   }
 
   return true;
 }
 
 const unsigned HostResolverImpl::kMaximumDnsFailures = 16;
 
 // Holds the data for a request that could not be completed synchronously.
 // It is owned by a Job. Canceled Requests are only marked as canceled rather
 // than removed from the Job's |requests_| list.
-class HostResolverImpl::Request {
+class HostResolverImpl::RequestImpl : public HostResolver::Request {
  public:
-  Request(const BoundNetLog& source_net_log,
-          const RequestInfo& info,
-          RequestPriority priority,
-          const CompletionCallback& callback,
-          AddressList* addresses)
+  RequestImpl(const BoundNetLog& source_net_log,
+              const RequestInfo& info,
+              RequestPriority priority,
+              const CompletionCallback& callback,
+              AddressList* addresses,
+              Job* job)
       : source_net_log_(source_net_log),
         info_(info),
         priority_(priority),
-        job_(nullptr),
+        job_(job),
         callback_(callback),
         addresses_(addresses),
         request_time_(base::TimeTicks::Now()) {}
 
-  // Mark the request as canceled.
-  void MarkAsCanceled() {
+  ~RequestImpl() override;
+
+  void ChangeRequestPriority(RequestPriority priority) override;
+
+  void OnJobCancelled(Job* job) {
+    DCHECK_EQ(job_, job);
     job_ = nullptr;
     addresses_ = nullptr;
     callback_.Reset();
   }
 
-  bool was_canceled() const {
-    return callback_.is_null();
-  }
-
-  void set_job(Job* job) {
-    DCHECK(job);
-    // Identify which job the request is waiting on.
-    job_ = job;
-  }
-
   // Prepare final AddressList and call completion callback.
-  void OnComplete(int error, const AddressList& addr_list) {
-    DCHECK(!was_canceled());
+  void OnJobCompleted(Job* job, int error, const AddressList& addr_list) {
+    DCHECK_EQ(job_, job);
     if (error == OK)
       *addresses_ = EnsurePortOnAddressList(addr_list, info_.port());
-    CompletionCallback callback = callback_;
-    MarkAsCanceled();
-    callback.Run(error);
+    job_ = nullptr;
+    addresses_ = nullptr;
+    base::ResetAndReturn(&callback_).Run(error);
   }
 
   Job* job() const {
     return job_;
   }
 
   // NetLog for the source, passed in HostResolver::Resolve.
   const BoundNetLog& source_net_log() {
     return source_net_log_;
   }
@@ skipping 19 matching lines @@
   Job* job_;
 
   // The user's callback to invoke when the request completes.
   CompletionCallback callback_;
 
   // The address list to save result into.
   AddressList* addresses_;
 
   const base::TimeTicks request_time_;
 
-  DISALLOW_COPY_AND_ASSIGN(Request);
+  DISALLOW_COPY_AND_ASSIGN(RequestImpl);
 };
 
 //------------------------------------------------------------------------------
 
 // Calls HostResolverProc using a worker task runner. Performs retries if
 // necessary.
 //
 // Whenever we try to resolve the host, we post a delayed task to check if host
 // resolution (OnLookupComplete) is completed or not. If the original attempt
 // hasn't completed, then we start another attempt for host resolution. We take
@@ skipping 696 matching lines @@
       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 (const std::unique_ptr<Request>& req : requests_) {
-      if (req->was_canceled())
-        continue;
-      DCHECK_EQ(this, req->job());
-      LogCancelRequest(req->source_net_log(), req->info());
+    if (!requests_.empty()) {
+      // Log any remaining Requests as cancelled.
+      for (RequestImpl* req : requests_) {
+        DCHECK_EQ(this, req->job());
+        req->OnJobCancelled(this);
+        LogCancelRequest(req->source_net_log(), req->info());
+      }
+      requests_.clear();
     }
   }
 
   // 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(std::unique_ptr<Request> req) {
-    DCHECK_EQ(key_.hostname, req->info().hostname());
+  void AddRequest(RequestImpl* request) {
+    DCHECK_EQ(key_.hostname, request->info().hostname());
 
-    req->set_job(this);
-    priority_tracker_.Add(req->priority());
+    priority_tracker_.Add(request->priority());
 
-    req->source_net_log().AddEvent(
+    request->source_net_log().AddEvent(
         NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_ATTACH,
         net_log_.source().ToEventParametersCallback());
 
     net_log_.AddEvent(
         NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_REQUEST_ATTACH,
-        base::Bind(&NetLogJobAttachCallback,
-                   req->source_net_log().source(),
+        base::Bind(&NetLogJobAttachCallback, request->source_net_log().source(),
                    priority()));
 
     // TODO(szym): Check if this is still needed.
-    if (!req->info().is_speculative()) {
+    if (!request->info().is_speculative()) {
       had_non_speculative_request_ = true;
       if (proc_task_.get())
         proc_task_->set_had_non_speculative_request();
     }
 
-    requests_.push_back(std::move(req));
+    requests_.push_back(request);
 
     UpdatePriority();
   }
 
-  void ChangeRequestPriority(Request* req, RequestPriority priority) {
+  void ChangeRequestPriority(RequestImpl* req, RequestPriority priority) {
     DCHECK_EQ(key_.hostname, req->info().hostname());
-    DCHECK(!req->was_canceled());
 
     priority_tracker_.Remove(req->priority());
     req->set_priority(priority);
     priority_tracker_.Add(req->priority());
     UpdatePriority();
   }
 
-  // Marks |req| as cancelled. If it was the last active Request, also finishes
-  // this Job, marking it as cancelled, and deletes it.
-  void CancelRequest(Request* req) {
-    DCHECK_EQ(key_.hostname, req->info().hostname());
-    DCHECK(!req->was_canceled());
+  // Detach cancelled request. If it was the last active Request, also finishes
+  // this Job.
+  void CancelRequest(RequestImpl* request) {
+    DCHECK_EQ(key_.hostname, request->info().hostname());
+    DCHECK(!requests_.empty());
 
-    // Don't remove it from |requests_| just mark it canceled.
-    req->MarkAsCanceled();
-    LogCancelRequest(req->source_net_log(), req->info());
+    LogCancelRequest(request->source_net_log(), request->info());
 
-    priority_tracker_.Remove(req->priority());
-    net_log_.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_REQUEST_DETACH,
-                      base::Bind(&NetLogJobAttachCallback,
-                                 req->source_net_log().source(),
-                                 priority()));
+    priority_tracker_.Remove(request->priority());
+    net_log_.AddEvent(
+        NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_REQUEST_DETACH,
+        base::Bind(&NetLogJobAttachCallback, request->source_net_log().source(),
+                   priority()));
 
     if (num_active_requests() > 0) {
       UpdatePriority();
+      RemoveRequest(request);
     } else {
       // If we were called from a Request's callback within CompleteRequests,
       // that Request could not have been cancelled, so num_active_requests()
       // could not be 0. Therefore, we are not in CompleteRequests().
       CompleteRequestsWithError(OK /* cancelled */);
     }
   }
 
+  void RemoveRequest(RequestImpl* request) {
+    auto it = std::find(requests_.begin(), requests_.end(), request);
+    DCHECK(it != requests_.end());
+    requests_.erase(it);
+  }
+
   // Called from AbortAllInProgressJobs. Completes all requests and destroys
   // the job. This currently assumes the abort is due to a network change.
   // TODO This should not delete |this|.
   void Abort() {
     DCHECK(is_running());
     CompleteRequestsWithError(ERR_NETWORK_CHANGED);
   }
 
   // If DnsTask present, abort it and fall back to ProcTask.
   void AbortDnsTask() {
@@ skipping 340 matching lines @@
       // configuration.
       UMA_HISTOGRAM_BOOLEAN("AsyncDNS.HaveDnsConfig",
                             resolver_->received_dns_config_);
     }
 
     bool did_complete = (entry.error() != ERR_NETWORK_CHANGED) &&
                         (entry.error() != ERR_HOST_RESOLVER_QUEUE_TOO_LARGE);
     if (did_complete)
       resolver_->CacheResult(key_, entry, ttl);
 
-    // Complete all of the requests that were attached to the job.
-    for (const std::unique_ptr<Request>& req : requests_) {
-      if (req->was_canceled())
-        continue;
-
+    // Complete all of the requests that were attached to the job and
+    // detach them.
+    while (!requests_.empty()) {
+      RequestImpl* req = requests_.front();
+      requests_.pop_front();
       DCHECK_EQ(this, req->job());
       // Update the net log and notify registered observers.
       LogFinishRequest(req->source_net_log(), req->info(), entry.error());
       if (did_complete) {
         // Record effective total time from creation to completion.
         RecordTotalTime(had_dns_config_, req->info().is_speculative(),
                         base::TimeTicks::Now() - req->request_time());
       }
-      req->OnComplete(entry.error(), entry.addresses());
+      req->OnJobCompleted(this, entry.error(), entry.addresses());
 
       // Check if the resolver was destroyed as a result of running the
       // callback. If it was, we could continue, but we choose to bail.
       if (!resolver_.get())
         return;
     }
   }
 
   // Convenience wrapper for CompleteRequests in case of failure.
   void CompleteRequestsWithError(int net_error) {
@@ skipping 40 matching lines @@
 
   BoundNetLog net_log_;
 
   // Resolves the host using a HostResolverProc.
   scoped_refptr<ProcTask> proc_task_;
 
   // Resolves the host using a DnsTransaction.
   std::unique_ptr<DnsTask> dns_task_;
 
   // All Requests waiting for the result of this Job. Some can be canceled.
-  std::vector<std::unique_ptr<Request>> requests_;
+  std::deque<RequestImpl*> requests_;
 
   // A handle used in |HostResolverImpl::dispatcher_|.
   PrioritizedDispatcher::Handle handle_;
 };
 
 //-----------------------------------------------------------------------------
 
 HostResolverImpl::ProcTaskParams::ProcTaskParams(
     HostResolverProc* resolver_proc,
     size_t max_retry_attempts)
@@ skipping 34 matching lines @@
 void HostResolverImpl::SetMaxQueuedJobs(size_t value) {
   DCHECK_EQ(0u, dispatcher_->num_queued_jobs());
   DCHECK_GT(value, 0u);
   max_queued_jobs_ = value;
 }
 
 int HostResolverImpl::Resolve(const RequestInfo& info,
                               RequestPriority priority,
                               AddressList* addresses,
                               const CompletionCallback& callback,
-                              RequestHandle* out_req,
+                              std::unique_ptr<Request>* out_req,
                               const BoundNetLog& source_net_log) {
   DCHECK(addresses);
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(false, callback.is_null());
+  DCHECK(out_req);
 
   // Check that the caller supplied a valid hostname to resolve.
   std::string labeled_hostname;
   if (!DNSDomainFromDot(info.hostname(), &labeled_hostname))
     return ERR_NAME_NOT_RESOLVED;
 
   LogStartRequest(source_net_log, info);
 
   IPAddress ip_address;
   IPAddress* ip_address_ptr = nullptr;
@@ skipping 32 matching lines @@
         LogFinishRequest(source_net_log, info, rv);
         return rv;
       }
     }
     jobs_.insert(jobit, std::make_pair(key, job));
   } else {
     job = jobit->second;
   }
 
   // Can't complete synchronously. Create and attach request.
-  std::unique_ptr<Request> req(
-      new Request(source_net_log, info, priority, callback, addresses));
-  if (out_req)
-    *out_req = reinterpret_cast<RequestHandle>(req.get());
+  std::unique_ptr<RequestImpl> req(new RequestImpl(
+      source_net_log, info, priority, callback, addresses, job));
+  job->AddRequest(req.get());
+  *out_req = std::move(req);
 
-  job->AddRequest(std::move(req));
   // Completion happens during Job::CompleteRequests().
   return ERR_IO_PENDING;
 }
 
 HostResolverImpl::HostResolverImpl(
     const Options& options,
     NetLog* net_log,
     scoped_refptr<base::TaskRunner> worker_task_runner)
     : max_queued_jobs_(0),
       proc_params_(NULL, options.max_retry_attempts),
@@ skipping 110 matching lines @@
     ip_address_ptr = &ip_address;
 
   Key key = GetEffectiveKeyForRequest(info, ip_address_ptr, source_net_log);
 
   int rv = ResolveHelper(key, info, ip_address_ptr, addresses, false, nullptr,
                          source_net_log);
   LogFinishRequest(source_net_log, info, rv);
   return rv;
 }
 
-void HostResolverImpl::ChangeRequestPriority(RequestHandle req_handle,
-                                             RequestPriority priority) {
-  DCHECK(CalledOnValidThread());
-  Request* req = reinterpret_cast<Request*>(req_handle);
-  DCHECK(req);
-  Job* job = req->job();
-  DCHECK(job);
-  job->ChangeRequestPriority(req, priority);
-}
-
-void HostResolverImpl::CancelRequest(RequestHandle req_handle) {
-  DCHECK(CalledOnValidThread());
-  Request* req = reinterpret_cast<Request*>(req_handle);
-  DCHECK(req);
-  Job* job = req->job();
-  DCHECK(job);
-  job->CancelRequest(req);
-}
-
 void HostResolverImpl::SetDnsClientEnabled(bool enabled) {
   DCHECK(CalledOnValidThread());
 #if defined(ENABLE_BUILT_IN_DNS)
   if (enabled && !dns_client_) {
     SetDnsClient(DnsClient::CreateClient(net_log_));
   } else if (!enabled && dns_client_) {
     SetDnsClient(std::unique_ptr<DnsClient>());
   }
 #endif
 }
@@ skipping 421 matching lines @@
     NetworkChangeNotifier::GetDnsConfig(&dns_config);
     dns_client_->SetConfig(dns_config);
     num_dns_failures_ = 0;
     if (dns_client_->GetConfig())
       UMA_HISTOGRAM_BOOLEAN("AsyncDNS.DnsClientEnabled", true);
   }
 
   AbortDnsTasks();
 }
 
+HostResolverImpl::RequestImpl::~RequestImpl() {
+  if (job_)
+    job_->CancelRequest(this);
+}
+
+void HostResolverImpl::RequestImpl::ChangeRequestPriority(
+    RequestPriority priority) {
+  job_->ChangeRequestPriority(this, priority);
+}
+
 }  // namespace net