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

base/threading/sequenced_task_runner_handle.cc

Index: base/threading/sequenced_task_runner_handle.cc
diff --git a/base/threading/sequenced_task_runner_handle.cc b/base/threading/sequenced_task_runner_handle.cc
index a03642de42061cef29a2842fee5abbbc9a17df44..b092e1639c2df79c34010675ecd6844b261c6cf2 100644
--- a/base/threading/sequenced_task_runner_handle.cc
+++ b/base/threading/sequenced_task_runner_handle.cc
@@ -6,23 +6,15 @@
 
 #include "base/sequenced_task_runner.h"
 #include "base/thread_task_runner_handle.h"
-#include "base/threading/sequenced_worker_pool.h"

[gab, 2015/11/05 23:29:18]

Still being used.

 
 namespace base {
 
 // static
 scoped_refptr<SequencedTaskRunner> SequencedTaskRunnerHandle::Get() {
-  // If we are on a worker thread for a SequencedBlockingPool that is running a
-  // sequenced task, return a SequencedTaskRunner for it.
-  scoped_refptr<base::SequencedWorkerPool> pool =
-      SequencedWorkerPool::GetWorkerPoolForCurrentThread();
-  if (pool) {
-    SequencedWorkerPool::SequenceToken sequence_token =
-        SequencedWorkerPool::GetSequenceTokenForCurrentThread();
-    DCHECK(sequence_token.IsValid());
-    DCHECK(pool->IsRunningSequenceOnCurrentThread(sequence_token));
-    return pool->GetSequencedTaskRunner(sequence_token);
-  }
+  scoped_refptr<base::SequencedTaskRunner> task_runner =
+      SequencedWorkerPool::GetSequencedTaskRunnerForCurrentThread();
+  if (task_runner)
+    return task_runner;

[gab, 2015/11/05 23:29:18]

How about:

// Return the SequencedTaskRunner if found or the SingleThreadedTaskRunner for
the current thread otherwise.
return task_runner ? task_runner : base::ThreadTaskRunnerHandle::Get();

[Bernhard Bauer, 2015/11/10 17:12:06]

Done, although I had to add an additional constructor call to make the ?: work.

[gab, 2015/11/10 19:54:18]

Oh interesting, without this the implicit conversion works but not with it?

[Bernhard Bauer, 2015/11/10 20:31:47]

Yes, I think the two alternatives in a conditional operator have to be the same
type.

 
   // Otherwise, return a SingleThreadTaskRunner for the current thread.
   return base::ThreadTaskRunnerHandle::Get();

[danakj, 2015/11/05 18:52:08]

I'm wondering if this is a good idea if we shouldn't default to using
SequencedTaskRunnerHandle::Get() instead of ThreadTaskRunnerHandle::Get().

I'd like to see that conversation resolved.. maybe it was? It didn't feel like
it to me. And maybe update the guidance comment in this CL accordingly.

[gab, 2015/11/05 23:29:18]

Hmmm? SequencedTaskRunnerHandle::Get() here would result in an infinite
recursion loop. The discussion was more around whether the preference towards
SequencedTaskRunnerHandle highlighted in thread_task_runner_handle.h is correct.

[danakj, 2015/11/05 23:31:15]

Oh I meant, if this isn't a general "always use that", then it's more of a "I
know/expect to be part of a sequenced worker pool" and maybe this should return
null, instead of ThreadTaskRunnerHandle, if it's not.

[gab, 2015/11/05 23:33:10]

Oh I see.

I don't think so. I think returning a SingleThreadedTaskRunner here as a
fallback is always the right thing to do.

The debate is more whether to prefer the loose API or the strict API from the
caller's standpoint I think.

@@ -30,8 +22,7 @@ scoped_refptr<SequencedTaskRunner> SequencedTaskRunnerHandle::Get() {
 
 // static
 bool SequencedTaskRunnerHandle::IsSet() {
-  return (SequencedWorkerPool::GetWorkerPoolForCurrentThread() &&
-          SequencedWorkerPool::GetSequenceTokenForCurrentThread().IsValid()) ||
+  return SequencedWorkerPool::GetSequencedTaskRunnerForCurrentThread() ||
          base::ThreadTaskRunnerHandle::IsSet();
 }