Change ProxyResolver::GetProxyForURL() to take a scoped_ptr<Request>* rather than a RequestHandle*

net/proxy/multi_threaded_proxy_resolver.cc

Index: net/proxy/multi_threaded_proxy_resolver.cc
diff --git a/net/proxy/multi_threaded_proxy_resolver.cc b/net/proxy/multi_threaded_proxy_resolver.cc
index 5eae4f744e8d7d79915c8afe3f157da5161fcdf7..e593ee07efbd256fb1d4355c56c7a955bd9db4c3 100644
--- a/net/proxy/multi_threaded_proxy_resolver.cc
+++ b/net/proxy/multi_threaded_proxy_resolver.cc
@@ -117,13 +117,19 @@ class MultiThreadedProxyResolver : public ProxyResolver,
   int GetProxyForURL(const GURL& url,
                      ProxyInfo* results,
                      const CompletionCallback& callback,
-                     RequestHandle* request,
+                     scoped_ptr<Request>* request,
                      const BoundNetLog& net_log) override;
-  void CancelRequest(RequestHandle request) override;
-  LoadState GetLoadState(RequestHandle request) const override;
+
+  void RemovePendingJob(base::WeakPtr<Job> job) {

[eroman, 2016/01/23 02:17:01]

I believe this can be deleted based on my later suggestion.

+    PendingJobsQueue::iterator it =
+        std::find(pending_jobs_.begin(), pending_jobs_.end(), job.get());
+    DCHECK(it != pending_jobs_.end());
+    pending_jobs_.erase(it);
+  }
 
  private:
   class GetProxyForURLJob;
+  class RequestImpl;
   // FIFO queue of pending jobs waiting to be started.
   // TODO(eroman): Make this priority queue.
   typedef std::deque<scoped_refptr<Job>> PendingJobsQueue;
@@ -148,7 +154,8 @@ class MultiThreadedProxyResolver : public ProxyResolver,
 
 // Job ---------------------------------------------
 
-class Job : public base::RefCountedThreadSafe<Job> {
+class Job : public base::SupportsWeakPtr<Job>,

[eroman, 2016/01/23 02:17:01]

I don't believe this should expose a weak pointer.

Since the type is already RefCountedThreadSafe it is easy to deal with shared
ownership by simply holding a refcount instead.

+            public base::RefCountedThreadSafe<Job> {
  public:
   // Identifies the subclass of Job (only being used for debugging purposes).
   enum Type {
@@ -231,6 +238,35 @@ class Job : public base::RefCountedThreadSafe<Job> {
   bool was_cancelled_;
 };
 
+class MultiThreadedProxyResolver::RequestImpl : public ProxyResolver::Request {
+ public:
+  RequestImpl(base::WeakPtr<Job> job, MultiThreadedProxyResolver* resolver)

[eroman, 2016/01/23 02:17:01]

Rather than a WeakPtr, can this instead just hold a strong reference to the Job?

As in, have a member:
 scoped_refptr<Job> job_;

There shouldn't be any reference cycles since callers own the
ProxyResolver::Request and must delete it before the ProxyResolver is deleted

+      : job_(job), resolver_(resolver) {}
+
+  ~RequestImpl() override {

[eroman, 2016/01/23 02:17:01]

We can simplify this to a single line:

job_->Cancel();

The RemovePendingJob() stuff can be rolled up into
MultiThreadedProxyResolver::OnExecutorReady by changing it to check for
cancelled jobs:

while (!pending_jobs_.empty()) {
  scoped_refptr<Job> job = pending_jobs_.front();
  pending_jobs_.pop_front();

  if (!job->was_cancelled()) {
    executor->StartJob(job.get());
    return;
  }
}

+    DCHECK(resolver_->CalledOnValidThread());
+    if (job_) {
+      if (job_->executor()) {
+        // If the job was already submitted to the executor, just mark it
+        // as cancelled so the user callback isn't run on completion.
+        job_->Cancel();
+      } else {
+        // Otherwise the job is just sitting in a queue.
+        resolver_->RemovePendingJob(job_);
+      }
+    }
+  }
+
+  LoadState GetLoadState() override {
+    DCHECK(resolver_->CalledOnValidThread());
+    return LOAD_STATE_RESOLVING_PROXY_FOR_URL;
+  }
+
+ private:
+  base::WeakPtr<Job> job_;
+  MultiThreadedProxyResolver* resolver_;

[eroman, 2016/01/23 02:17:01]

With the suggestion above, this member is no longer needed, and RequestImpl is
nothing more than a light wrapper around the Job.

+};
+
 // CreateResolverJob -----------------------------------------------------------
 
 // Runs on the worker thread to call ProxyResolverFactory::CreateProxyResolver.
@@ -442,8 +478,11 @@ MultiThreadedProxyResolver::~MultiThreadedProxyResolver() {
 }
 
 int MultiThreadedProxyResolver::GetProxyForURL(
-    const GURL& url, ProxyInfo* results, const CompletionCallback& callback,
-    RequestHandle* request, const BoundNetLog& net_log) {
+    const GURL& url,
+    ProxyInfo* results,
+    const CompletionCallback& callback,
+    scoped_ptr<Request>* request,
+    const BoundNetLog& net_log) {
   DCHECK(CalledOnValidThread());
   DCHECK(!callback.is_null());
 
@@ -453,7 +492,7 @@ int MultiThreadedProxyResolver::GetProxyForURL(
   // Completion will be notified through |callback|, unless the caller cancels
   // the request using |request|.
   if (request)
-    *request = reinterpret_cast<RequestHandle>(job.get());
+    request->reset(new RequestImpl(job->AsWeakPtr(), this));
 
   // If there is an executor that is ready to run this request, submit it!
   Executor* executor = FindIdleExecutor();
@@ -476,31 +515,6 @@ int MultiThreadedProxyResolver::GetProxyForURL(
   return ERR_IO_PENDING;
 }
 
-void MultiThreadedProxyResolver::CancelRequest(RequestHandle req) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(req);
-
-  Job* job = reinterpret_cast<Job*>(req);
-  DCHECK_EQ(Job::TYPE_GET_PROXY_FOR_URL, job->type());
-
-  if (job->executor()) {
-    // If the job was already submitted to the executor, just mark it
-    // as cancelled so the user callback isn't run on completion.
-    job->Cancel();
-  } else {
-    // Otherwise the job is just sitting in a queue.
-    PendingJobsQueue::iterator it =
-        std::find(pending_jobs_.begin(), pending_jobs_.end(), job);
-    DCHECK(it != pending_jobs_.end());
-    pending_jobs_.erase(it);
-  }
-}
-
-LoadState MultiThreadedProxyResolver::GetLoadState(RequestHandle req) const {
-  DCHECK(CalledOnValidThread());
-  DCHECK(req);
-  return LOAD_STATE_RESOLVING_PROXY_FOR_URL;
-}
 
 Executor* MultiThreadedProxyResolver::FindIdleExecutor() {
   DCHECK(CalledOnValidThread());