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

Unified Diff: net/base/host_resolver_impl.cc

Issue 5710002: Create base::WorkerPoolJob. Use it for HostResolverImpl and DirectoryLister. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Address comments. Created 10 years 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
Index: net/base/host_resolver_impl.cc
diff --git a/net/base/host_resolver_impl.cc b/net/base/host_resolver_impl.cc
index b53ec6d3ccd5caeba55953ec6c195d117a396b07..f40814de73d49414c44260df638ae3ee92516b7d 100644
--- a/net/base/host_resolver_impl.cc
+++ b/net/base/host_resolver_impl.cc
@@ -27,7 +27,7 @@
#include "base/time.h"
#include "base/utf_string_conversions.h"
#include "base/values.h"
-#include "base/worker_pool.h"
+#include "base/worker_pool_job.h"
#include "net/base/address_list.h"
#include "net/base/address_list_net_log_param.h"
#include "net/base/host_port_pair.h"
@@ -338,8 +338,7 @@ class HostResolverImpl::Request {
// This class represents a request to the worker pool for a "getaddrinfo()"
// call.
-class HostResolverImpl::Job
- : public base::RefCountedThreadSafe<HostResolverImpl::Job> {
+class HostResolverImpl::Job : public base::WorkerPoolJob {
public:
Job(int id,
HostResolverImpl* resolver,
@@ -349,7 +348,6 @@ class HostResolverImpl::Job
: id_(id),
key_(key),
resolver_(resolver),
- origin_loop_(MessageLoop::current()),
resolver_proc_(resolver->effective_resolver_proc()),
error_(OK),
os_error_(0),
@@ -381,33 +379,17 @@ class HostResolverImpl::Job
void Start() {
start_time_ = base::TimeTicks::Now();
- // Dispatch the job to a worker thread.
- if (!WorkerPool::PostTask(FROM_HERE,
- NewRunnableMethod(this, &Job::DoLookup), true)) {
- NOTREACHED();
-
- // Since we could be running within Resolve() right now, we can't just
- // call OnLookupComplete(). Instead we must wait until Resolve() has
- // returned (IO_PENDING).
- error_ = ERR_UNEXPECTED;
- MessageLoop::current()->PostTask(
- FROM_HERE, NewRunnableMethod(this, &Job::OnLookupComplete));
- }
+ StartJob();
}
// Cancels the current job. Callable from origin thread.
void Cancel() {
- net_log_.AddEvent(NetLog::TYPE_CANCELLED, NULL);
+ if (canceled())
+ return;
- HostResolver* resolver = resolver_;
- resolver_ = NULL;
+ CancelJob();
- // Mark the job as cancelled, so when worker thread completes it will
- // not try to post completion to origin loop.
- {
- AutoLock locked(origin_loop_lock_);
- origin_loop_ = NULL;
- }
+ net_log_.AddEvent(NetLog::TYPE_CANCELLED, NULL);
// End here to prevent issues when a Job outlives the HostResolver that
// spawned it.
@@ -419,16 +401,11 @@ class HostResolverImpl::Job
it != requests_.end(); ++it) {
HostResolverImpl::Request* req = *it;
if (!req->was_cancelled())
- resolver->CancelRequest(req);
+ resolver_->CancelRequest(req);
}
}
// Called from origin thread.
- bool was_cancelled() const {
- return resolver_ == NULL;
- }
-
- // Called from origin thread.
const Key& key() const {
return key_;
}
@@ -448,7 +425,6 @@ class HostResolverImpl::Job
// Returns the first request attached to the job.
const Request* initial_request() const {
- DCHECK_EQ(origin_loop_, MessageLoop::current());
DCHECK(!requests_.empty());
return requests_[0];
}
@@ -458,6 +434,9 @@ class HostResolverImpl::Job
return key_ == resolver_->GetEffectiveKeyForRequest(req_info);
}
+ // TODO(willchan): Eliminate the need for this.
+ using WorkerPoolJob::canceled;
+
private:
friend class base::RefCountedThreadSafe<HostResolverImpl::Job>;
@@ -470,7 +449,7 @@ class HostResolverImpl::Job
// wait for it to finish, so we must be very careful here about using other
// objects (like MessageLoops, Singletons, etc). During shutdown these objects
// may no longer exist.
- void DoLookup() {
+ virtual void RunJob() {
// Running on the worker thread
error_ = ResolveAddrInfo(resolver_proc_,
key_.hostname,
@@ -478,24 +457,10 @@ class HostResolverImpl::Job
key_.host_resolver_flags,
&results_,
&os_error_);
-
- // The origin loop could go away while we are trying to post to it, so we
- // need to call its PostTask method inside a lock. See ~HostResolver.
- {
- AutoLock locked(origin_loop_lock_);
- if (origin_loop_) {
- origin_loop_->PostTask(FROM_HERE,
- NewRunnableMethod(this, &Job::OnLookupComplete));
- }
- }
}
- // Callback for when DoLookup() completes (runs on origin thread).
- void OnLookupComplete() {
- // Should be running on origin loop.
- // TODO(eroman): this is being hit by URLRequestTest.CancelTest*,
- // because MessageLoop::current() == NULL.
- //DCHECK_EQ(origin_loop_, MessageLoop::current());
+ // Callback for when RunJob() completes (runs on origin thread).
+ virtual void OnJobCompleted() {
DCHECK(error_ || results_.head());
// Ideally the following code would be part of host_resolver_proc.cc,
@@ -506,9 +471,6 @@ class HostResolverImpl::Job
RecordPerformanceHistograms();
- if (was_cancelled())
- return;
-
scoped_refptr<NetLog::EventParameters> params;
if (error_ != OK) {
params = new HostResolveFailedParams(error_, os_error_);
@@ -590,8 +552,6 @@ class HostResolverImpl::Job
}
}
-
-
// Immutable. Can be read from either thread,
const int id_;
@@ -602,10 +562,6 @@ class HostResolverImpl::Job
HostResolverImpl* resolver_;
RequestsList requests_; // The requests waiting on this job.
- // Used to post ourselves onto the origin thread.
- Lock origin_loop_lock_;
- MessageLoop* origin_loop_;
-
// Hold an owning reference to the HostResolverProc that we are going to use.
// This may not be the current resolver procedure by the time we call
// ResolveAddrInfo, but that's OK... we'll use it anyways, and the owning
@@ -636,36 +592,16 @@ class HostResolverImpl::Job
// This class represents a request to the worker pool for a "probe for IPv6
// support" call.
-class HostResolverImpl::IPv6ProbeJob
- : public base::RefCountedThreadSafe<HostResolverImpl::IPv6ProbeJob> {
+class HostResolverImpl::IPv6ProbeJob : public base::WorkerPoolJob {
public:
explicit IPv6ProbeJob(HostResolverImpl* resolver)
: resolver_(resolver),
- origin_loop_(MessageLoop::current()) {
- DCHECK(!was_cancelled());
+ address_family_(ADDRESS_FAMILY_UNSPECIFIED) {
+ DCHECK(resolver_);
}
- void Start() {
- if (was_cancelled())
- return;
- DCHECK(IsOnOriginThread());
- const bool kIsSlow = true;
- WorkerPool::PostTask(
- FROM_HERE, NewRunnableMethod(this, &IPv6ProbeJob::DoProbe), kIsSlow);
- }
-
- // Cancels the current job.
- void Cancel() {
- if (was_cancelled())
- return;
- DCHECK(IsOnOriginThread());
- resolver_ = NULL; // Read/write ONLY on origin thread.
- {
- AutoLock locked(origin_loop_lock_);
- // Origin loop may be destroyed before we can use it!
- origin_loop_ = NULL; // Write ONLY on origin thread.
- }
- }
+ void Start() { StartJob(); }
+ void Cancel() { CancelJob(); }
private:
friend class base::RefCountedThreadSafe<HostResolverImpl::IPv6ProbeJob>;
@@ -673,57 +609,22 @@ class HostResolverImpl::IPv6ProbeJob
~IPv6ProbeJob() {
}
- // Should be run on |orgin_thread_|, but that may not be well defined now.
- bool was_cancelled() const {
- if (!resolver_ || !origin_loop_) {
- DCHECK(!resolver_);
- DCHECK(!origin_loop_);
- return true;
- }
- return false;
- }
-
// Run on worker thread.
- void DoProbe() {
+ virtual void RunJob() {
// Do actual testing on this thread, as it takes 40-100ms.
- AddressFamily family = IPv6Supported() ? ADDRESS_FAMILY_UNSPECIFIED
- : ADDRESS_FAMILY_IPV4;
-
- Task* reply = NewRunnableMethod(this, &IPv6ProbeJob::OnProbeComplete,
- family);
-
- // The origin loop could go away while we are trying to post to it, so we
- // need to call its PostTask method inside a lock. See ~HostResolver.
- {
- AutoLock locked(origin_loop_lock_);
- if (origin_loop_) {
- origin_loop_->PostTask(FROM_HERE, reply);
- return;
- }
- }
-
- // We didn't post, so delete the reply.
- delete reply;
+ address_family_ = IPv6Supported() ? ADDRESS_FAMILY_UNSPECIFIED
+ : ADDRESS_FAMILY_IPV4;
}
// Callback for when DoProbe() completes (runs on origin thread).
- void OnProbeComplete(AddressFamily address_family) {
- if (was_cancelled())
- return;
- DCHECK(IsOnOriginThread());
- resolver_->IPv6ProbeSetDefaultAddressFamily(address_family);
- }
-
- bool IsOnOriginThread() const {
- return !MessageLoop::current() || origin_loop_ == MessageLoop::current();
+ virtual void OnJobCompleted() {
+ resolver_->IPv6ProbeSetDefaultAddressFamily(address_family_);
}
// Used/set only on origin thread.
- HostResolverImpl* resolver_;
+ HostResolverImpl* const resolver_;
- // Used to post ourselves onto the origin thread.
- Lock origin_loop_lock_;
- MessageLoop* origin_loop_;
+ AddressFamily address_family_;
DISALLOW_COPY_AND_ASSIGN(IPv6ProbeJob);
};
@@ -1230,7 +1131,7 @@ void HostResolverImpl::OnJobCompleteInternal(
// Check if the job was cancelled as a result of running the callback.
// (Meaning that |this| was deleted).
- if (job->was_cancelled())
+ if (job->canceled())
return;
}
}

Powered by Google App Engine
This is Rietveld 408576698