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

net/proxy/proxy_resolver_factory_mojo.cc

Index: net/proxy/proxy_resolver_factory_mojo.cc
diff --git a/net/proxy/proxy_resolver_factory_mojo.cc b/net/proxy/proxy_resolver_factory_mojo.cc
index 32c1203e19b0dade14ed4cdb1318ab7ed4d832b5..6e8376518187bd3762c3abac75b7ae9033585d77 100644
--- a/net/proxy/proxy_resolver_factory_mojo.cc
+++ b/net/proxy/proxy_resolver_factory_mojo.cc
@@ -116,13 +116,13 @@ class ProxyResolverMojo : public ProxyResolver {
   int GetProxyForURL(const GURL& url,
                      ProxyInfo* results,
                      const net::CompletionCallback& callback,
-                     RequestHandle* request,
+                     scoped_ptr<Request>* request,
                      const BoundNetLog& net_log) override;
-  void CancelRequest(RequestHandle request) override;
-  LoadState GetLoadState(RequestHandle request) const override;
+  base::ThreadChecker thread_checker_;  // TODO this should be private

[eroman, 2015/11/24 01:20:59]

why not make it private here?

 
  private:
   class Job;
+  class RequestImpl;
 
   // Mojo error handler.
   void OnConnectionError();
@@ -140,13 +140,25 @@ class ProxyResolverMojo : public ProxyResolver {
 
   std::set<Job*> pending_jobs_;
 
-  base::ThreadChecker thread_checker_;
 
   scoped_ptr<base::ScopedClosureRunner> on_delete_callback_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(ProxyResolverMojo);
 };
 
+class ProxyResolverMojo::RequestImpl : public ProxyResolver::Request {
+ public:
+  RequestImpl(Job* job, ProxyResolverMojo* resolver);
+
+  ~RequestImpl() override;
+
+  LoadState GetLoadState() override;
+
+ private:
+  Job* job_;
+  ProxyResolverMojo* resolver_;
+};
+
 class ProxyResolverMojo::Job
     : public ClientMixin<interfaces::ProxyResolverRequestClient> {
  public:
@@ -181,6 +193,22 @@ class ProxyResolverMojo::Job
   mojo::Binding<interfaces::ProxyResolverRequestClient> binding_;
 };
 
+ProxyResolverMojo::RequestImpl::RequestImpl(Job* job,
+                                            ProxyResolverMojo* resolver)
+    : job_(job), resolver_(resolver) {}
+
+ProxyResolverMojo::RequestImpl::~RequestImpl() {
+  DCHECK(resolver_->thread_checker_.CalledOnValidThread());
+  DCHECK(job_);
+  job_->Cancel();

[eroman, 2015/11/24 01:20:59]

Same issue on ownership -- Can't job_ already be deleted here?

+  resolver_->RemoveJob(job_);
+}
+
+LoadState ProxyResolverMojo::RequestImpl::GetLoadState() {
+  CHECK_EQ(1u, resolver_->pending_jobs_.count(job_));
+  return job_->GetLoadState();
+}
+
 ProxyResolverMojo::Job::Job(ProxyResolverMojo* resolver,
                             const GURL& url,
                             ProxyInfo* results,
@@ -284,7 +312,7 @@ void ProxyResolverMojo::RemoveJob(Job* job) {
 int ProxyResolverMojo::GetProxyForURL(const GURL& url,
                                       ProxyInfo* results,
                                       const CompletionCallback& callback,
-                                      RequestHandle* request,
+                                      scoped_ptr<Request>* request,
                                       const BoundNetLog& net_log) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
@@ -294,24 +322,12 @@ int ProxyResolverMojo::GetProxyForURL(const GURL& url,
   Job* job = new Job(this, url, results, callback, net_log);
   bool inserted = pending_jobs_.insert(job).second;
   DCHECK(inserted);
-  *request = job;
+  request->reset(new RequestImpl(job, this));
 
   return ERR_IO_PENDING;
 }
 
-void ProxyResolverMojo::CancelRequest(RequestHandle request) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  Job* job = static_cast<Job*>(request);
-  DCHECK(job);
-  job->Cancel();
-  RemoveJob(job);
-}
 
-LoadState ProxyResolverMojo::GetLoadState(RequestHandle request) const {
-  Job* job = static_cast<Job*>(request);
-  CHECK_EQ(1u, pending_jobs_.count(job));
-  return job->GetLoadState();
-}
 
 }  // namespace