Allow SequencedTaskRunnerHandle::Get() while running unsequenced tasks.

base/threading/sequenced_worker_pool.cc

Index: base/threading/sequenced_worker_pool.cc
diff --git a/base/threading/sequenced_worker_pool.cc b/base/threading/sequenced_worker_pool.cc
index ed79897fd450f5e796bc9db5e79ffeeb59eed2ab..6e88877437f69e07a827bb0918eaa2517d085379 100644
--- a/base/threading/sequenced_worker_pool.cc
+++ b/base/threading/sequenced_worker_pool.cc
@@ -296,7 +296,7 @@ class SequencedWorkerPool::Inner {
 
   ~Inner();
 
-  SequenceToken GetSequenceToken();
+  static SequenceToken GetSequenceToken();
 
   SequenceToken GetNamedSequenceToken(const std::string& name);
 
@@ -314,6 +314,9 @@ class SequencedWorkerPool::Inner {
 
   bool IsRunningSequenceOnCurrentThread(SequenceToken sequence_token) const;
 
+  void SetRunningTaskInfoForCurrentThread(SequenceToken sequence_token,
+                                          WorkerShutdown shutdown_behavior);
+
   void CleanupForTesting();
 
   void SignalHasWorkForTesting();
@@ -583,6 +586,7 @@ SequencedWorkerPool::Inner::~Inner() {
     testing_observer_->OnDestruct();
 }
 
+// static
 SequencedWorkerPool::SequenceToken
 SequencedWorkerPool::Inner::GetSequenceToken() {
   // Need to add one because StaticAtomicSequenceNumber starts at zero, which
@@ -683,6 +687,17 @@ bool SequencedWorkerPool::Inner::IsRunningSequenceOnCurrentThread(
          sequence_token.Equals(found->second->task_sequence_token());
 }
 
+void SequencedWorkerPool::Inner::SetRunningTaskInfoForCurrentThread(
+    SequenceToken sequence_token,
+    WorkerShutdown shutdown_behavior) {
+  AutoLock lock(lock_);
+  ThreadMap::const_iterator found = threads_.find(PlatformThread::CurrentId());
+  DCHECK(found != threads_.end());
+  DCHECK(found->second->is_processing_task());
+  DCHECK(!found->second->task_sequence_token().IsValid());
+  found->second->set_running_task_info(sequence_token, shutdown_behavior);
+}
+
 // See https://code.google.com/p/chromium/issues/detail?id=168415
 void SequencedWorkerPool::Inner::CleanupForTesting() {
   DCHECK(!RunsTasksOnCurrentThread());
@@ -1181,13 +1196,24 @@ SequencedWorkerPool::GetSequenceTokenForCurrentThread() {
 }
 
 // static
-scoped_refptr<SequencedWorkerPool>
-SequencedWorkerPool::GetWorkerPoolForCurrentThread() {
+scoped_refptr<SequencedTaskRunner>
+SequencedWorkerPool::GetSequencedTaskRunnerForCurrentThread() {
   Worker* worker = Worker::GetForCurrentThread();
   if (!worker)
     return nullptr;
 
-  return worker->worker_pool();
+  scoped_refptr<SequencedWorkerPool> pool = worker->worker_pool();
+  SequenceToken sequence_token = worker->task_sequence_token();
+  WorkerShutdown shutdown_behavior = worker->task_shutdown_behavior();

[danakj, 2015/11/17 22:03:39]

Can you DCHECK that is_processing_task() is true?

[Bernhard Bauer, 2015/11/18 13:15:50]

task_shutdown_behavior() already does that. Alternatively, we could put the
sequence token and shutdown behavior into a struct and keep a scoped_ptr to it
that we reset in reset_running_task_info()?

[danakj, 2015/11/20 19:18:43]

Can you add a comment at the !worker early out explaining why if worker exists,
we know we're inside/processing a sequenced task? (Cuz worker is only non-null
inside WorkerPool::Run(), which will run sequenced tasks and does not call
GetSequencedTaskRunnerForCurrentThread itself?)

[Bernhard Bauer, 2015/12/10 16:09:58]

Done, modulo s/sequenced //, because a worker thread will run both sequenced and
unsequenced tasks. If it is running a sequenced task, we can just return a
SequencedWorkerPoolSequencedTaskRunner for the current sequence token; if it is
running an unsequenced task, we have to do the dance below.

+  if (!sequence_token.IsValid()) {
+    sequence_token = Inner::GetSequenceToken();
+    pool->inner_->SetRunningTaskInfoForCurrentThread(sequence_token,

[danakj, 2015/11/17 22:03:39]

Why do you do this? Now the thread is left in a state where it's bound to this
sequence after calling this. Normally we reset it to no sequence when you finish
a task. What will clean that up? What is going to use what you're setting here?
You're not going to Run() a task here.

[Bernhard Bauer, 2015/11/18 13:15:50]

I do this to make sure that a task posted to the new SequencedTaskRunner will
not start running immediately on another thread... but now that you mention it,
I did forget to mark the sequence token as in use, so I fixed that. We still
need to set these so that calling this twice in a row will return the same
sequence.

Once the task is done, the task info will be reset (the old sequence token will
stay around, but |is_processing_task_| will be false, which guards all accesses;
see also my comment above).

[danakj, 2015/11/20 19:18:43]

Can you encode this into a comment in here?

And can you add a unit test that would fail without your fix mentioned here?

[Bernhard Bauer, 2015/12/10 16:09:58]

Done.
I tried that, but it's tricky, because I can't really verify that a task posted
to the new SequencedTaskRunner will *not* immediately start running -- I can
fail if it does, but if the tasks happen to run in sequence, it doesn't
necessarily mean that wasn't just a coincidence.

I could post a task to the new SequencedTaskRunner and verify that it tries to
signal an existing thread that there is work instead of starting a new thread,
which indicates that the task isn't runnable yet, but that also seems like it
relies on a bunch of implementation details of the SWP... Do you have any better
ideas?

[danakj, 2015/12/16 21:53:32]

What about just calling it twice and making sure you get the same sequence token
and that IsSequenceTokenRunnable() is true?

+                                                     shutdown_behavior);
+  }
+
+  DCHECK(pool->IsRunningSequenceOnCurrentThread(sequence_token));
+  return new SequencedWorkerPoolSequencedTaskRunner(pool, sequence_token,

[danakj, 2015/11/17 22:03:39]

can you move() the pool smart pointer? no need to keep a ref here.

[Bernhard Bauer, 2015/11/18 13:15:50]

Done.

+                                                    shutdown_behavior);
 }
 
 SequencedWorkerPool::SequencedWorkerPool(size_t max_threads,
@@ -1206,8 +1232,7 @@ SequencedWorkerPool::SequencedWorkerPool(size_t max_threads,
 SequencedWorkerPool::~SequencedWorkerPool() {}
 
 void SequencedWorkerPool::OnDestruct() const {
-  // Avoid deleting ourselves on a worker thread (which would
-  // deadlock).
+  // Avoid deleting ourselves on a worker thread (which would deadlock).
   if (RunsTasksOnCurrentThread()) {
     constructor_task_runner_->DeleteSoon(FROM_HERE, this);
   } else {
@@ -1215,8 +1240,9 @@ void SequencedWorkerPool::OnDestruct() const {
   }
 }
 
+// static
 SequencedWorkerPool::SequenceToken SequencedWorkerPool::GetSequenceToken() {
-  return inner_->GetSequenceToken();
+  return Inner::GetSequenceToken();
 }
 
 SequencedWorkerPool::SequenceToken SequencedWorkerPool::GetNamedSequenceToken(