Prefer returning a SingleThreadTaskRunner in SequencedTaskRunnerHandle::Get().

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 53f3261d9f7c914982cc7992aa356ffb0b81e2e2..7d08fbd6d47eb6aa32ef0073bde383f2486ee73b 100644
--- a/base/threading/sequenced_task_runner_handle.cc
+++ b/base/threading/sequenced_task_runner_handle.cc
@@ -16,45 +16,51 @@ namespace base {
 
 namespace {
 
-base::LazyInstance<base::ThreadLocalPointer<SequencedTaskRunnerHandle>>::Leaky
+LazyInstance<ThreadLocalPointer<SequencedTaskRunnerHandle>>::Leaky
     lazy_tls_ptr = LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
 // static
 scoped_refptr<SequencedTaskRunner> SequencedTaskRunnerHandle::Get() {
+  // Return the registered SingleThreadTaskRunner, if any. This must be at the
+  // top so that a SingleThreadTaskRunner has priority over a
+  // SequencedTaskRunner (RLZ registers both on the same thread despite that
+  // being prevented by DCHECKs).
+  // TODO(fdoray): Move this to the bottom once RLZ stops registering a
+  // SingleThreadTaskRunner and a SequencedTaskRunner on the same thread.
+  // https://crbug.com/618530

[gab, 2017/02/24 16:34:41]

https://crbug.com/618530#c14 (to point directly at relevant context)

[fdoray, 2017/02/24 17:14:00]

Done.

+  if (ThreadTaskRunnerHandle::IsSet())
+    return ThreadTaskRunnerHandle::Get();
+
   // Return the registered SequencedTaskRunner, if any.
   const SequencedTaskRunnerHandle* handle = lazy_tls_ptr.Pointer()->Get();
   if (handle) {
     // Various modes of setting SequencedTaskRunnerHandle don't combine.
-    DCHECK(!base::ThreadTaskRunnerHandle::IsSet());
+    DCHECK(!ThreadTaskRunnerHandle::IsSet());

[gab, 2017/02/24 16:34:41]

That will prevent this DCHECK from being hit (i.e. allows more problems to crop
up)...

DCHECK(!lazy_tls_ptr.Pointer()->Get() &&
!SequencedWorkerPool::GetWorkerPoolForCurrentThread()) in early return above?
Will keep checking in RLZ but I don't know that we want to remove that check...

[fdoray, 2017/02/24 17:14:00]

Done.

     DCHECK(!SequencedWorkerPool::GetSequenceTokenForCurrentThread().IsValid());
     return handle->task_runner_;
   }
 
   // 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 =
+  scoped_refptr<SequencedWorkerPool> pool =
       SequencedWorkerPool::GetWorkerPoolForCurrentThread();
-  if (pool) {
-    SequencedWorkerPool::SequenceToken sequence_token =
-        SequencedWorkerPool::GetSequenceTokenForCurrentThread();
-    DCHECK(sequence_token.IsValid());
-    scoped_refptr<SequencedTaskRunner> sequenced_task_runner(
-        pool->GetSequencedTaskRunner(sequence_token));
-    DCHECK(sequenced_task_runner->RunsTasksOnCurrentThread());
-    return sequenced_task_runner;

[gab, 2017/02/24 16:34:41]

Interesting: just realized this didn't DCHECK !TTRH::IsSet(): probably explains
why RLZ hits this more recently (i.e. it's hit behind task_scheduler redirection
only).

[fdoray, 2017/02/24 17:14:00]

No need to add a DCHECK now as it would never be hit.

[gab, 2017/02/24 17:44:36]

Agreed, just an observation.

-  }
-
-  // Return the SingleThreadTaskRunner for the current thread otherwise.
-  return base::ThreadTaskRunnerHandle::Get();
+  DCHECK(pool);
+  SequencedWorkerPool::SequenceToken sequence_token =
+      SequencedWorkerPool::GetSequenceTokenForCurrentThread();
+  DCHECK(sequence_token.IsValid());
+  scoped_refptr<SequencedTaskRunner> sequenced_task_runner(
+      pool->GetSequencedTaskRunner(sequence_token));
+  DCHECK(sequenced_task_runner->RunsTasksOnCurrentThread());
+  return sequenced_task_runner;
 }
 
 // static
 bool SequencedTaskRunnerHandle::IsSet() {
   return lazy_tls_ptr.Pointer()->Get() ||
          SequencedWorkerPool::GetSequenceTokenForCurrentThread().IsValid() ||
-         base::ThreadTaskRunnerHandle::IsSet();
+         ThreadTaskRunnerHandle::IsSet();
 }
 
 SequencedTaskRunnerHandle::SequencedTaskRunnerHandle(