Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(278)

Unified Diff: net/proxy/proxy_resolver_v8_tracing.cc

Issue 1439053002: Change ProxyResolver::GetProxyForURL() to take a scoped_ptr<Request>* rather than a RequestHandle* (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Restore scoped_ptr to mock and nits Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/proxy/proxy_resolver_v8_tracing.h ('k') | net/proxy/proxy_resolver_v8_tracing_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 dd2fe401d5a4b5539aeab912abd10d9cee79bca4..937596999da09dcd7685971db53ac81aa158de67 100644
--- a/net/proxy/proxy_resolver_v8_tracing.cc
+++ b/net/proxy/proxy_resolver_v8_tracing.cc
@@ -311,10 +311,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 ProxyResolver::Request {
+ public:
+ explicit RequestImpl(scoped_refptr<Job> job);
+ ~RequestImpl() override;
+ LoadState GetLoadState() override;
+
+ private:
+ scoped_refptr<Job> job_;
+ };
private:
// The worker thread on which the ProxyResolverV8 will be run.
@@ -383,10 +391,16 @@ void Job::Cancel() {
// posted after the DNS dependency was resolved and saved to local cache.
// (f) The script execution completed entirely, and posted a task to the
// origin thread to notify the caller.
+ // (g) The job is already completed.
//
// |cancelled_| is read on both the origin thread and worker thread. The
// code that runs on the worker thread is littered with checks on
// |cancelled_| to break out early.
+
+ // If the job already completed, there is nothing to be cancelled.
+ if (callback_.is_null())
+ return;
+
cancelled_.Set();
ReleaseCallback();
@@ -934,34 +948,33 @@ ProxyResolverV8TracingImpl::~ProxyResolverV8TracingImpl() {
thread_.reset();
}
+ProxyResolverV8TracingImpl::RequestImpl::RequestImpl(scoped_refptr<Job> job)
+ : job_(std::move(job)) {}
+
+ProxyResolverV8TracingImpl::RequestImpl::~RequestImpl() {
+ 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());
scoped_refptr<Job> job = new Job(job_params_.get(), std::move(bindings));
- if (request)
- *request = job.get();
+ request->reset(new RequestImpl(job));
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:
« no previous file with comments | « net/proxy/proxy_resolver_v8_tracing.h ('k') | net/proxy/proxy_resolver_v8_tracing_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698