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

net/proxy/proxy_resolver_v8_tracing.cc

Index: net/proxy/proxy_resolver_v8_tracing.cc
diff --git a/net/proxy/proxy_resolver_v8_tracing.cc b/net/proxy/proxy_resolver_v8_tracing.cc
index 94ad9f0d6c4df9fcd4a6b1c401f51968b6eb7fc2..3cbc483b1f4127e9eab5cc33767039fbfcfce305 100644
--- a/net/proxy/proxy_resolver_v8_tracing.cc
+++ b/net/proxy/proxy_resolver_v8_tracing.cc
@@ -75,6 +75,7 @@ const size_t kMaxAlertsAndErrorsBytes = 2048;
 // that spawned it. Destruction might happen on either the origin thread or the
 // worker thread.
 class Job : public base::RefCountedThreadSafe<Job>,
+            public base::SupportsWeakPtr<Job>,
             public ProxyResolverV8::JSBindings {
  public:
   struct Params {
@@ -311,10 +312,18 @@ class ProxyResolverV8TracingImpl : public ProxyResolverV8Tracing,
   void GetProxyForURL(const GURL& url,
                       ProxyInfo* results,
                       const CompletionCallback& callback,
-                      ProxyResolver::RequestHandle* request,
+                      scoped_ptr<ProxyResolver::Request>* request,
                       scoped_ptr<Bindings> bindings) override;
-  void CancelRequest(ProxyResolver::RequestHandle request) override;
-  LoadState GetLoadState(ProxyResolver::RequestHandle request) const override;
+
+  class RequestImpl : public ProxyResolverV8Tracing::Request {
+   public:
+    RequestImpl(base::WeakPtr<Job> job);
+    ~RequestImpl() override;
+    LoadState GetLoadState() override;
+
+   private:
+    base::WeakPtr<Job> job_;
+  };
 
  private:
   // The worker thread on which the ProxyResolverV8 will be run.
@@ -434,6 +443,8 @@ void Job::SetCallback(const CompletionCallback& callback) {
 }
 
 void Job::ReleaseCallback() {
+  if (callback_.is_null())
+    return;
   CheckIsOnOriginThread();
   DCHECK(!callback_.is_null());
   CHECK_GT(*params_->num_outstanding_callbacks, 0);
@@ -934,11 +945,23 @@ ProxyResolverV8TracingImpl::~ProxyResolverV8TracingImpl() {
   thread_.reset();
 }
 
+ProxyResolverV8TracingImpl::RequestImpl::RequestImpl(base::WeakPtr<Job> job)
+    : job_(job) {}

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

Same comment as for MultiThreadedProxyResolver. I don't think we want a weak
pointer here, can instead hold a strong reference.

+
+ProxyResolverV8TracingImpl::RequestImpl::~RequestImpl() {
+  if (job_)

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

Shouldn't need this check once making job_ a strong reference.

+    job_->Cancel();
+}
+
+LoadState ProxyResolverV8TracingImpl::RequestImpl::GetLoadState() {
+  return job_->GetLoadState();
+}
+
 void ProxyResolverV8TracingImpl::GetProxyForURL(
     const GURL& url,
     ProxyInfo* results,
     const CompletionCallback& callback,
-    ProxyResolver::RequestHandle* request,
+    scoped_ptr<ProxyResolver::Request>* request,
     scoped_ptr<Bindings> bindings) {
   DCHECK(CalledOnValidThread());
   DCHECK(!callback.is_null());
@@ -946,22 +969,11 @@ void ProxyResolverV8TracingImpl::GetProxyForURL(
   scoped_refptr<Job> job = new Job(job_params_.get(), std::move(bindings));
 
   if (request)

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

I wonder if we actually use this check (should at some mandate that |request !=
nullptr|.
Shouldn't matter for this CL though since the Job is kept alive by the executor
already.

-    *request = job.get();
+    request->reset(new RequestImpl(job->AsWeakPtr()));
 
   job->StartGetProxyForURL(url, results, callback);
 }
 
-void ProxyResolverV8TracingImpl::CancelRequest(
-    ProxyResolver::RequestHandle request) {
-  Job* job = reinterpret_cast<Job*>(request);
-  job->Cancel();
-}
-
-LoadState ProxyResolverV8TracingImpl::GetLoadState(
-    ProxyResolver::RequestHandle request) const {
-  Job* job = reinterpret_cast<Job*>(request);
-  return job->GetLoadState();
-}
 
 class ProxyResolverV8TracingFactoryImpl : public ProxyResolverV8TracingFactory {
  public: