TaskScheduler: Remove base/task_scheduler/utils.h/.cc

TaskScheduler: Remove base/task_scheduler/utils.h/.cc

- Rename SchedulerThreadPool -> SchedulerThreadPoolImpl.
- Rename SchedulerTaskExecutor -> SchedulerThreadPool.
    * For now, this interface is implemented by SchedulerThreadPoolImpl.
      In the future, it could be implemented by a thread pool backed
      by the Windows thread pool.
- Move the declaration of methods that would make sense for any
  thread pool implementation to SchedulerThreadPool.
- Remove utils.h/.cc. Move the code of helper functions defined in
  these files directly in scheduler_thread_pool_impl.cc.
    * scheduler_worker_thread.cc won't use these functions as
      initially planned, which means that there is no need to
      make them accessible outside of scheduler_thread_pool_impl.cc

BUG=553459
Committed: https://crrev.com/583b226e92c4d416c149628baa7b8ea76e7aed48
Cr-Commit-Position: refs/heads/master@{#389567}

[fdoray, 2016-04-21 19:33:56]

fdoray@chromium.org changed reviewers:
+ gab@chromium.org, robliao@chromium.org

[fdoray, 2016-04-21 19:33:57]

Can you review this CL? Thanks.

[robliao, 2016-04-21 19:38:59]

What would this look like for child task runners (currently they choose the
priority queue). Would child task runners instead choose the threadpool they
want to post to?

[fdoray, 2016-04-21 19:56:57]

On 2016/04/21 19:38:59, robliao wrote:
> What would this look like for child task runners (currently they choose the
> priority queue). Would child task runners instead choose the threadpool they
> want to post to?

Yes, child task runners would choose their thread pool instead of their PQ. Note
that this is already the case (TaskRunners don't have a pointer to a
PriorityQueue
https://code.google.com/p/chromium/codesearch#chromium/src/base/task_schedule...).

Could we implement child task runners like this:
- SchedulerTaskRunner::CreateChildTaskRunner calls
TaskScheduler::CreateTaskRunnerWithTraits with its Sequence. Sequence is an
optional argument of TaskScheduler::CreateTaskRunnerWithTraits.
- TaskScheduler::CreateTaskRunnerWithTraits calls
STP::CreateTaskRunnerWithTraits on the appropriate STP (based on TaskTraits).
- STP::CreateTaskRunnerWithTraits constructs the appropriate TaskRunner type
with the Sequence it received (can be nullptr).
- The constructor of Sequenced/SingleThreadTaskRunner stores the Sequence it
receives, if any. Otherwise, it creates a new one (like it does currently).

I don't remember why a TaskRunner wrapper was a good idea.

[robliao, 2016-04-21 20:06:36]

On 2016/04/21 19:56:57, fdoray wrote:
> On 2016/04/21 19:38:59, robliao wrote:
> > What would this look like for child task runners (currently they choose the
> > priority queue). Would child task runners instead choose the threadpool they
> > want to post to?
> 
> Yes, child task runners would choose their thread pool instead of their PQ.
Note
> that this is already the case (TaskRunners don't have a pointer to a
> PriorityQueue
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/task_schedule...).
> 
> Could we implement child task runners like this:
> - SchedulerTaskRunner::CreateChildTaskRunner calls
> TaskScheduler::CreateTaskRunnerWithTraits with its Sequence. Sequence is an
> optional argument of TaskScheduler::CreateTaskRunnerWithTraits.
> - TaskScheduler::CreateTaskRunnerWithTraits calls
> STP::CreateTaskRunnerWithTraits on the appropriate STP (based on TaskTraits).
> - STP::CreateTaskRunnerWithTraits constructs the appropriate TaskRunner type
> with the Sequence it received (can be nullptr).
> - The constructor of Sequenced/SingleThreadTaskRunner stores the Sequence it
> receives, if any. Otherwise, it creates a new one (like it does currently).
> 
> I don't remember why a TaskRunner wrapper was a good idea.

Do you mean SchedulerTaskRunner wrapping the underlying TaskRunner?

[fdoray, 2016-04-21 20:32:36]

On 2016/04/21 20:06:36, robliao wrote:
> On 2016/04/21 19:56:57, fdoray wrote:
> > On 2016/04/21 19:38:59, robliao wrote:
> > > What would this look like for child task runners (currently they choose
the
> > > priority queue). Would child task runners instead choose the threadpool
they
> > > want to post to?
> > 
> > Yes, child task runners would choose their thread pool instead of their PQ.
> Note
> > that this is already the case (TaskRunners don't have a pointer to a
> > PriorityQueue
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/task_schedule...).
> > 
> > Could we implement child task runners like this:
> > - SchedulerTaskRunner::CreateChildTaskRunner calls
> > TaskScheduler::CreateTaskRunnerWithTraits with its Sequence. Sequence is an
> > optional argument of TaskScheduler::CreateTaskRunnerWithTraits.
> > - TaskScheduler::CreateTaskRunnerWithTraits calls
> > STP::CreateTaskRunnerWithTraits on the appropriate STP (based on
TaskTraits).
> > - STP::CreateTaskRunnerWithTraits constructs the appropriate TaskRunner type
> > with the Sequence it received (can be nullptr).
> > - The constructor of Sequenced/SingleThreadTaskRunner stores the Sequence it
> > receives, if any. Otherwise, it creates a new one (like it does currently).
> > 
> > I don't remember why a TaskRunner wrapper was a good idea.
> 
> Do you mean SchedulerTaskRunner wrapping the underlying TaskRunner?

Oh, I remember now. We decided to have a SchedulerTaskRunner that wraps our
various TaskRunner types so that we can call CreateChildTaskRunner on the task
runner returned by CreateChildTaskRunnerWithTraits without having diamond
inheritance. Anyways, I don't think it changes anything if TaskRunners choose
their ThreadPool instead of their PriorityQueue.

[gab, 2016-04-22 18:53:56]

lgtm % comments

https://codereview.chromium.org/1906083002/diff/1/base/task_scheduler/schedul...
File base/task_scheduler/scheduler_thread_pool_interface.h (right):

https://codereview.chromium.org/1906083002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_thread_pool_interface.h:18: // Interface for a
thread pool.
s/Interface/Minimal post task interface/ ?

https://codereview.chromium.org/1906083002/diff/1/base/task_scheduler/utils_u...
File base/task_scheduler/utils_unittest.cc (left):

https://codereview.chromium.org/1906083002/diff/1/base/task_scheduler/utils_u...
base/task_scheduler/utils_unittest.cc:141: }  // namespace base
Is this functionality already fully covered by the existing SchedulerThreadPool
tests?

[danakj, 2016-04-22 20:09:56]

danakj@chromium.org changed reviewers:
+ danakj@chromium.org

[danakj, 2016-04-22 20:09:56]

LGTM!

[robliao, 2016-04-22 20:22:25]

https://codereview.chromium.org/1906083002/diff/1/base/task_scheduler/schedul...
File base/task_scheduler/scheduler_thread_pool_interface.h (right):

https://codereview.chromium.org/1906083002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_thread_pool_interface.h:19: class BASE_EXPORT
SchedulerThreadPoolInterface {
It's not common to have a class with Interface as part of its name in Chromium.
Maybe SequencePoster? Other names welcome.

[fdoray, 2016-04-22 22:18:14]

robliao@: Can you take another look? Thanks.

https://codereview.chromium.org/1906083002/diff/1/base/task_scheduler/schedul...
File base/task_scheduler/scheduler_thread_pool_interface.h (right):

https://codereview.chromium.org/1906083002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_thread_pool_interface.h:18: // Interface for a
thread pool.
On 2016/04/22 18:53:56, gab wrote:
> s/Interface/Minimal post task interface/ ?

Done. s/Interface/Minimal interface/. It's not just a post task interface now.

https://codereview.chromium.org/1906083002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_thread_pool_interface.h:19: class BASE_EXPORT
SchedulerThreadPoolInterface {
On 2016/04/22 20:22:25, robliao wrote:
> It's not common to have a class with Interface as part of its name in
Chromium.
> Maybe SequencePoster? Other names welcome.

Renamed SchedulerThreadPoolInterface -> SchedulerThreadPool and
SchedulerThreadPool -> SchedulerThreadPoolImpl as discussed offline. Also moved
the declaration of a few methods that would make sense for any thread pool
implementation to SchedulerThreadPool. A thread pool implementation backed by
the Windows thread pool could reuse the SchedulerThreadPool interface.

https://codereview.chromium.org/1906083002/diff/1/base/task_scheduler/utils_u...
File base/task_scheduler/utils_unittest.cc (left):

https://codereview.chromium.org/1906083002/diff/1/base/task_scheduler/utils_u...
base/task_scheduler/utils_unittest.cc:141: }  // namespace base
On 2016/04/22 18:53:56, gab wrote:
> Is this functionality already fully covered by the existing
SchedulerThreadPool
> tests?

PostTaskAllowed and PostTaskInEmptySequence are covered by
STPTest::PostTasksWaitAllThreadsIdle

PostTaskInNonEmptySequence is covered by STPTest::PostTasks

Added STPTest::PostTaskAfterShutdown and STPTest::PostDelayedTask to cover the
existing PostDelayedTaskAllowed and PostTaskNotAllowed.

[danakj, 2016-04-22 22:21:28]

That's all cool with me

[robliao, 2016-04-22 22:56:42]

Looks mostly good. Update the CL description to reflect the changes in the CL as
well.

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/del...
File base/task_scheduler/delayed_task_manager.h (right):

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/del...
base/task_scheduler/delayed_task_manager.h:58: virtual TimeTicks Now() const;
I probably missed it, but can this be made private again?

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_thread_pool_impl.h (right):

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
base/task_scheduler/scheduler_thread_pool_impl.h:69: scoped_refptr<TaskRunner>
CreateTaskRunnerWithTraits(
Can these be made private or do we have callers that will need both sets of
methods from SchedulerThreadPoolImpl and SchedulerThreadPool?

[fdoray, 2016-04-25 15:05:49]

Description was changed from

==========
TaskScheduler: Remove base/task_scheduler/utils.h/.cc

All TaskRunners of the TaskScheduler will post tasks through a
SchedulerThreadPool. That means that helper functions in utils.h/.cc
won't be used by scheduler_worker_thread.cc as initially planned. This
CL moves the code of these helper functions directly in methods of
SchedulerThreadPool.

Also, SchedulerWorkerThread won't implement the SchedulerTaskExecutor
interface as initially planned. This CL renames this interface to
SchedulerThreadPoolInterface (having an interface is still useful to
create mocks - see delayed_task_manager_unittest.cc).

BUG=553459
==========

to

==========
TaskScheduler: Remove base/task_scheduler/utils.h/.cc

- Rename SchedulerThreadPool -> SchedulerThreadPoolImpl.
- Rename SchedulerTaskExecutor -> SchedulerThreadPool.
    * For now, this interface is implemented by SchedulerThreadPoolImpl.
      In the future, it could be implemented by a thread pool backed
      by the Windows thread pool.
- Move the declaration of methods that would make sense for any
  thread pool implementation to SchedulerThreadPool.
- Remove utils.h/.cc. Move the code of helper functions defined in
  these files directly in scheduler_thread_pool_impl.cc.
    * scheduler_worker_thread.cc won't use these functions as
      initially planned, which means that there is no need to
      make them accessible outside of scheduler_thread_pool_impl.cc

BUG=553459
==========

[fdoray, 2016-04-25 15:19:12]

robliao@: Can you take another look?

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/del...
File base/task_scheduler/delayed_task_manager.h (right):

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/del...
base/task_scheduler/delayed_task_manager.h:58: virtual TimeTicks Now() const;
On 2016/04/22 22:56:41, robliao wrote:
> I probably missed it, but can this be made private again?

Can't be private because it's used here
https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
Using the delayed manger's time instead of TimeTicks::Now() is required for this
unit test
https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_thread_pool_impl.h (right):

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
base/task_scheduler/scheduler_thread_pool_impl.h:69: scoped_refptr<TaskRunner>
CreateTaskRunnerWithTraits(
On 2016/04/22 22:56:41, robliao wrote:
> Can these be made private or do we have callers that will need both sets of
> methods from SchedulerThreadPoolImpl and SchedulerThreadPool?

Relevant discussions:

tl;dr Overrides are public except when the base class clearly states in which
context they should be called (e.g. methods that override MyClassDelegate can be
private because it's clear that they shouldn't be called outside of MyClass).

https://codereview.chromium.org/1708773002/diff/610001/base/task_scheduler/sc...
https://codereview.chromium.org/1708773002/diff/470001/base/task_scheduler/sc...

[gab, 2016-04-25 17:52:43]

New direction lgtm overall % comments. Will leave it up to Rob for final
detailed review.

Thanks,
Gab

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_thread_pool.h (right):

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
base/task_scheduler/scheduler_thread_pool.h:37: virtual void
EnqueueSequence(scoped_refptr<Sequence> sequence,
How about we call this ReEnqueueSequence?

(I know it already existed but I care more about the name now that this is on an
interface instead of an impl)

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_thread_pool_impl_unittest.cc (right):

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
base/task_scheduler/scheduler_thread_pool_impl_unittest.cc:359:
thread_pool_->CreateTaskRunnerWithTraits(TaskTraits(), GetParam())
Create the TaskRunner before calling Shutdown() to make it clearer this is
testing PostTask?

[fdoray, 2016-04-25 19:14:51]

robliao@: Can you take another look? Thanks.

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_thread_pool.h (right):

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
base/task_scheduler/scheduler_thread_pool.h:37: virtual void
EnqueueSequence(scoped_refptr<Sequence> sequence,
On 2016/04/25 17:52:43, gab wrote:
> How about we call this ReEnqueueSequence?
> 
> (I know it already existed but I care more about the name now that this is on
an
> interface instead of an impl)

Done.

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_thread_pool_impl_unittest.cc (right):

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
base/task_scheduler/scheduler_thread_pool_impl_unittest.cc:359:
thread_pool_->CreateTaskRunnerWithTraits(TaskTraits(), GetParam())
On 2016/04/25 17:52:43, gab wrote:
> Create the TaskRunner before calling Shutdown() to make it clearer this is
> testing PostTask?

Done.

[gab, 2016-04-25 19:28:07]

https://codereview.chromium.org/1906083002/diff/100001/base/task_scheduler/sc...
File base/task_scheduler/scheduler_worker_thread_unittest.cc (right):

https://codereview.chromium.org/1906083002/diff/100001/base/task_scheduler/sc...
base/task_scheduler/scheduler_worker_thread_unittest.cc:136: // Tasks and adds
it to |re_re_enqueued_sequences_|. Unlike a normal
s/re_re_/re_/ :-)

[fdoray, 2016-04-25 19:35:09]

robliao@: Can you take another look? Thanks.

https://codereview.chromium.org/1906083002/diff/100001/base/task_scheduler/sc...
File base/task_scheduler/scheduler_worker_thread_unittest.cc (right):

https://codereview.chromium.org/1906083002/diff/100001/base/task_scheduler/sc...
base/task_scheduler/scheduler_worker_thread_unittest.cc:136: // Tasks and adds
it to |re_re_enqueued_sequences_|. Unlike a normal
On 2016/04/25 19:28:07, gab wrote:
> s/re_re_/re_/ :-)

Done.

[robliao, 2016-04-25 20:08:10]

lgtm

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/del...
File base/task_scheduler/delayed_task_manager.h (right):

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/del...
base/task_scheduler/delayed_task_manager.h:58: virtual TimeTicks Now() const;
On 2016/04/25 15:19:12, fdoray wrote:
> On 2016/04/22 22:56:41, robliao wrote:
> > I probably missed it, but can this be made private again?
> 
> Can't be private because it's used here
>
https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
> Using the delayed manger's time instead of TimeTicks::Now() is required for
this
> unit test
>
https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...

Gotcha. One of these days, we should look into indirecting these Time APIs.
Having a time API on DelayedTaskManager feels very wrong to me.

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_thread_pool_impl.h (right):

https://codereview.chromium.org/1906083002/diff/80001/base/task_scheduler/sch...
base/task_scheduler/scheduler_thread_pool_impl.h:69: scoped_refptr<TaskRunner>
CreateTaskRunnerWithTraits(
On 2016/04/25 15:19:12, fdoray wrote:
> On 2016/04/22 22:56:41, robliao wrote:
> > Can these be made private or do we have callers that will need both sets of
> > methods from SchedulerThreadPoolImpl and SchedulerThreadPool?
> 
> Relevant discussions:
> 
> tl;dr Overrides are public except when the base class clearly states in which
> context they should be called (e.g. methods that override MyClassDelegate can
be
> private because it's clear that they shouldn't be called outside of MyClass).
> 
>
https://codereview.chromium.org/1708773002/diff/610001/base/task_scheduler/sc...
>
https://codereview.chromium.org/1708773002/diff/470001/base/task_scheduler/sc...

The chromium-dev discussion on the topic is here:
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/CwzjcWd9cYM/dis...

The main benefit to making overridden methods private is it helps enforce that
callers to use the interface form of the class rather than the implementation
form of the class.

I don't have a really strong opinion in this case here since there are good
arguments to both sides. I'm fine with it being public, just wanted to make sure
we're doing this deliberately.

[fdoray, 2016-04-25 20:09:08]

The CQ bit was checked by fdoray@chromium.org

[fdoray, 2016-04-25 20:09:08]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
gab@chromium.org
Link to the patchset: https://codereview.chromium.org/1906083002/#ps140001
(title: "typos")

[commit-bot: I haz the power, 2016-04-25 20:09:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1906083002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1906083002/140001

[commit-bot: I haz the power, 2016-04-25 21:34:42]

Description was changed from

==========
TaskScheduler: Remove base/task_scheduler/utils.h/.cc

- Rename SchedulerThreadPool -> SchedulerThreadPoolImpl.
- Rename SchedulerTaskExecutor -> SchedulerThreadPool.
    * For now, this interface is implemented by SchedulerThreadPoolImpl.
      In the future, it could be implemented by a thread pool backed
      by the Windows thread pool.
- Move the declaration of methods that would make sense for any
  thread pool implementation to SchedulerThreadPool.
- Remove utils.h/.cc. Move the code of helper functions defined in
  these files directly in scheduler_thread_pool_impl.cc.
    * scheduler_worker_thread.cc won't use these functions as
      initially planned, which means that there is no need to
      make them accessible outside of scheduler_thread_pool_impl.cc

BUG=553459
==========

to

==========
TaskScheduler: Remove base/task_scheduler/utils.h/.cc

- Rename SchedulerThreadPool -> SchedulerThreadPoolImpl.
- Rename SchedulerTaskExecutor -> SchedulerThreadPool.
    * For now, this interface is implemented by SchedulerThreadPoolImpl.
      In the future, it could be implemented by a thread pool backed
      by the Windows thread pool.
- Move the declaration of methods that would make sense for any
  thread pool implementation to SchedulerThreadPool.
- Remove utils.h/.cc. Move the code of helper functions defined in
  these files directly in scheduler_thread_pool_impl.cc.
    * scheduler_worker_thread.cc won't use these functions as
      initially planned, which means that there is no need to
      make them accessible outside of scheduler_thread_pool_impl.cc

BUG=553459
==========

[commit-bot: I haz the power, 2016-04-25 21:34:43]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-04-25 21:36:44]

Description was changed from

==========
TaskScheduler: Remove base/task_scheduler/utils.h/.cc

- Rename SchedulerThreadPool -> SchedulerThreadPoolImpl.
- Rename SchedulerTaskExecutor -> SchedulerThreadPool.
    * For now, this interface is implemented by SchedulerThreadPoolImpl.
      In the future, it could be implemented by a thread pool backed
      by the Windows thread pool.
- Move the declaration of methods that would make sense for any
  thread pool implementation to SchedulerThreadPool.
- Remove utils.h/.cc. Move the code of helper functions defined in
  these files directly in scheduler_thread_pool_impl.cc.
    * scheduler_worker_thread.cc won't use these functions as
      initially planned, which means that there is no need to
      make them accessible outside of scheduler_thread_pool_impl.cc

BUG=553459
==========

to

==========
TaskScheduler: Remove base/task_scheduler/utils.h/.cc

- Rename SchedulerThreadPool -> SchedulerThreadPoolImpl.
- Rename SchedulerTaskExecutor -> SchedulerThreadPool.
    * For now, this interface is implemented by SchedulerThreadPoolImpl.
      In the future, it could be implemented by a thread pool backed
      by the Windows thread pool.
- Move the declaration of methods that would make sense for any
  thread pool implementation to SchedulerThreadPool.
- Remove utils.h/.cc. Move the code of helper functions defined in
  these files directly in scheduler_thread_pool_impl.cc.
    * scheduler_worker_thread.cc won't use these functions as
      initially planned, which means that there is no need to
      make them accessible outside of scheduler_thread_pool_impl.cc

BUG=553459

Committed: https://crrev.com/583b226e92c4d416c149628baa7b8ea76e7aed48
Cr-Commit-Position: refs/heads/master@{#389567}
==========

[commit-bot: I haz the power, 2016-04-25 21:36:45]

Patchset 8 (id:??) landed as
https://crrev.com/583b226e92c4d416c149628baa7b8ea76e7aed48
Cr-Commit-Position: refs/heads/master@{#389567}

[gab, 2016-04-26 21:50:05]

https://codereview.chromium.org/1906083002/diff/140001/base/task_scheduler/sc...
File base/task_scheduler/scheduler_thread_pool.h (right):

https://codereview.chromium.org/1906083002/diff/140001/base/task_scheduler/sc...
base/task_scheduler/scheduler_thread_pool.h:46: // scheduler's TaskTracker must
have allowed |task| to be posted before this
Just realized while doing a local merge that this still mentions "TaskTracker"
although the generic interface doesn't know about it. Can we re-structure the
comments to avoid that?

[gab, 2016-04-26 22:00:56]

https://codereview.chromium.org/1906083002/diff/140001/base/task_scheduler/sc...
File base/task_scheduler/scheduler_thread_pool.h (right):

https://codereview.chromium.org/1906083002/diff/140001/base/task_scheduler/sc...
base/task_scheduler/scheduler_thread_pool.h:36: // worker thread doesn't have to
belong to this thread pool.
Actually this comment is also a bit outdated. Probably worth revisiting these
comments to make sure they match the new generic interface and situation.