Always use TaskScheduler::InitParams to initialize a TaskScheduler.

base/task_scheduler/scheduler_single_thread_task_runner_manager.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/task_scheduler/scheduler_single_thread_task_runner_manager.h"
 
 #include <algorithm>
 #include <memory>
 #include <string>
 #include <utility>
@@ skipping 295 matching lines @@
   }
 
   SchedulerSingleThreadTaskRunnerManager* const outer_;
   const TaskTraits traits_;
   SchedulerWorker* const worker_;
 
   DISALLOW_COPY_AND_ASSIGN(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_);
 }
 
 SchedulerSingleThreadTaskRunnerManager::
     ~SchedulerSingleThreadTaskRunnerManager() {
 #if DCHECK_IS_ON()
   size_t workers_unregistered_during_join =
       subtle::NoBarrier_Load(&workers_unregistered_during_join_);
   DCHECK_EQ(workers_unregistered_during_join, workers_.size())
@@ skipping 13 matching lines @@
   // Start workers that were created before this method was called. Other
   // workers are started as they are created.
   for (scoped_refptr<SchedulerWorker> worker : workers_to_start) {
     worker->Start();
     worker->WakeUp();
   }
 }
 
 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)
 
 void SchedulerSingleThreadTaskRunnerManager::JoinForTesting() {
   decltype(workers_) local_workers;
   {
     AutoSchedulerLock auto_lock(lock_);
     local_workers = std::move(workers_);
   }
 
   for (const auto& worker : local_workers)
     worker->JoinForTesting();
 
   {
     AutoSchedulerLock auto_lock(lock_);
     DCHECK(workers_.empty())
         << "New worker(s) unexpectedly registered during join.";
     workers_ = std::move(local_workers);
   }
 }
 
 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)
 
 template <typename DelegateType>
 SchedulerWorker*
 SchedulerSingleThreadTaskRunnerManager::CreateAndRegisterSchedulerWorker(
-    const SchedulerWorkerPoolParams& params) {
+    const std::string& name,
+    ThreadPriority priority_hint) {
   SchedulerWorker* worker;
   bool start_worker;
 
   {
     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_;
   }
 
   if (start_worker)
     worker->Start();
 
   return worker;
 }
@@ skipping 22 matching lines @@
                      });
     DCHECK(worker_iter != workers_.end());
     worker_to_destroy = std::move(*worker_iter);
     workers_.erase(worker_iter);
   }
   worker_to_destroy->Cleanup();
 }
 
 }  // namespace internal
 }  // namespace base