Make SequencedWorkerPool::IsRunningSequenceOnCurrentThread() private.

base/threading/sequenced_worker_pool_unittest.cc

Index: base/threading/sequenced_worker_pool_unittest.cc
diff --git a/base/threading/sequenced_worker_pool_unittest.cc b/base/threading/sequenced_worker_pool_unittest.cc
index 18270a6f4f913bea794be5c70d4c710f8b291212..7abdc730d4dfa1a55c170ebeb7fb56bab359760a 100644
--- a/base/threading/sequenced_worker_pool_unittest.cc
+++ b/base/threading/sequenced_worker_pool_unittest.cc
@@ -769,55 +769,51 @@ TEST_F(SequencedWorkerPoolTest, SpuriousWorkSignal) {
   EXPECT_EQ(old_has_work_call_count + 1, has_work_call_count());
 }
 
-void IsRunningOnCurrentThreadTask(
-    SequencedWorkerPool::SequenceToken test_positive_token,
-    SequencedWorkerPool::SequenceToken test_negative_token,
+void VerifyRunsTasksOnCurrentThread(
+    scoped_refptr<TaskRunner> test_positive_task_runner,
+    scoped_refptr<TaskRunner> test_negative_task_runner,
     SequencedWorkerPool* pool,
     SequencedWorkerPool* unused_pool) {
+  EXPECT_TRUE(test_positive_task_runner->RunsTasksOnCurrentThread());
+  EXPECT_FALSE(test_negative_task_runner->RunsTasksOnCurrentThread());
   EXPECT_TRUE(pool->RunsTasksOnCurrentThread());
-  EXPECT_TRUE(pool->IsRunningSequenceOnCurrentThread(test_positive_token));
-  EXPECT_FALSE(pool->IsRunningSequenceOnCurrentThread(test_negative_token));
   EXPECT_FALSE(unused_pool->RunsTasksOnCurrentThread());
-  EXPECT_FALSE(
-      unused_pool->IsRunningSequenceOnCurrentThread(test_positive_token));
-  EXPECT_FALSE(
-      unused_pool->IsRunningSequenceOnCurrentThread(test_negative_token));
 }
 
-// Verify correctness of the IsRunningSequenceOnCurrentThread method.
-TEST_F(SequencedWorkerPoolTest, IsRunningOnCurrentThread) {
-  SequencedWorkerPool::SequenceToken token1 = pool()->GetSequenceToken();
-  SequencedWorkerPool::SequenceToken token2 = pool()->GetSequenceToken();
-  SequencedWorkerPool::SequenceToken unsequenced_token;
+// Verify correctness of the RunsTasksOnCurrentThread() method on
+// SequencedWorkerPool and on TaskRunners it returns.
+TEST_F(SequencedWorkerPoolTest, RunsTasksOnCurrentThread) {
+  const scoped_refptr<SequencedTaskRunner> sequenced_task_runner_1 =
+      pool()->GetSequencedTaskRunner(SequencedWorkerPool::GetSequenceToken());
+  const scoped_refptr<SequencedTaskRunner> sequenced_task_runner_2 =
+      pool()->GetSequencedTaskRunner(SequencedWorkerPool::GetSequenceToken());
+  const scoped_refptr<TaskRunner> unsequenced_task_runner =
+      pool()->GetTaskRunnerWithShutdownBehavior(
+          SequencedWorkerPool::BLOCK_SHUTDOWN);
 
   SequencedWorkerPoolOwner unused_pool_owner(2, "unused_pool");
 
   EXPECT_FALSE(pool()->RunsTasksOnCurrentThread());
-  EXPECT_FALSE(pool()->IsRunningSequenceOnCurrentThread(token1));
-  EXPECT_FALSE(pool()->IsRunningSequenceOnCurrentThread(token2));
-  EXPECT_FALSE(pool()->IsRunningSequenceOnCurrentThread(unsequenced_token));
+  EXPECT_FALSE(sequenced_task_runner_1->RunsTasksOnCurrentThread());
+  EXPECT_FALSE(sequenced_task_runner_2->RunsTasksOnCurrentThread());
+  EXPECT_FALSE(unsequenced_task_runner->RunsTasksOnCurrentThread());
   EXPECT_FALSE(unused_pool_owner.pool()->RunsTasksOnCurrentThread());
-  EXPECT_FALSE(
-      unused_pool_owner.pool()->IsRunningSequenceOnCurrentThread(token1));
-  EXPECT_FALSE(
-      unused_pool_owner.pool()->IsRunningSequenceOnCurrentThread(token2));
-  EXPECT_FALSE(unused_pool_owner.pool()->IsRunningSequenceOnCurrentThread(
-      unsequenced_token));
 
-  pool()->PostSequencedWorkerTask(
-      token1, FROM_HERE,
-      base::Bind(&IsRunningOnCurrentThreadTask, token1, token2,
-                 base::RetainedRef(pool()),
+  sequenced_task_runner_1->PostTask(

[danakj, 2016/09/15 18:23:42]

Can you add a comment on each of these 3 PostTasks just explaining what
condition it is or isn't checking? It's rather indirect, esp getting subtle wrt
unsequenced_task_runner.

[fdoray, 2016/09/15 18:34:36]

Done.

I removed the second PostTask() since it doesn't test anything different than
the first one (just replace 1 <--> 2).

+      FROM_HERE,
+      base::Bind(&VerifyRunsTasksOnCurrentThread, sequenced_task_runner_1,
+                 sequenced_task_runner_2, base::RetainedRef(pool()),
                  base::RetainedRef(unused_pool_owner.pool())));
-  pool()->PostSequencedWorkerTask(
-      token2, FROM_HERE,
-      base::Bind(&IsRunningOnCurrentThreadTask, token2, unsequenced_token,
-                 base::RetainedRef(pool()),
+  sequenced_task_runner_2->PostTask(
+      FROM_HERE,
+      base::Bind(&VerifyRunsTasksOnCurrentThread, sequenced_task_runner_2,
+                 sequenced_task_runner_1, base::RetainedRef(pool()),

[gab, 2016/09/15 16:34:39]

This maps unsequenced_token -> sequenced_task_runner_1? Shouldn't that be
unsequenced_task_runner?

[fdoray, 2016/09/15 16:47:15]

If we do that, we'll verify that
unsequenced_task_runner->RunsTasksOnCurrentThread() returns false when called
from a task posted to |sequenced_task_runner_2|. However, it returns true.

unsequenced_task_runner->RunsTasksOnCurrentThread() is not equivalent to
pool()->IsRunningSequenceOnCurrentThread(unsequenced_task_runner).

The first call returns true if |unsequenced_task_runner| may run tasks on the
current thread. The second call returns true if the current thread is running an
unsequenced task.

[gab, 2016/09/15 17:27:07]

Ah, got it, thanks!

+                 base::RetainedRef(unused_pool_owner.pool())));
+  unsequenced_task_runner->PostTask(
+      FROM_HERE,
+      base::Bind(&VerifyRunsTasksOnCurrentThread, unsequenced_task_runner,
+                 sequenced_task_runner_1, base::RetainedRef(pool()),
                  base::RetainedRef(unused_pool_owner.pool())));
-  pool()->PostWorkerTask(
-      FROM_HERE, base::Bind(&IsRunningOnCurrentThreadTask, unsequenced_token,
-                            token1, base::RetainedRef(pool()),
-                            base::RetainedRef(unused_pool_owner.pool())));
 }
 
 // Checks that tasks are destroyed in the right context during shutdown. If a