Add TaskScheduler::InitParams and accept it in TaskSchedulerImpl constructor.

base/task_scheduler/task_scheduler.cc

Index: base/task_scheduler/task_scheduler.cc
diff --git a/base/task_scheduler/task_scheduler.cc b/base/task_scheduler/task_scheduler.cc
index 592d6f4a15ecb97f79f890cbd66ee9001c82f596..3a2680ab71ac19a23b87d5318f41a9123c3585db 100644
--- a/base/task_scheduler/task_scheduler.cc
+++ b/base/task_scheduler/task_scheduler.cc
@@ -21,8 +21,29 @@ namespace {
 // |g_task_scheduler| is intentionally leaked on shutdown.
 TaskScheduler* g_task_scheduler = nullptr;
 
+SchedulerWorkerPoolParams GetWorkerPoolParamsWithPredefinedNameAndPriority(

[gab, 2017/03/20 20:31:56]

It's not really "predefined" as it's being defined by this method... how about
AugmentWorkerPoolParamsWithNameAndPriority()? (with a TODO tagged to same bug to
remove it as documentation for anyone wondering)

[fdoray, 2017/03/20 20:43:54]

Done.

+    const std::string& name,
+    ThreadPriority priority_hint,

[robliao, 2017/03/20 20:47:32]

The end state version won't be using SchedulerWorkerPoolParams as direct
parameters to the SchedulerWorkerPoolImpls, right?

I'd be okay with starting that work here.

[fdoray, 2017/03/20 20:55:35]

Yes it will
https://codereview.chromium.org/2749303002/diff/160001/base/task_scheduler/sc....

SchedulerWorkerPoolParams is exposed outside of TaskScheduler, so it can't
contain params that can't be customized.

[robliao, 2017/03/20 21:00:56]

So once we remove those params, what will this file use?

[robliao, 2017/03/20 21:55:00]

Inlining here so we don't lose track of the thread:
fdoray:
The constructor of SWP will have name, priority_hint and
scheduler_worker_pool_params arguments. TaskSchedulerImpl will instantiate SWPs
with predefined names and priority_hints and externally provided
scheduler_worker_pool_params. This file will not have knowledge of the
predefined names and priority_hints.

See reference CL.

[robliao, 2017/03/20 22:06:27]

I guess this is where I'm a little confused.

From the reference CL
https://codereview.chromium.org/2749303002/diff/160001/base/task_scheduler/ta...
kEnvironmentParams prescribes a name and priority to be used in
SchedulerWorkerPoolImpl::Create

In this CL and in 
https://codereview.chromium.org/2749303002/diff/160001/base/task_scheduler/ta...
, we specify a fixed set of worker pools.

In this CL,
https://codereview.chromium.org/2764603002/diff/20001/base/task_scheduler/sch...
has a comment to remove the constructor that has |name| and |priority_hint| (the
one used below).

The reference CL
https://codereview.chromium.org/2749303002/diff/160001/base/task_scheduler/sc...
also removes |name| and |priority_hint|.

So if we remove |name| and |priority_hint|, this file will have to use something
else, won't it?

Feel free to continue this thread on hangouts.

[fdoray, 2017/03/21 14:41:26]

In this CL, TaskSchedulerImpl is still initialized with a vector of
SchedulerWorkerPoolParams. The new API calls that old API.
This file won't have knowledge of names and priority hints. Those will be
hard-coded in task_scheduler_impl.cc (kEnvironmentParams). Names and priority
hints are currently hard-coded in this file, but this is just temporary.
The final state is:

-- task_scheduler.cc --
void TaskScheduler::CreateAndSetDefaultTaskScheduler(
    const std::string& name,
    const TaskSchedulerInitParams& init_params) {
  SetInstance(internal::TaskSchedulerImpl::Create(name, init_params));
}

-- task_scheduler_impl.cc --
std::unique_ptr<TaskSchedulerImpl> TaskSchedulerImpl::Create(
     const std::string& name,
    const InitParams& init_params) {
  std::unique_ptr<TaskSchedulerImpl> scheduler(new TaskSchedulerImpl(name));
  scheduler->Initialize(init_params);
  return scheduler;
}

void TaskSchedulerImpl::Initialize(const InitParams& init_params) {
  ...

  ************
  Start four worker pools using the names and priority hints from
  |kEnvironmentParams| (not customizable) and the
  SchedulerWorkerPoolParams in |init_params| (customizable).
  ************

  // Start worker pools.
  for (size_t index = 0; index < ENVIRONMENT_COUNT; ++index) {
    worker_pools_[index] = SchedulerWorkerPoolImpl::Create(
    name_ + kEnvironmentParams[index].name_suffix,
    kEnvironmentParams[index].priority_hint,
    *reinterpret_cast<const SchedulerWorkerPoolParams*>(
         reinterpret_cast<const char*>(&init_params) +
         kEnvironmentParams[index].offset),
    ...);
  }
  ...
}
  

[robliao, 2017/03/21 20:02:29]

Summary from offline discussion:
An alternative solution is to have this split be overloaded at TaskSchedulerImpl
and have TaskSchedulerImpl handle initialization from either a vector+function
or InitParams. This means we can avoid the removal of this code later on.

[fdoray, 2017/03/22 13:07:38]

Done.

+    const SchedulerWorkerPoolParams params) {
+  return SchedulerWorkerPoolParams(
+      name, priority_hint, params.standby_thread_policy(), params.max_threads(),
+      params.suggested_reclaim_time(), params.backward_compatibility());
+}
+
 }  // namespace
 
+TaskScheduler::InitParams::InitParams(
+    const SchedulerWorkerPoolParams& background_worker_pool_params_in,
+    const SchedulerWorkerPoolParams& background_blocking_worker_pool_params_in,
+    const SchedulerWorkerPoolParams& foreground_worker_pool_params_in,
+    const SchedulerWorkerPoolParams& foreground_blocking_worker_pool_params_in)
+    : background_worker_pool_params(background_worker_pool_params_in),
+      background_blocking_worker_pool_params(
+          background_blocking_worker_pool_params_in),
+      foreground_worker_pool_params(foreground_worker_pool_params_in),
+      foreground_blocking_worker_pool_params(
+          foreground_blocking_worker_pool_params_in) {}
+
 #if !defined(OS_NACL)
 // static
 void TaskScheduler::CreateAndSetSimpleTaskScheduler(const std::string& name) {
@@ -31,7 +52,7 @@ void TaskScheduler::CreateAndSetSimpleTaskScheduler(const std::string& name) {
   worker_pool_params_vector.emplace_back(
       name, ThreadPriority::NORMAL,
       SchedulerWorkerPoolParams::StandbyThreadPolicy::LAZY,
-      std::max(kMinNumThreads, base::SysInfo::NumberOfProcessors()),
+      std::max(kMinNumThreads, SysInfo::NumberOfProcessors()),
       TimeDelta::FromSeconds(30));
   CreateAndSetDefaultTaskScheduler(
       worker_pool_params_vector,
@@ -48,6 +69,55 @@ void TaskScheduler::CreateAndSetDefaultTaskScheduler(
       worker_pool_params_vector, worker_pool_index_for_traits_callback));
 }
 
+void TaskScheduler::CreateAndSetDefaultTaskScheduler(
+    const std::string& name,
+    const InitParams& init_params) {
+  enum WorkerPoolType : size_t {
+    BACKGROUND = 0,
+    BACKGROUND_BLOCKING,
+    FOREGROUND,
+    FOREGROUND_BLOCKING,
+    WORKER_POOL_COUNT  // Always last.
+  };
+
+  std::vector<SchedulerWorkerPoolParams> worker_pool_params_vector;
+
+  DCHECK_EQ(BACKGROUND, worker_pool_params_vector.size());
+  worker_pool_params_vector.emplace_back(
+      GetWorkerPoolParamsWithPredefinedNameAndPriority(
+          name + "Background", ThreadPriority::BACKGROUND,
+          init_params.background_worker_pool_params));
+
+  DCHECK_EQ(BACKGROUND_BLOCKING, worker_pool_params_vector.size());
+  worker_pool_params_vector.emplace_back(
+      GetWorkerPoolParamsWithPredefinedNameAndPriority(
+          name + "BackgroundBlocking", ThreadPriority::BACKGROUND,
+          init_params.background_blocking_worker_pool_params));
+
+  DCHECK_EQ(FOREGROUND, worker_pool_params_vector.size());
+  worker_pool_params_vector.emplace_back(
+      GetWorkerPoolParamsWithPredefinedNameAndPriority(
+          name + "Foreground", ThreadPriority::NORMAL,
+          init_params.foreground_worker_pool_params));
+
+  DCHECK_EQ(FOREGROUND_BLOCKING, worker_pool_params_vector.size());
+  worker_pool_params_vector.emplace_back(
+      GetWorkerPoolParamsWithPredefinedNameAndPriority(
+          name + "ForegroundBlocking", ThreadPriority::NORMAL,
+          init_params.foreground_blocking_worker_pool_params));
+
+  DCHECK_EQ(WORKER_POOL_COUNT, worker_pool_params_vector.size());
+
+  CreateAndSetDefaultTaskScheduler(
+      worker_pool_params_vector, Bind([](const TaskTraits& traits) -> size_t {
+        const bool is_background =
+            traits.priority() == TaskPriority::BACKGROUND;
+        if (traits.may_block() || traits.with_base_sync_primitives())
+          return is_background ? BACKGROUND_BLOCKING : FOREGROUND_BLOCKING;
+        return is_background ? BACKGROUND : FOREGROUND;
+      }));
+}
+
 // static
 void TaskScheduler::SetInstance(std::unique_ptr<TaskScheduler> task_scheduler) {
   delete g_task_scheduler;