Implement SequenceChecker, which is a generalization of ThreadChecker

base/sequence_checker_impl.cc

+// Copyright (c) 2012 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/sequence_checker_impl.h"
+
+#include "base/sequenced_task_runner.h"
+
+namespace base {
+
+SequenceCheckerImpl::SequenceCheckerImpl(
+    const scoped_refptr<SequencedTaskRunner>& sequenced_task_runner)
+    : sequenced_task_runner_(sequenced_task_runner) {}
+
+SequenceCheckerImpl::~SequenceCheckerImpl() {}
+
+bool SequenceCheckerImpl::CalledOnValidSequence() const {
+  AutoLock auto_lock(lock_);
+  return sequenced_task_runner_.get() ?
+      sequenced_task_runner_->RunsTasksOnCurrentThread() :

[michaeln, 2012/12/21 22:07:31]

Hmmm... as coded right now, the base::Bind closures and parameters are deleted
outside of the scope of IsRunningSequenceOnCurrentThread.

          this_worker->set_running_task_info(
              SequenceToken(task.sequence_token_id), task.shutdown_behavior);

          task.task.Run();

          this_worker->set_running_task_info(
              SequenceToken(), CONTINUE_ON_SHUTDOWN);

          task.task = Closure();  // <---- Closures killed here.

I haven't looked much at your CLs for this, but I think this might be wrinkle in
your intended use of of SequenceChecker? I don't recall exactly why closures are
cleared outside of the scope.

[akalin, 2012/12/21 23:11:51]

Hmm that may indeed be a problem.  I'll look into it and fix it in another CL.

[jar (doing other things), 2012/12/21 23:50:41]

It is always good hygiene to do the least possible work within a locked region. 
Especially notable is to avoid calling arbitrary functions from within a locked
region.  A destructor is generally one such example.  Often the pattern is to
take local ownership of a an instance that you are about to delete, release the
lock, and then instigate the destruction. 

On 2012/12/21 22:07:31, michaeln wrote:

[akalin, 2012/12/22 00:00:04]

But that entire block of code (that michaeln@ mentioned) is within an AutoUnlock
block.  So I think moving the closure-killing before the set_running_task_info
is pretty safe.

On 2012/12/21 23:50:41, jar wrote:

[michaeln, 2012/12/22 00:21:53]

This IsRunningSequenceOnCurrentThread went in with this old CL...
  https://chromiumcodereview.appspot.com/9845037

The rationale jim mentioned is probably what I had in mind (even though this
isn't a lock exactly). Also I vaguely remember thinking about refcounted object
writers making assertions about dtor thread and thinking they really shouldn't
be depending on base::Bind closures to drop that ref on the right thread.

I think closure dtor'ing could be moved inside the scope safely if it helps with
WeakPtr instrumentation. Lets ask the trybots.

[jar (doing other things), 2012/12/26 22:39:08]

My mistake.  I conflated the comment about *distant* code, with the code above. 
My comments are completely off... the code is releasing the lock before allowing
all these calls to go wild.

On 2012/12/22 00:21:53, michaeln wrote:

+      thread_checker_.CalledOnValidThread();
+}
+
+void SequenceCheckerImpl::ChangeSequence(
+    const scoped_refptr<SequencedTaskRunner>& sequenced_task_runner) {
+  AutoLock auto_lock(lock_);
+  sequenced_task_runner_ = sequenced_task_runner;
+  thread_checker_.DetachFromThread();
+}
+
+}  // namespace base