Create base::WorkerPoolJob. Use it for HostResolverImpl and DirectoryLister.

base/worker_pool_job.cc

Index: base/worker_pool_job.cc
diff --git a/base/worker_pool_job.cc b/base/worker_pool_job.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1a6c16e5b47dd00b83acc27e8b8e195e70f356b0
--- /dev/null
+++ b/base/worker_pool_job.cc
@@ -0,0 +1,135 @@
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/worker_pool_job.h"
+
+#include "base/logging.h"
+#include "base/message_loop_proxy.h"
+#include "base/worker_pool.h"
+
+namespace base {
+
+WorkerPoolJob::WorkerPoolJob(bool is_slow)
+    : origin_loop_(base::MessageLoopProxy::CreateForCurrentThread()),
+      is_slow_(is_slow),
+      canceled_(false),
+      state_(NONE) {}
+
+WorkerPoolJob::~WorkerPoolJob() {}
+
+void WorkerPoolJob::StartJob() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_EQ(state_, NONE);
+  state_ = RUNNING;
+  WorkerPool::PostTask(

[eroman, 2010/12/22 00:51:47]

PostTask can fail. Please handle that case (at the very least by crashing).

[willchan no longer on Chromium, 2010/12/22 02:04:01]

Done.

+      FROM_HERE,
+      NewRunnableMethod(this, &WorkerPoolJob::RunJobOnWorkerPool),
+      is_slow_);
+}
+
+void WorkerPoolJob::CancelJob() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  {
+    AutoLock auto_lock(lock_);
+    DCHECK_GT(state_, NONE);
+    DCHECK_LT(state_, DONE);
+    DCHECK(!canceled_);
+    canceled_ = true;
+  }
+}
+
+bool WorkerPoolJob::canceled() const {
+  // No thread check since RunJob() implementations may check to see if they've
+  // been canceled so they can bail out early.
+  AutoLock auto_lock(lock_);
+  DCHECK_NE(state_, NONE);
+  return canceled_;
+}
+
+bool WorkerPoolJob::is_running() const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  AutoLock auto_lock(lock_);
+  DCHECK_NE(state_, NONE);
+  return state_ == RUNNING;
+}
+
+bool WorkerPoolJob::started() const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  AutoLock auto_lock(lock_);
+  return state_ != NONE;
+}
+
+bool WorkerPoolJob::done() const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  AutoLock auto_lock(lock_);
+  return state_ > FINISHING;
+}
+
+void WorkerPoolJob::RunJobOnWorkerPool() {
+  DCHECK(!thread_checker_.CalledOnValidThread());
+
+  // Optimization to avoid work if we're already canceled.
+  bool canceled = false;
+  {
+    AutoLock auto_lock(lock_);

[eroman, 2010/12/22 00:51:47]

This optimization doesn't seem worthwhile. I don't think it would be common at
all to post a task, and then cancel it so quickly that it didn't even get a
chance to run yet.

IMO we are better off just acquiring the lock once when done

[willchan no longer on Chromium, 2010/12/22 02:04:01]

OK.

+    DCHECK_EQ(state_, RUNNING);
+    canceled = canceled_;
+  }
+
+  if (canceled) {
+    OnCanceled();
+    {
+      AutoLock auto_lock(lock_);
+      state_ = CANCELED_ON_WORKER;
+    }
+    return;
+  }
+
+  RunJob();
+
+  // Optimization to avoid posting to origin loop if we're already canceled.
+  {
+    AutoLock auto_lock(lock_);
+    canceled = canceled_;
+  }
+
+  if (canceled) {
+    OnCanceled();
+    {
+      AutoLock auto_lock(lock_);
+      state_ = CANCELED_ON_WORKER;
+    }
+    return;
+  }
+
+  {
+    AutoLock auto_lock(lock_);
+    state_ = FINISHING;
+  }
+
+  origin_loop_->PostTask(

[eroman, 2010/12/22 00:51:47]

This is racy.

Access to the origin loop MUST be protected within a lock. Since it is possible
for Cancel() to be called from the origin loop right before we call PostTask.

And Cancel() is basically called to prevent the completion from reaching the
origin loop (since we may be deleting it).

[willchan no longer on Chromium, 2010/12/22 02:04:01]

I don't believe it's racy.  |origin_loop_| is a MessageLoopProxy, so it won't
crash on the PostTask() even if CancelJob() is called. 
CompleteJobOnOriginLoop() will check |canceled_| before calling the
CompleteJob() private virtual.  So cancellation seems to me like it'll still
work.

[eroman, 2010/12/22 04:58:48]

Oh, my bad! I didn't realize origin_loop_ was a MessageLoopProxy (when I read
through this I assumed it was a MessageLoop)

+      FROM_HERE,
+      NewRunnableMethod(this, &WorkerPoolJob::CompleteJobOnOriginLoop));
+}
+
+void WorkerPoolJob::CompleteJobOnOriginLoop() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  // No need to lock, since the only relevant writes happen on the worker
+  // thread, and posting a task will invoke a memory barrier, so those writes
+  // are guaranteed to be visible.
+  DCHECK_EQ(state_, FINISHING);
+
+  // No need to lock, since writes only happen on origin loop.
+  if (canceled_) {
+    OnCanceled();
+    state_ = DONE;
+    return;
+  }
+
+  CompleteJob();
+
+  state_ = DONE;
+}
+
+}  // namespace base