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

net/dns/host_resolver.h

 // 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.
 
 #ifndef NET_DNS_HOST_RESOLVER_H_
 #define NET_DNS_HOST_RESOLVER_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 24 matching lines @@
 // literal) to an AddressList object.
 //
 // HostResolver can handle multiple requests at a time, so when cancelling a
 // request the RequestHandle that was returned by Resolve() needs to be
 // given.  A simpler alternative for consumers that only have 1 outstanding
 // request at a time is to create a SingleRequestHostResolver wrapper around
 // HostResolver (which will automatically cancel the single request when it
 // goes out of scope).
 class NET_EXPORT HostResolver {
  public:
+  // HostResolver::Request class is used to cancel the request and change it's
+  // priority. It must be owned by consumer. Deletion cancels the request.
+  class Request {
+   public:
+    virtual ~Request() {}
+
+    // Changes the priority of the specified request. |req| is the handle
+    // returned
+    // by Resolve(). ChangeRequestPriority must NOT be called after the
+    // request's
+    // completion callback has already run or the request was canceled.

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

This comment is outdated (There is no |req| or specified request,
ChangeRequestPriority can't be called after the request was cancelled, since the
way to cancel the request is now bug deleting it)

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

Done.

+    virtual void ChangeRequestPriority(RequestPriority priority) = 0;
+  };
+
   // |max_concurrent_resolves| is how many resolve requests will be allowed to
   // run in parallel. Pass HostResolver::kDefaultParallelism to choose a
   // default value.
   // |max_retry_attempts| is the maximum number of times we will retry for host
   // resolution. Pass HostResolver::kDefaultRetryAttempts to choose a default
   // value.
   // |enable_caching| controls whether a HostCache is used.
   struct NET_EXPORT Options {
     Options();
 
@@ skipping 53 matching lines @@
     bool allow_cached_response_;
 
     // Whether this request was started by the DNS prefetcher.
     bool is_speculative_;
 
     // Indicates a request for myIpAddress (to differentiate from other requests
     // for localhost, currently used by Chrome OS).
     bool is_my_ip_address_;
   };
 
-  // Opaque type used to cancel a request.
-  typedef void* RequestHandle;
-
   // Set Options.max_concurrent_resolves to this to select a default level
   // of concurrency.
   static const size_t kDefaultParallelism = 0;
 
   // Set Options.max_retry_attempts to this to select a default retry value.
   static const size_t kDefaultRetryAttempts = static_cast<size_t>(-1);
 
   // If any completion callbacks are pending when the resolver is destroyed,
   // the host resolutions are cancelled, and the completion callbacks will not
   // be called.
   virtual ~HostResolver();
 
   // Resolves the given hostname (or IP address literal), filling out the
   // |addresses| object upon success.  The |info.port| parameter will be set as
   // the sin(6)_port field of the sockaddr_in{6} struct.  Returns OK if
   // successful or an error code upon failure.  Returns
   // ERR_NAME_NOT_RESOLVED if hostname is invalid, or if it is an
   // incompatible IP literal (e.g. IPv6 is disabled and it is an IPv6
   // literal).
   //
   // If the operation cannot be completed synchronously, ERR_IO_PENDING will
   // be returned and the real result code will be passed to the completion
   // callback.  Otherwise the result code is returned immediately from this
   // call.
   //
-  // If |out_req| is non-NULL, then |*out_req| will be filled with a handle to
+  // |out_req| is non-NULL, then |*out_req| will be filled with a handle to
   // the async request. This handle is not valid after the request has
   // completed.
   //
+  // [out_req] must be owned by a caller. When Resolve() is called, the request
+  // is filled with a request handle and attached to a job. After the request
+  // has completed, [out_req] is detached from the job.

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

job is an internal implementation detail of the HostResolverImpl, so shouldn't
be mentioned in the HostResolver docs.  I think this paragraph can be combined
with the previous one, and say something like:

"|out_req| must be non-NULL. If the request is not completed synchronously, it
will be filled with a handle to the request. It must be completed or destroyed
before the HostResolver itself is destroyed."

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

Requests can even be cancelled after the HostResolver itself is destroyed. There
are no restrictions for that. 

+  //
+  // Requests can be cancelled any time during a resolve job by deletion of the
+  // [out_req]. If it is deleted before Resolve() has completed it's job, the
+  // request will not be completed.

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

Maybe delete "during a resolve job", and remove the second sentence?  Or maybe
replace with paragraph with something like "Deleting |out_req| will cancel the
request, and cause |callback| not to be invoked."

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

Done.

+  //
   // Profiling information for the request is saved to |net_log| if non-NULL.
   virtual int Resolve(const RequestInfo& info,
                       RequestPriority priority,
                       AddressList* addresses,
                       const CompletionCallback& callback,
-                      RequestHandle* out_req,
+                      std::unique_ptr<Request>* out_req,
                       const BoundNetLog& net_log) = 0;
 
-  // Changes the priority of the specified request. |req| is the handle returned
-  // by Resolve(). ChangeRequestPriority must NOT be called after the request's
-  // completion callback has already run or the request was canceled.
-  virtual void ChangeRequestPriority(RequestHandle req,
-                                     RequestPriority priority);
-
-  // Cancels the specified request. |req| is the handle returned by Resolve().
-  // After a request is canceled, its completion callback will not be called.
-  // CancelRequest must NOT be called after the request's completion callback
-  // has already run or the request was canceled.
-  virtual void CancelRequest(RequestHandle req) = 0;
-
   // Resolves the given hostname (or IP address literal) out of cache or HOSTS
   // file (if enabled) only. This is guaranteed to complete synchronously.
   // This acts like |Resolve()| if the hostname is IP literal, or cached value
   // or HOSTS entry exists. Otherwise, ERR_DNS_CACHE_MISS is returned.
   virtual int ResolveFromCache(const RequestInfo& info,
                                AddressList* addresses,
                                const BoundNetLog& net_log) = 0;
 
   // Enable or disable the built-in asynchronous DnsClient.
   virtual void SetDnsClientEnabled(bool enabled);
@@ skipping 19 matching lines @@
  protected:
   HostResolver();
 
  private:
   DISALLOW_COPY_AND_ASSIGN(HostResolver);
 };
 
 }  // namespace net
 
 #endif  // NET_DNS_HOST_RESOLVER_H_