Test SequencedWorkerPool with redirection to the TaskScheduler.

This CL makes most SequencedWorkerPoolTest a TestWithParam. The parameter
controls whether redirection to TaskScheduler is enabled.

BUG=622400

Review-Url: https://codereview.chromium.org/2285633003
Cr-Commit-Position: refs/heads/master@{#419322}

base/threading/sequenced_worker_pool.cc

@@ -54,12 +54,16 @@ namespace base {
 
 namespace {
 
-// An enum representing the state of all pools. Any given process should only
-// ever transition from NONE_ACTIVE to the active states, transitions between
-// actives states are unexpected. The REDIRECTED_TO_TASK_SCHEDULER transition
-// occurs when RedirectSequencedWorkerPoolsToTaskSchedulerForProcess() is called
-// and the WORKER_CREATED transition occurs when a Worker needs to be created
-// because the first task was posted and the state is still NONE_ACTIVE.
+// An enum representing the state of all pools. Any given non-test process
+// should only ever transition from NONE_ACTIVE to one of the active states.
+// Transitions between actives states are unexpected. The
+// REDIRECTED_TO_TASK_SCHEDULER transition occurs when
+// RedirectToTaskSchedulerForProcess() is called. The WORKER_CREATED transition
+// occurs when a Worker needs to be created because the first task was posted
+// and the state is still NONE_ACTIVE. In a test process, a transition to
+// NONE_ACTIVE occurs when ResetRedirectToTaskSchedulerForProcessForTesting() is
+// called.
+//
 // |g_all_pools_state| uses relaxed atomic operations to ensure no data race
 // between reads/writes, strict memory ordering isn't required per no other
 // state being inferred from its value. Explicit synchronization (e.g. locks or
@@ -67,6 +71,7 @@ namespace {
 // NONE_ACTIVE after the first Worker was created -- this is not possible for
 // REDIRECTED_TO_TASK_SCHEDULER per its API requesting to be invoked while no
 // other threads are active).
+//
 // TODO(gab): Remove this if http://crbug.com/622400 fails (SequencedWorkerPool
 // will be phased out completely otherwise).
 enum AllPoolsState : subtle::Atomic32 {
@@ -902,6 +907,8 @@ bool SequencedWorkerPool::Inner::IsRunningSequenceOnCurrentThread(
 
 // See https://code.google.com/p/chromium/issues/detail?id=168415
 void SequencedWorkerPool::Inner::CleanupForTesting() {
+  DCHECK_NE(subtle::NoBarrier_Load(&g_all_pools_state),
+            AllPoolsState::REDIRECTED_TO_TASK_SCHEDULER);
   DCHECK(!RunsTasksOnCurrentThread());
   base::ThreadRestrictions::ScopedAllowWait allow_wait;
   AutoLock lock(lock_);
@@ -1449,8 +1456,7 @@ SequencedWorkerPool::GetWorkerPoolForCurrentThread() {
 }
 
 // static
-void SequencedWorkerPool::
-    RedirectSequencedWorkerPoolsToTaskSchedulerForProcess() {
+void SequencedWorkerPool::RedirectToTaskSchedulerForProcess() {
   DCHECK(TaskScheduler::GetInstance());
   // Hitting this DCHECK indicates that a task was posted to a
   // SequencedWorkerPool before the TaskScheduler was initialized and
@@ -1462,6 +1468,16 @@ void SequencedWorkerPool::
                           AllPoolsState::REDIRECTED_TO_TASK_SCHEDULER);
 }
 
+// static
+void SequencedWorkerPool::ResetRedirectToTaskSchedulerForProcessForTesting() {
+  // This can be called when the current state is REDIRECTED_TO_TASK_SCHEDULER
+  // to stop redirecting tasks. It can also be called when the current state is
+  // WORKER_CREATED to allow RedirectToTaskSchedulerForProcess() to be called
+  // (RedirectToTaskSchedulerForProcess() cannot be called after a worker has
+  // been created if this isn't called).
+  subtle::NoBarrier_Store(&g_all_pools_state, AllPoolsState::NONE_ACTIVE);
+}
+
 SequencedWorkerPool::SequencedWorkerPool(size_t max_threads,
                                          const std::string& thread_name_prefix,
                                          base::TaskPriority task_priority)

base/threading/sequenced_worker_pool.h

@@ -172,16 +172,27 @@ class BASE_EXPORT SequencedWorkerPool : public TaskRunner {
   // PostSequencedWorkerTask(). Valid tokens are always nonzero.
   static SequenceToken GetSequenceToken();
 
-  // Invoke this once on the main thread of a process, before any other threads
-  // are created and before any tasks are posted to that process'
-  // SequencedWorkerPools but after TaskScheduler was instantiated, to force all
-  // SequencedWorkerPools in that process to redirect their tasks to the
-  // TaskScheduler. Note: SequencedWorkerPool instances with |max_threads == 1|
-  // will be special cased to send all of their work as
-  // ExecutionMode::SINGLE_THREADED.
+  // Starts redirecting tasks posted to this process' SequencedWorkerPools to
+  // the registered TaskScheduler. This cannot be called after a task has been
+  // posted to a SequencedWorkerPool. This is not thread-safe; proper
+  // synchronization is required to use any SequencedWorkerPool method after
+  // calling this. There must be a registered TaskScheduler when this is called.
+  // Ideally, call this on the main thread of a process, before any other
+  // threads are created and before any tasks are posted to that process'
+  // SequencedWorkerPools.
+  // Note: SequencedWorkerPool instances with |max_threads == 1| will be special
+  // cased to send all of their work as ExecutionMode::SINGLE_THREADED.
   // TODO(gab): Remove this if http://crbug.com/622400 fails
   // (SequencedWorkerPool will be phased out completely otherwise).
-  static void RedirectSequencedWorkerPoolsToTaskSchedulerForProcess();
+  static void RedirectToTaskSchedulerForProcess();
+
+  // Stops redirecting tasks posted to this process' SequencedWorkerPools to the
+  // registered TaskScheduler and allows RedirectToTaskSchedulerForProcess() to
+  // be called even if tasks have already posted to a SequencedWorkerPool in
+  // this process. Calling this while there are active SequencedWorkerPools is
+  // not supported. This is not thread-safe; proper synchronization is required
+  // to use any SequencedWorkerPool method after calling this.
+  static void ResetRedirectToTaskSchedulerForProcessForTesting();
 
   // When constructing a SequencedWorkerPool, there must be a
   // ThreadTaskRunnerHandle on the current thread unless you plan to

chrome/browser/chrome_browser_main.cc

@@ -450,8 +450,7 @@ void MaybeInitializeTaskScheduler() {
       variation_params.find("RedirectSequencedWorkerPools");
   if (sequenced_worker_pool_param != variation_params.end() &&
       sequenced_worker_pool_param->second == "true") {
-    base::SequencedWorkerPool::
-        RedirectSequencedWorkerPoolsToTaskSchedulerForProcess();
+    base::SequencedWorkerPool::RedirectToTaskSchedulerForProcess();
   }
 }