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

net/proxy/mock_proxy_resolver.cc

Index: net/proxy/mock_proxy_resolver.cc
diff --git a/net/proxy/mock_proxy_resolver.cc b/net/proxy/mock_proxy_resolver.cc
index 21cac0026cb24a8975e01868ed8d403fccaf19bf..19ed4600b8fd783946e3e373719f9d91533b8ce2 100644
--- a/net/proxy/mock_proxy_resolver.cc
+++ b/net/proxy/mock_proxy_resolver.cc
@@ -11,61 +11,73 @@
 
 namespace net {
 
-MockAsyncProxyResolver::Request::Request(MockAsyncProxyResolver* resolver,
-                                         const GURL& url,
-                                         ProxyInfo* results,
-                                         const CompletionCallback& callback)
+MockAsyncProxyResolver::RequestImpl::RequestImpl(scoped_refptr<Job> job)
+    : job_(job) {
+  DCHECK(job_);
+}
+
+MockAsyncProxyResolver::RequestImpl::~RequestImpl() {
+  job_->Resolver()->AddCancelledJob(job_);

[eroman, 2016/02/24 03:08:05]

At a minimum add a comment explaining that AddCancelledJob() checks to see if
the request was already completed (otherwise this looks incorrect to casual
readers).

+}
+
+LoadState MockAsyncProxyResolver::RequestImpl::GetLoadState() {
+  return LOAD_STATE_RESOLVING_PROXY_FOR_URL;
+}
+
+MockAsyncProxyResolver::Job::Job(MockAsyncProxyResolver* resolver,
+                                 const GURL& url,
+                                 ProxyInfo* results,
+                                 const CompletionCallback& callback)
     : resolver_(resolver),
       url_(url),
       results_(results),
       callback_(callback),
-      origin_loop_(base::MessageLoop::current()) {
-}
+      origin_loop_(base::MessageLoop::current()) {}
+
+MockAsyncProxyResolver::Job::~Job() {}
 
-void MockAsyncProxyResolver::Request::CompleteNow(int rv) {
+void MockAsyncProxyResolver::Job::CompleteNow(int rv) {
   CompletionCallback callback = callback_;
 
   // May delete |this|.

[eroman, 2016/02/24 03:08:05]

Remove this comment, as I don't believe it is true anymore.

-  resolver_->RemovePendingRequest(this);
+  resolver_->RemovePendingJob(this);
 
   callback.Run(rv);
 }
 
-MockAsyncProxyResolver::Request::~Request() {}
-
 MockAsyncProxyResolver::~MockAsyncProxyResolver() {}
 
 int MockAsyncProxyResolver::GetProxyForURL(const GURL& url,
                                            ProxyInfo* results,
                                            const CompletionCallback& callback,
-                                           RequestHandle* request_handle,
+                                           scoped_ptr<Request>* request,
                                            const BoundNetLog& /*net_log*/) {
-  scoped_refptr<Request> request = new Request(this, url, results, callback);
-  pending_requests_.push_back(request);
+  scoped_refptr<Job> job = new Job(this, url, results, callback);
 
-  if (request_handle)
-    *request_handle = reinterpret_cast<RequestHandle>(request.get());
+  pending_jobs_.push_back(job.get());
+  request->reset(new RequestImpl(job));
 
   // Test code completes the request by calling request->CompleteNow().

[eroman, 2016/02/24 03:08:05]

Please update this comment.

   return ERR_IO_PENDING;
 }
 
-void MockAsyncProxyResolver::CancelRequest(RequestHandle request_handle) {
-  scoped_refptr<Request> request = reinterpret_cast<Request*>(request_handle);
-  cancelled_requests_.push_back(request);
-  RemovePendingRequest(request.get());
-}
-
-LoadState MockAsyncProxyResolver::GetLoadState(
-    RequestHandle request_handle) const {
-  return LOAD_STATE_RESOLVING_PROXY_FOR_URL;
+void MockAsyncProxyResolver::AddCancelledJob(scoped_refptr<Job> job) {
+  std::vector<Job*>::iterator it =
+      std::find(pending_jobs_.begin(), pending_jobs_.end(), job.get());
+  // Because this is called always when RequestImpl is destructed,
+  // we need to check if it is still in pending jobs.
+  if (it != pending_jobs_.end()) {
+    cancelled_jobs_.push_back(job);
+    pending_jobs_.erase(it);
+  }
 }
 
-void MockAsyncProxyResolver::RemovePendingRequest(Request* request) {
-  RequestsList::iterator it = std::find(
-      pending_requests_.begin(), pending_requests_.end(), request);
-  DCHECK(it != pending_requests_.end());
-  pending_requests_.erase(it);
+void MockAsyncProxyResolver::RemovePendingJob(Job* job) {
+  DCHECK(job);
+  std::vector<Job*>::iterator it =
+      std::find(pending_jobs_.begin(), pending_jobs_.end(), job);
+  DCHECK(it != pending_jobs_.end());
+  pending_jobs_.erase(it);
 }
 
 MockAsyncProxyResolver::MockAsyncProxyResolver() {
@@ -165,17 +177,9 @@ ForwardingProxyResolver::ForwardingProxyResolver(ProxyResolver* impl)
 int ForwardingProxyResolver::GetProxyForURL(const GURL& query_url,
                                             ProxyInfo* results,
                                             const CompletionCallback& callback,
-                                            RequestHandle* request,
+                                            scoped_ptr<Request>* request,
                                             const BoundNetLog& net_log) {
   return impl_->GetProxyForURL(query_url, results, callback, request, net_log);
 }
 
-void ForwardingProxyResolver::CancelRequest(RequestHandle request) {
-  impl_->CancelRequest(request);
-}
-
-LoadState ForwardingProxyResolver::GetLoadState(RequestHandle request) const {
-  return impl_->GetLoadState(request);
-}
-
 }  // namespace net