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

net/dns/host_resolver_mojo.cc

 // Copyright 2015 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_mojo.h"
 
 #include <utility>
 
+#include "base/callback_helpers.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "net/base/address_list.h"
 #include "net/base/net_errors.h"
 #include "net/dns/mojo_host_type_converters.h"
 #include "net/log/net_log.h"
 
 namespace net {
 namespace {
 
 // Default TTL for successful host resolutions.
@@ skipping 25 matching lines @@
   // Mojo error handler.
   void OnConnectionError();
 
   const HostCache::Key key_;
   AddressList* addresses_;
   CompletionCallback callback_;
   mojo::Binding<interfaces::HostResolverRequestClient> binding_;
   base::WeakPtr<HostCache> host_cache_;
 };
 
+class HostResolverMojo::RequestImpl : public HostResolver::Request {
+ public:
+  explicit RequestImpl(std::unique_ptr<Job> job) : job_(std::move(job)) {}
+
+  ~RequestImpl() override { job_.reset(); }

[mmenke, 2016/07/19 19:03:56]

You can just give this an empty body - job_ will be destroyed since it's stored
in a unique_ptr.

[maksims (do not use this acc), 2016/07/21 07:12:46]

Done.

+
+  void ChangeRequestPriority(RequestPriority priority) override {}
+
+ private:
+  std::unique_ptr<Job> job_;
+};
+
 HostResolverMojo::HostResolverMojo(Impl* impl)
     : impl_(impl),
       host_cache_(HostCache::CreateDefaultCache()),
       host_cache_weak_factory_(host_cache_.get()) {
 }
 
 HostResolverMojo::~HostResolverMojo() = default;
 
 int HostResolverMojo::Resolve(const RequestInfo& info,
                               RequestPriority priority,
                               AddressList* addresses,
                               const CompletionCallback& callback,
-                              RequestHandle* request_handle,
+                              std::unique_ptr<Request>* request,
                               const BoundNetLog& source_net_log) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) << "Resolve " << info.host_port_pair().ToString();
 
   HostCache::Key key = CacheKeyForRequest(info);
   int cached_result = ResolveFromCacheInternal(info, key, addresses);
   if (cached_result != ERR_DNS_CACHE_MISS) {
     DVLOG(1) << "Resolved " << info.host_port_pair().ToString()
              << " from cache";
     return cached_result;
   }
 
   interfaces::HostResolverRequestClientPtr handle;
-  *request_handle = new Job(key, addresses, callback, mojo::GetProxy(&handle),
-                            host_cache_weak_factory_.GetWeakPtr());
+  std::unique_ptr<Job> job(new Job(key, addresses, callback,
+                                   mojo::GetProxy(&handle),
+                                   host_cache_weak_factory_.GetWeakPtr()));
+  (*request).reset(new RequestImpl(std::move(job)));

[mmenke, 2016/07/19 19:03:56]

nit:  request->reset(new RequestImpl(std::move(job));

[maksims (do not use this acc), 2016/07/21 07:12:46]

Done.

+
   impl_->ResolveDns(interfaces::HostResolverRequestInfo::From(info),
                     std::move(handle));
   return ERR_IO_PENDING;
 }
 
 int HostResolverMojo::ResolveFromCache(const RequestInfo& info,
                                        AddressList* addresses,
                                        const BoundNetLog& source_net_log) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) << "ResolveFromCache " << info.host_port_pair().ToString();
   return ResolveFromCacheInternal(info, CacheKeyForRequest(info), addresses);
 }
 
-void HostResolverMojo::ChangeRequestPriority(RequestHandle req,
-                                             RequestPriority priority) {
-  // Do nothing, since Resolve() discarded the priority anyway.
-}
-
-void HostResolverMojo::CancelRequest(RequestHandle req) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  // Deleting the Job closes the HostResolverRequestClient connection,
-  // signalling cancellation of the request.
-  delete static_cast<Job*>(req);
-}
-
 HostCache* HostResolverMojo::GetHostCache() {
   return host_cache_.get();
 }
 
 int HostResolverMojo::ResolveFromCacheInternal(const RequestInfo& info,
                                                const HostCache::Key& key,
                                                AddressList* addresses) {
   if (!info.allow_cached_response())
     return ERR_DNS_CACHE_MISS;
 
@@ skipping 25 matching lines @@
     int32_t error,
     interfaces::AddressListPtr address_list) {
   if (error == OK && address_list)
     *addresses_ = address_list->To<AddressList>();
   if (host_cache_) {
     base::TimeDelta ttl = base::TimeDelta::FromSeconds(
         error == OK ? kCacheEntryTTLSeconds : kNegativeCacheEntryTTLSeconds);
     HostCache::Entry entry(error, *addresses_, ttl);
     host_cache_->Set(key_, entry, base::TimeTicks::Now(), ttl);
   }
-  callback_.Run(error);
-  delete this;
+  if (binding_.is_bound())
+    binding_.Close();
+  base::ResetAndReturn(&callback_).Run(error);
 }
 
 void HostResolverMojo::Job::OnConnectionError() {
   ReportResult(ERR_FAILED, interfaces::AddressListPtr());
 }
 
 }  // namespace net