Reland "Change ProxyResolver::GetProxyForURL() to take a unique_ptr<Request>* "

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 9b4246671c0c68670d72a3db4066fc939e569c19..f328db932651fa6d3519549f0a15faf0ff8deb49 100644
--- a/net/proxy/proxy_resolver_factory_mojo.cc
+++ b/net/proxy/proxy_resolver_factory_mojo.cc
@@ -124,19 +124,18 @@ class ProxyResolverMojo : public ProxyResolver {
   int GetProxyForURL(const GURL& url,
                      ProxyInfo* results,
                      const net::CompletionCallback& callback,
-                     RequestHandle* request,
+                     std::unique_ptr<Request>* request,
                      const NetLogWithSource& net_log) override;
-  void CancelRequest(RequestHandle request) override;
-  LoadState GetLoadState(RequestHandle request) const override;
 
  private:
   class Job;
+  class RequestImpl;
+
+  base::ThreadChecker thread_checker_;
 
   // Mojo error handler.
   void OnConnectionError();
 
-  void RemoveJob(Job* job);
-
   // Connection to the Mojo proxy resolver.
   interfaces::ProxyResolverPtr mojo_proxy_resolver_ptr_;
 
@@ -146,15 +145,25 @@ class ProxyResolverMojo : public ProxyResolver {
 
   NetLog* net_log_;
 
-  std::set<Job*> pending_jobs_;
-
-  base::ThreadChecker thread_checker_;
-
   std::unique_ptr<base::ScopedClosureRunner> on_delete_callback_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(ProxyResolverMojo);
 };
 
+class ProxyResolverMojo::RequestImpl : public ProxyResolver::Request {
+ public:
+  explicit RequestImpl(std::unique_ptr<Job> job);
+
+  ~RequestImpl() override {}
+
+  LoadState GetLoadState() override;
+
+ private:
+  std::unique_ptr<Job> job_;
+
+  DISALLOW_COPY_AND_ASSIGN(RequestImpl);
+};
+
 class ProxyResolverMojo::Job
     : public ClientMixin<interfaces::ProxyResolverRequestClient> {
  public:
@@ -165,9 +174,6 @@ class ProxyResolverMojo::Job
       const NetLogWithSource& net_log);
   ~Job() override;
 
-  // Cancels the job and prevents the callback from being run.
-  void Cancel();
-
   // Returns the LoadState of this job.
   LoadState GetLoadState();
 
@@ -178,15 +184,26 @@ class ProxyResolverMojo::Job
   // Overridden from interfaces::ProxyResolverRequestClient:
   void ReportResult(int32_t error, const net::ProxyInfo& proxy_info) override;
 
-  ProxyResolverMojo* resolver_;
+  // Completes a request with a result code.
+  void CompleteRequest(int result);
+
   const GURL url_;
   ProxyInfo* results_;
   CompletionCallback callback_;
 
   base::ThreadChecker thread_checker_;
   mojo::Binding<interfaces::ProxyResolverRequestClient> binding_;
+
+  DISALLOW_COPY_AND_ASSIGN(Job);
 };
 
+ProxyResolverMojo::RequestImpl::RequestImpl(std::unique_ptr<Job> job)
+    : job_(std::move(job)) {}
+
+LoadState ProxyResolverMojo::RequestImpl::GetLoadState() {
+  return job_->GetLoadState();
+}
+
 ProxyResolverMojo::Job::Job(ProxyResolverMojo* resolver,
                             const GURL& url,
                             ProxyInfo* results,
@@ -197,28 +214,17 @@ ProxyResolverMojo::Job::Job(ProxyResolverMojo* resolver,
           resolver->error_observer_.get(),
           resolver->net_log_,
           net_log),
-      resolver_(resolver),
       url_(url),
       results_(results),
       callback_(callback),
       binding_(this) {
-  resolver_->mojo_proxy_resolver_ptr_->GetProxyForUrl(
+  resolver->mojo_proxy_resolver_ptr_->GetProxyForUrl(
       url_, binding_.CreateInterfacePtrAndBind());
   binding_.set_connection_error_handler(base::Bind(
       &ProxyResolverMojo::Job::OnConnectionError, base::Unretained(this)));
 }
 
-ProxyResolverMojo::Job::~Job() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  if (!callback_.is_null())
-    callback_.Run(ERR_PAC_SCRIPT_TERMINATED);
-}
-
-void ProxyResolverMojo::Job::Cancel() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(!callback_.is_null());
-  callback_.Reset();
-}
+ProxyResolverMojo::Job::~Job() {}
 
 LoadState ProxyResolverMojo::Job::GetLoadState() {
   return dns_request_in_progress() ? LOAD_STATE_RESOLVING_HOST_IN_PROXY_SCRIPT
@@ -228,7 +234,14 @@ LoadState ProxyResolverMojo::Job::GetLoadState() {
 void ProxyResolverMojo::Job::OnConnectionError() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) << "ProxyResolverMojo::Job::OnConnectionError";
-  resolver_->RemoveJob(this);
+  CompleteRequest(ERR_PAC_SCRIPT_TERMINATED);
+}
+
+void ProxyResolverMojo::Job::CompleteRequest(int result) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  CompletionCallback callback = base::ResetAndReturn(&callback_);
+  binding_.Close();
+  callback.Run(result);
 }
 
 void ProxyResolverMojo::Job::ReportResult(int32_t error,
@@ -241,10 +254,7 @@ void ProxyResolverMojo::Job::ReportResult(int32_t error,
     DVLOG(1) << "Servers: " << results_->ToPacString();
   }
 
-  CompletionCallback callback = callback_;
-  callback_.Reset();
-  resolver_->RemoveJob(this);
-  callback.Run(error);
+  CompleteRequest(error);
 }
 
 ProxyResolverMojo::ProxyResolverMojo(
@@ -264,8 +274,6 @@ ProxyResolverMojo::ProxyResolverMojo(
 
 ProxyResolverMojo::~ProxyResolverMojo() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  // All pending requests should have been cancelled.
-  DCHECK(pending_jobs_.empty());
 }
 
 void ProxyResolverMojo::OnConnectionError() {
@@ -276,45 +284,22 @@ void ProxyResolverMojo::OnConnectionError() {
   mojo_proxy_resolver_ptr_.reset();
 }
 
-void ProxyResolverMojo::RemoveJob(Job* job) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  size_t num_erased = pending_jobs_.erase(job);
-  DCHECK(num_erased);
-  delete job;
-}
-
 int ProxyResolverMojo::GetProxyForURL(const GURL& url,
                                       ProxyInfo* results,
                                       const CompletionCallback& callback,
-                                      RequestHandle* request,
+                                      std::unique_ptr<Request>* request,
                                       const NetLogWithSource& net_log) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!mojo_proxy_resolver_ptr_)
     return ERR_PAC_SCRIPT_TERMINATED;
 
-  Job* job = new Job(this, url, results, callback, net_log);
-  bool inserted = pending_jobs_.insert(job).second;
-  DCHECK(inserted);
-  *request = job;
+  std::unique_ptr<Job> job(new Job(this, url, results, callback, net_log));
+  request->reset(new RequestImpl(std::move(job)));
 
   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
 
 // A Job to create a ProxyResolver instance.