Rename SchedulerWorkerThread* to SchedulerWorker*

Rename SchedulerWorkerThread* to SchedulerWorker*

This rename is in preparation for virtualized Scheduler Workers which will introduce SchedulerWorker::Thread.

BUG=553459
TBR=danakj

Committed: https://crrev.com/7a58439650da70e6b6cfedb80de104b55b092603
Cr-Commit-Position: refs/heads/master@{#401364}

[robliao, 2016-06-17 21:32:43]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[robliao, 2016-06-17 21:33:01]

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

[robliao, 2016-06-17 21:33:01]

Here's the second round!

[commit-bot: I haz the power, 2016-06-17 21:33:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2077093002/1

[commit-bot: I haz the power, 2016-06-17 22:21:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-17 22:21:27]

Dry run: This issue passed the CQ dry run.

[fdoray, 2016-06-20 15:08:40]

PTAnL

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

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker.h:36: // the thread managed by the
SchedulerWorker instance.
*a* thread

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker.h:41: // Called by |worker| when it enters
its main function.
when *a thread it manages* enters

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker.h:53: // if the thread's WakeUp() method is
called.
worker's

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
File base/task_scheduler/scheduler_worker_pool_impl.cc (right):

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.cc:186: // single-threaded
PriorityQueue's transactions. |index| will be appended to
|index| will be appended to the pool name to label threads managed by the worker
for which this delegate is constructed.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.cc:300: // another thread after
|sequence| is inserted in |shared_priority_queue_|. If
another *worker* after

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.cc:301: // we did wake up another
thread, we would waste resources by having more
we did wake up another *worker*

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.cc:302: // threads trying to get
a Sequence from |shared_priority_queue_| than the
*workers* trying to get

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.cc:387: // Wait for
|outer_->threads_created_| to avoid traversing
workers_created_ (if you change the variable's name)

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

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.h:55: // Creates a
SchedulerWorkerPoolImpl labeled |name| with up to |max_threads|
|max_workers| workers of priority ... ?

Just a suggestion, "thread" is not wrong here.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.h:114: // The name of this worker
pool, used to label its worker threads.
label its workers

Just a suggestion, "thread" is not wrong here.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.h:136: // lock as its predecessor
so that a thread can be pushed to
a *worker* can be

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.h:151: // Signaled when all
threads have been created.
all *workers*

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
File base/task_scheduler/scheduler_worker_pool_impl_unittest.cc (right):

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:102:
WAIT_FOR_ALL_THREADS_IDLE,
WAIT_FOR_ALL_WORKERS_IDLE

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:177:
TEST_P(TaskSchedulerWorkerPoolImplTest, PostTasksWaitAllThreadsIdle) {
WaitAllWorkersIdle

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:179: // up when new
tasks are posted, wait for all threads to become idle before
for all *workers* to become

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:222:
TEST_P(TaskSchedulerWorkerPoolImplTest, PostTasksWithOneAvailableThread) {
OneAvailableWorker

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:223: // Post blocking
tasks to keep all threads busy except one until |event| is
all *workers* busy

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:239: // that only one
thread in |worker_pool_| isn't busy.
one *worker*

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
File base/task_scheduler/scheduler_worker_stack_unittest.cc (right):

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_stack_unittest.cc:34: "The mock thread is
not expected to be woken before it is shutdown";
mock worker

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_stack_unittest.cc:42: thread_a_ =
SchedulerWorker::Create(
worker_a_

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_stack_unittest.cc:46: thread_b_ =
SchedulerWorker::Create(
worker_b_

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_stack_unittest.cc:50: thread_c_ =
SchedulerWorker::Create(
worker_c_

[robliao, 2016-06-20 17:50:09]

Done. Thanks!

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

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker.h:36: // the thread managed by the
SchedulerWorker instance.
On 2016/06/20 15:08:39, fdoray wrote:
> *a* thread

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker.h:41: // Called by |worker| when it enters
its main function.
On 2016/06/20 15:08:39, fdoray wrote:
> when *a thread it manages* enters

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker.h:53: // if the thread's WakeUp() method is
called.
On 2016/06/20 15:08:39, fdoray wrote:
> worker's

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
File base/task_scheduler/scheduler_worker_pool_impl.cc (right):

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.cc:186: // single-threaded
PriorityQueue's transactions. |index| will be appended to
On 2016/06/20 15:08:39, fdoray wrote:
> |index| will be appended to the pool name to label threads managed by the
worker
> for which this delegate is constructed.

Took a different approach for this comment. Updated.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.cc:300: // another thread after
|sequence| is inserted in |shared_priority_queue_|. If
On 2016/06/20 15:08:39, fdoray wrote:
> another *worker* after

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.cc:301: // we did wake up another
thread, we would waste resources by having more
On 2016/06/20 15:08:39, fdoray wrote:
> we did wake up another *worker*

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.cc:302: // threads trying to get
a Sequence from |shared_priority_queue_| than the
On 2016/06/20 15:08:39, fdoray wrote:
> *workers* trying to get

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.cc:387: // Wait for
|outer_->threads_created_| to avoid traversing
On 2016/06/20 15:08:39, fdoray wrote:
> workers_created_ (if you change the variable's name)

Done.

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

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.h:55: // Creates a
SchedulerWorkerPoolImpl labeled |name| with up to |max_threads|
On 2016/06/20 15:08:40, fdoray wrote:
> |max_workers| workers of priority ... ?
> 
> Just a suggestion, "thread" is not wrong here.

Yeah, I kept this as threads as it's more representative of what this should be.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.h:114: // The name of this worker
pool, used to label its worker threads.
On 2016/06/20 15:08:40, fdoray wrote:
> label its workers
> 
> Just a suggestion, "thread" is not wrong here.

I kept "worker threads" as this is the only place the label actually appears to
the developer.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.h:136: // lock as its predecessor
so that a thread can be pushed to
On 2016/06/20 15:08:39, fdoray wrote:
> a *worker* can be

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl.h:151: // Signaled when all
threads have been created.
On 2016/06/20 15:08:40, fdoray wrote:
> all *workers*

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
File base/task_scheduler/scheduler_worker_pool_impl_unittest.cc (right):

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:102:
WAIT_FOR_ALL_THREADS_IDLE,
On 2016/06/20 15:08:40, fdoray wrote:
> WAIT_FOR_ALL_WORKERS_IDLE

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:177:
TEST_P(TaskSchedulerWorkerPoolImplTest, PostTasksWaitAllThreadsIdle) {
On 2016/06/20 15:08:40, fdoray wrote:
> WaitAllWorkersIdle

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:179: // up when new
tasks are posted, wait for all threads to become idle before
On 2016/06/20 15:08:40, fdoray wrote:
> for all *workers* to become

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:222:
TEST_P(TaskSchedulerWorkerPoolImplTest, PostTasksWithOneAvailableThread) {
On 2016/06/20 15:08:40, fdoray wrote:
> OneAvailableWorker

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:223: // Post blocking
tasks to keep all threads busy except one until |event| is
On 2016/06/20 15:08:40, fdoray wrote:
> all *workers* busy

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:239: // that only one
thread in |worker_pool_| isn't busy.
On 2016/06/20 15:08:40, fdoray wrote:
> one *worker*

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
File base/task_scheduler/scheduler_worker_stack_unittest.cc (right):

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_stack_unittest.cc:34: "The mock thread is
not expected to be woken before it is shutdown";
On 2016/06/20 15:08:40, fdoray wrote:
> mock worker

Went ahead and removed this ADD_FAILURE. Calling GetSleepTimeout isn't
inherently bad for this test and the test does not care about the call ordering
of the delegate.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_stack_unittest.cc:42: thread_a_ =
SchedulerWorker::Create(
On 2016/06/20 15:08:40, fdoray wrote:
> worker_a_

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_stack_unittest.cc:46: thread_b_ =
SchedulerWorker::Create(
On 2016/06/20 15:08:40, fdoray wrote:
> worker_b_

Done.

https://codereview.chromium.org/2077093002/diff/1/base/task_scheduler/schedul...
base/task_scheduler/scheduler_worker_stack_unittest.cc:50: thread_c_ =
SchedulerWorker::Create(
On 2016/06/20 15:08:40, fdoray wrote:
> worker_c_

Done.

[robliao, 2016-06-22 00:38:19]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-22 00:38:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2077093002/20001

[commit-bot: I haz the power, 2016-06-22 02:16:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-22 02:16:08]

Dry run: This issue passed the CQ dry run.

[fdoray, 2016-06-22 17:17:23]

lgtm

[gab, 2016-06-22 17:18:13]

Rename rubberstamp lgtm (didn't look closely)

[robliao, 2016-06-22 17:18:46]

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

[robliao, 2016-06-22 17:18:47]

danakj@chromium.org: Please review this name change CL. Thanks!

[gab, 2016-06-22 17:23:40]

On 2016/06/22 17:18:47, robliao wrote:
> https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=danakj@chromium.org: Please
review this name change CL. Thanks!

I think it's okay to TBR Dana for the gyp/GN changes, she's got more than enough
real changes to look at already :-)

PS: Please add to CL description that the follow-up CL will introduce
SchedulerWorker::Thread hence this rename.

[robliao, 2016-06-22 17:26:42]

Description was changed from

==========
Rename SchedulerWorkerThread* to SchedulerWorker*

This rename is in preparation for virtualized Scheduler Workers.

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

to

==========
Rename SchedulerWorkerThread* to SchedulerWorker*

This rename is in preparation for virtualized Scheduler Workers which will
introduce SchedulerWorker::Thread.

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

[robliao, 2016-06-22 17:27:03]

Description was changed from

==========
Rename SchedulerWorkerThread* to SchedulerWorker*

This rename is in preparation for virtualized Scheduler Workers which will
introduce SchedulerWorker::Thread.

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

to

==========
Rename SchedulerWorkerThread* to SchedulerWorker*

This rename is in preparation for virtualized Scheduler Workers which will
introduce SchedulerWorker::Thread.

BUG=553459
TBR=danakj
==========

[robliao, 2016-06-22 17:27:22]

On 2016/06/22 17:23:40, gab wrote:
> On 2016/06/22 17:18:47, robliao wrote:
> > https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=danakj@chromium.org:
Please
> review this name change CL. Thanks!
> 
> I think it's okay to TBR Dana for the gyp/GN changes, she's got more than
enough
> real changes to look at already :-)
> 
> PS: Please add to CL description that the follow-up CL will introduce
> SchedulerWorker::Thread hence this rename.

Sounds good. Amended the CL description.

[robliao, 2016-06-22 18:17:35]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-22 18:18:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2077093002/20001

[commit-bot: I haz the power, 2016-06-22 18:27:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-22 18:27:50]

Dry run: This issue passed the CQ dry run.

[robliao, 2016-06-22 18:28:51]

The CQ bit was checked by robliao@chromium.org

[commit-bot: I haz the power, 2016-06-22 18:29:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2077093002/20001

[commit-bot: I haz the power, 2016-06-22 18:36:57]

Description was changed from

==========
Rename SchedulerWorkerThread* to SchedulerWorker*

This rename is in preparation for virtualized Scheduler Workers which will
introduce SchedulerWorker::Thread.

BUG=553459
TBR=danakj
==========

to

==========
Rename SchedulerWorkerThread* to SchedulerWorker*

This rename is in preparation for virtualized Scheduler Workers which will
introduce SchedulerWorker::Thread.

BUG=553459
TBR=danakj
==========

[commit-bot: I haz the power, 2016-06-22 18:36:59]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-22 18:39:24]

Description was changed from

==========
Rename SchedulerWorkerThread* to SchedulerWorker*

This rename is in preparation for virtualized Scheduler Workers which will
introduce SchedulerWorker::Thread.

BUG=553459
TBR=danakj
==========

to

==========
Rename SchedulerWorkerThread* to SchedulerWorker*

This rename is in preparation for virtualized Scheduler Workers which will
introduce SchedulerWorker::Thread.

BUG=553459
TBR=danakj

Committed: https://crrev.com/7a58439650da70e6b6cfedb80de104b55b092603
Cr-Commit-Position: refs/heads/master@{#401364}
==========

[commit-bot: I haz the power, 2016-06-22 18:39:26]

Patchset 2 (id:??) landed as
https://crrev.com/7a58439650da70e6b6cfedb80de104b55b092603
Cr-Commit-Position: refs/heads/master@{#401364}

[danakj, 2016-06-22 19:36:55]

LGTM