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

base/worker_pool_job.h

+// 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.
+
+#ifndef BASE_WORKER_POOL_JOB_H_
+#define BASE_WORKER_POOL_JOB_H_
+#pragma once
+
+#include "base/basictypes.h"
+#include "base/lock.h"
+#include "base/message_loop_proxy.h"
+#include "base/ref_counted.h"
+#include "base/thread_checker.h"
+
+class MessageLoop;

[eroman, 2010/12/22 23:25:08]

this is not used.

[willchan no longer on Chromium, 2010/12/24 00:47:34]

Done.

+
+namespace base {
+
+// WARNING: Are you sure you want to use WorkerPool in the first place?  The
+// thread will _not_ be joined, so the job may still be running during shutdown.
+// Global objects that are deleted by AtExitManager should not be accessed on
+// the WorkerPool thread, otherwise there is potential for shutdown crashes.
+//
+// WorkerPoolJob is a helper class for implementing jobs that get created on an
+// origin thread, get posted to run on a WorkerPool thread, and then run back on
+// the origin thread upon completion.  WorkerPoolJob also handles cancellation
+// of a job which prevents it from calling back.  Note that we use inheritance
+// rather than composition here, since WorkerPoolJob also handles lifetime
+// management via refcounting.
+class WorkerPoolJob : public base::RefCountedThreadSafe<WorkerPoolJob> {
+ public:
+  // ScopedHandle is provided to automate cancellation of the WorkerPoolJob.
+  template <typename JobType>
+  class ScopedHandle {
+   public:
+    explicit ScopedHandle(JobType* job) : job_(job) {}
+
+    ~ScopedHandle() {

[eroman, 2010/12/22 23:25:08]

Can't this simply be job_->CancelJob() ?

[willchan no longer on Chromium, 2010/12/24 00:47:34]

No.  I am a bit anal about cancellation.  I want people only to call CancelJob()
when they have something to cancel.  ScopedHandle reads into the private state
to see if it needs to cancel or not.  I'm fine with ScopedHandle doing that
since people usually aren't canceling themselves if they're using the handle. 
But if they are doing things manually, then I want them to cancel only once. 
Multiple invocations is usually a sign of a bug or arguably bad design.

[eroman, 2011/01/05 01:24:52]

FWIW we use the pattern of calling Cancel() on already cancelled objects in
other places. Especially in destructors which blanket cancel stuff without
checking if they have already been cancelled.

I am ok with this though.

+      AutoLock auto_lock(job_->lock_);
+      if (!job_->canceled_ && job_->state_ > NONE &&
+          job_->state_ < CANCELED_ON_WORKER)
+        job_->canceled_ = true;
+    }
+
+    JobType* job() const { return job_.get(); }
+
+   private:
+    const scoped_refptr<JobType> job_;
+    DISALLOW_COPY_AND_ASSIGN(ScopedHandle);
+  };
+
+ protected:
+  WorkerPoolJob();
+
+  // This may be deleted on the origin or worker thread.
+  virtual ~WorkerPoolJob();
+
+  // Starts the job.  Will post itself to the WorkerPool, where it will invoke
+  // RunJob().  After RunJob() runs, it will post itself back to the origin
+  // thread, where it will invoke CompleteJob().  This can only be called once
+  // and must be called on the origin thread.
+  void StartJob();
+
+  // Cancels the job.  CompleteJob() is guaranteed not to be called when this
+  // happens.  May only be called once, and only on the origin thread.
+  void CancelJob();
+
+  // Returns true if CancelJob() has been called.
+  bool canceled() const;
+
+  // Returns true if the job is running on the worker thread.  Obviously if it
+  // returns true once, there's still no guarantee that it is still running
+  // after it returns.  Once it returns false though, then it's guaranteed to
+  // stay false.  is_running() is only allowed to be called on the origin
+  // thread.
+  bool is_running() const;

[eroman, 2010/12/22 23:25:08]

This seems like it should be only expose to unit-tests, as I don't think we want
production code to rely on this.

[willchan no longer on Chromium, 2010/12/24 00:47:34]

Done.

+
+  // NOTE(willchan): I've restricted this to unit tests only, primarily because
+  // I haven't found a good reason for allowing subtypes to access this.  If
+  // there is a motivating reason to do so, I am open to it.
+#if defined(UNIT_TEST)
+  scoped_refptr<MessageLoopProxy> origin_loop() const {
+    return origin_loop_;
+  }
+#endif  // defined(UNIT_TEST)
+
+ private:
+  friend class base::RefCountedThreadSafe<WorkerPoolJob>;
+  template <typename JobType> friend class ScopedHandle;

[eroman, 2010/12/22 23:25:08]

See earlier comment, I don't think we need to friend here if it just calls
CancelJob().

[willchan no longer on Chromium, 2010/12/24 00:47:34]

Done.

+
+  // State machine, primarily for enforcing that certain events only happen
+  // once.  Also lets us implement is_running().
+  enum State {
+    NONE,
+    RUNNING,
+    FINISHING,
+    CANCELED_ON_WORKER,
+    DONE,
+  };
+
+  // API for subtypes to implement.
+
+  // RunJob() runs on the worker thread.  If CancelJob() is called on the origin
+  // thread , then it may never be invoked, but that's a race.
+  virtual void RunJob() = 0;
+
+  // CompleteJob() runs on the origin thread.  If CancelJob() is called, then it
+  // may never be invoked.
+  virtual void CompleteJob() = 0;

[eroman, 2010/12/22 23:25:08]

I think this would be better named OnJobCompleted(), since it is more of a
delegate method than it is an action method.

[willchan no longer on Chromium, 2010/12/24 00:47:34]

Done.

+
+  void RunJobOnWorkerPool();
+  void CompleteJobOnOriginLoop();
+
+  // Returns true after StartJob() has been called.  Only valid to be called on
+  // the origin thread.
+  bool started() const;

[eroman, 2010/12/22 23:25:08]

Is this needed other than for unittests?

[willchan no longer on Chromium, 2010/12/24 00:47:34]

Removed

+
+  // Returns true after job completes successfully or has been canceled.
+  bool done() const;
+
+  ThreadChecker thread_checker_;
+  const scoped_refptr<MessageLoopProxy> origin_loop_;
+
+  mutable Lock lock_;  // Protects all member variables below.
+  bool canceled_;
+  State state_;
+
+  DISALLOW_COPY_AND_ASSIGN(WorkerPoolJob);
+};
+
+}  // namespace base
+
+#endif  // BASE_WORKER_POOL_JOB_H_