Always use TaskScheduler::InitParams to initialize a TaskScheduler.

base/task_scheduler/scheduler_single_thread_task_runner_manager.cc

Index: base/task_scheduler/scheduler_single_thread_task_runner_manager.cc
diff --git a/base/task_scheduler/scheduler_single_thread_task_runner_manager.cc b/base/task_scheduler/scheduler_single_thread_task_runner_manager.cc
index e2ff1ef1058a49e61724b2bfdd211a1f7a7a1564..7bd44928ea7931890ff31086ca92de489e54515f 100644
--- a/base/task_scheduler/scheduler_single_thread_task_runner_manager.cc
+++ b/base/task_scheduler/scheduler_single_thread_task_runner_manager.cc
@@ -313,18 +313,9 @@ class SchedulerSingleThreadTaskRunnerManager::SchedulerSingleThreadTaskRunner
 };
 
 SchedulerSingleThreadTaskRunnerManager::SchedulerSingleThreadTaskRunnerManager(
-    const std::vector<SchedulerWorkerPoolParams>& worker_pool_params_vector,
-    const TaskScheduler::WorkerPoolIndexForTraitsCallback&
-        worker_pool_index_for_traits_callback,
     TaskTracker* task_tracker,
     DelayedTaskManager* delayed_task_manager)
-    : worker_pool_params_vector_(worker_pool_params_vector),
-      worker_pool_index_for_traits_callback_(
-          worker_pool_index_for_traits_callback),
-      task_tracker_(task_tracker),
-      delayed_task_manager_(delayed_task_manager) {
-  DCHECK_GT(worker_pool_params_vector_.size(), 0U);
-  DCHECK(worker_pool_index_for_traits_callback_);
+    : task_tracker_(task_tracker), delayed_task_manager_(delayed_task_manager) {
   DCHECK(task_tracker_);
   DCHECK(delayed_task_manager_);
 }
@@ -358,17 +349,21 @@ void SchedulerSingleThreadTaskRunnerManager::Start() {
 
 scoped_refptr<SingleThreadTaskRunner>
 SchedulerSingleThreadTaskRunnerManager::CreateSingleThreadTaskRunnerWithTraits(
+    const std::string& name,
+    ThreadPriority priority_hint,
     const TaskTraits& traits) {
   return CreateSingleThreadTaskRunnerWithDelegate<SchedulerWorkerDelegate>(
-      traits);
+      name, priority_hint, traits);
 }
 
 #if defined(OS_WIN)
 scoped_refptr<SingleThreadTaskRunner>
 SchedulerSingleThreadTaskRunnerManager::CreateCOMSTATaskRunnerWithTraits(
+    const std::string& name,
+    ThreadPriority priority_hint,
     const TaskTraits& traits) {
   return CreateSingleThreadTaskRunnerWithDelegate<SchedulerWorkerCOMDelegate>(
-      traits);
+      name, priority_hint, traits);
 }
 #endif  // defined(OS_WIN)
 
@@ -392,32 +387,29 @@ void SchedulerSingleThreadTaskRunnerManager::JoinForTesting() {
 
 template <typename DelegateType>
 scoped_refptr<SingleThreadTaskRunner> SchedulerSingleThreadTaskRunnerManager::
-    CreateSingleThreadTaskRunnerWithDelegate(const TaskTraits& traits) {
-  size_t index = worker_pool_index_for_traits_callback_.Run(traits);
-  DCHECK_LT(index, worker_pool_params_vector_.size());
+    CreateSingleThreadTaskRunnerWithDelegate(const std::string& name,
+                                             ThreadPriority priority_hint,
+                                             const TaskTraits& traits) {
   return new SchedulerSingleThreadTaskRunner(
       this, traits,
-      CreateAndRegisterSchedulerWorker<DelegateType>(
-          worker_pool_params_vector_[index]));
+      CreateAndRegisterSchedulerWorker<DelegateType>(name, priority_hint));
 }
 
 template <>
 std::unique_ptr<SchedulerWorkerDelegate>
 SchedulerSingleThreadTaskRunnerManager::CreateSchedulerWorkerDelegate<
-    SchedulerWorkerDelegate>(const SchedulerWorkerPoolParams& params, int id) {
-  return MakeUnique<SchedulerWorkerDelegate>(StringPrintf(
-      "TaskSchedulerSingleThreadWorker%d%s", id, params.name().c_str()));
+    SchedulerWorkerDelegate>(const std::string& name, int id) {
+  return MakeUnique<SchedulerWorkerDelegate>(
+      StringPrintf("TaskScheduler%sSingleThread%d", name.c_str(), id));

[robliao, 2017/04/12 19:08:13]

This naming makes it harder to sort against all of the single threaded workers.
This is why I kept %s at the end instead of the middle.

[fdoray, 2017/04/12 20:00:23]

Done.

[robliao, 2017/04/12 20:37:50]

This leads me to my second question. It does seem strange that the caller
specifies both the priority and the traits to create a SingleThreadTaskRunner.
Priority is generally derived from the traits and that policy is set by the
TaskSchedulerImpl. Previously, SchedulerSingleThreadTaskRunnerManager accepted
that policy determination from TaskSchedulerImpl to determine the priority here.
Would a similar callback make sense here too?

[fdoray, 2017/04/12 20:57:48]

In this CL, TaskSchedulerImpl derives the thread priority from TaskTraits and
provides it directly to SchedulerSingleThreadTaskRunnerManager instead of
providing a callback+vector<SchedulerWorkerPoolParams>. 

[robliao, 2017/04/12 21:09:05]

Right. This also means that later on someone could pass
ThreadPriority::BACKGROUND with
TaskTraits().WithPriority(TaskPriority::USER_BLOCKING), which definitely seems
wrong.

If it makes this CL easier to land, I'm fine with this shortcoming, but would
just like to voice my objection to this signature and think the right thing to
do is have this call back to TaskSchedulerImpl for the priority instead of
receiving it from TaskSchedulerImpl.

[fdoray, 2017/04/19 12:21:21]

Acknowledged.

 }
 
 #if defined(OS_WIN)
 template <>
 std::unique_ptr<SchedulerWorkerDelegate>
 SchedulerSingleThreadTaskRunnerManager::CreateSchedulerWorkerDelegate<
-    SchedulerWorkerCOMDelegate>(const SchedulerWorkerPoolParams& params,
-                                int id) {
+    SchedulerWorkerCOMDelegate>(const std::string& name, int id) {
   return MakeUnique<SchedulerWorkerCOMDelegate>(
-      StringPrintf("TaskSchedulerSingleThreadWorker%d%sCOMSTA", id,
-                   params.name().c_str()),
+      StringPrintf("TaskScheduler%sSingleThreadCOMSTA%d", name.c_str(), id),
       task_tracker_);
 }
 #endif  // defined(OS_WIN)
@@ -425,7 +417,8 @@ SchedulerSingleThreadTaskRunnerManager::CreateSchedulerWorkerDelegate<
 template <typename DelegateType>
 SchedulerWorker*
 SchedulerSingleThreadTaskRunnerManager::CreateAndRegisterSchedulerWorker(
-    const SchedulerWorkerPoolParams& params) {
+    const std::string& name,
+    ThreadPriority priority_hint) {
   SchedulerWorker* worker;
   bool start_worker;
 
@@ -433,8 +426,7 @@ SchedulerSingleThreadTaskRunnerManager::CreateAndRegisterSchedulerWorker(
     AutoSchedulerLock auto_lock(lock_);
     int id = next_worker_id_++;
     workers_.emplace_back(make_scoped_refptr(new SchedulerWorker(
-        params.priority_hint(),
-        CreateSchedulerWorkerDelegate<DelegateType>(params, id),
+        priority_hint, CreateSchedulerWorkerDelegate<DelegateType>(name, id),
         task_tracker_)));
     worker = workers_.back().get();
     start_worker = started_;