Generate TaskScheduler::InitParams from components/task_scheduler_util/.

Generate TaskScheduler::InitParams from components/task_scheduler_util/.

Initialization of TaskScheduler with a vector of
SchedulerWorkerPoolParams is being deprecated in favor of
initialization with a TaskScheduler::InitParams struct that contains
a fixed number of SchedulerWorkerPoolParams members.

BUG=690706
Review-Url: https://codereview.chromium.org/2788223002
Cr-Commit-Position: refs/heads/master@{#461698}
Committed: https://chromium.googlesource.com/chromium/src/+/fd083b7d9394059c80f9131b4874fc54662ee3bc

[fdoray, 2017-03-31 20:02:12]

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

[fdoray, 2017-03-31 20:02:13]

PTAL

[gab, 2017-04-03 16:20:10]

Interface looks fine to me. Deferring impl review to robliao to ease 14 reviews
on my plate this morning :)

[robliao, 2017-04-03 17:42:36]

https://codereview.chromium.org/2788223002/diff/20001/components/task_schedul...
File components/task_scheduler_util/common/variations_util.cc (right):

https://codereview.chromium.org/2788223002/diff/20001/components/task_schedul...
components/task_scheduler_util/common/variations_util.cc:68: if
(params->max_threads() < 0 ||
Break this into the individual failure cases:

if (params->max_threads() < 0) {
  DLOG(ERROR) << "Invalid max threads in the Worker Pool Descriptor: "
              << params->max_threads();
}

if (params->suggested...) {
  DLOG(ERROR) << "Invalid suggested reclaim time in the Worker Pool Descriptor:
"
              << params->suggested_reclaim_time();
}

https://codereview.chromium.org/2788223002/diff/20001/components/task_schedul...
components/task_scheduler_util/common/variations_util.cc:69:
params->suggested_reclaim_time() > base::TimeDelta()) {
Shouldn't this be
params->suggested_reclaim_time() < base::TimeDelta()?

https://codereview.chromium.org/2788223002/diff/20001/components/task_schedul...
File components/task_scheduler_util/common/variations_util.h (left):

https://codereview.chromium.org/2788223002/diff/20001/components/task_schedul...
components/task_scheduler_util/common/variations_util.h:51: const
std::map<std::string, std::string>& variation_params);
The browser and renderer have the same pool configuration aside from the names.
A different way to structure this is to keep this function here and just have it
take both the variation params and the prefix to use for the name. Thoughts?

This way StringToWorkerPoolParams doesn't need to be exposed.

[robliao, 2017-04-03 18:31:50]

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

[commit-bot: I haz the power, 2017-04-03 18:33:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 18:45:04]

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

[commit-bot: I haz the power, 2017-04-03 18:45:05]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[fdoray, 2017-04-03 18:47:52]

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

[commit-bot: I haz the power, 2017-04-03 18:49:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fdoray, 2017-04-03 19:00:58]

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

[commit-bot: I haz the power, 2017-04-03 19:01:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fdoray, 2017-04-03 19:01:44]

PTAnL

https://codereview.chromium.org/2788223002/diff/20001/components/task_schedul...
File components/task_scheduler_util/common/variations_util.cc (right):

https://codereview.chromium.org/2788223002/diff/20001/components/task_schedul...
components/task_scheduler_util/common/variations_util.cc:68: if
(params->max_threads() < 0 ||
On 2017/04/03 17:42:36, robliao wrote:
> Break this into the individual failure cases:
> 
> if (params->max_threads() < 0) {
>   DLOG(ERROR) << "Invalid max threads in the Worker Pool Descriptor: "
>               << params->max_threads();
> }
> 
> if (params->suggested...) {
>   DLOG(ERROR) << "Invalid suggested reclaim time in the Worker Pool
Descriptor:
> "
>               << params->suggested_reclaim_time();
> }

Done.

https://codereview.chromium.org/2788223002/diff/20001/components/task_schedul...
components/task_scheduler_util/common/variations_util.cc:69:
params->suggested_reclaim_time() > base::TimeDelta()) {
On 2017/04/03 17:42:35, robliao wrote:
> Shouldn't this be
> params->suggested_reclaim_time() < base::TimeDelta()?

Done.

https://codereview.chromium.org/2788223002/diff/20001/components/task_schedul...
File components/task_scheduler_util/common/variations_util.h (left):

https://codereview.chromium.org/2788223002/diff/20001/components/task_schedul...
components/task_scheduler_util/common/variations_util.h:51: const
std::map<std::string, std::string>& variation_params);
On 2017/04/03 17:42:36, robliao wrote:
> The browser and renderer have the same pool configuration aside from the
names.
> A different way to structure this is to keep this function here and just have
it
> take both the variation params and the prefix to use for the name. Thoughts?
> 
> This way StringToWorkerPoolParams doesn't need to be exposed.

Done.

[robliao, 2017-04-03 19:13:12]

lgtm + Commit and nits. Thanks!

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
File components/task_scheduler_util/browser/initialization.cc (right):

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
components/task_scheduler_util/browser/initialization.cc:23: enum WorkerPoolType
: size_t {
This can be removed.

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
File components/task_scheduler_util/common/variations_util.cc (right):

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
components/task_scheduler_util/common/variations_util.cc:96: if
(params->suggested_reclaim_time() <= base::TimeDelta()) {
While undesirable, it is still legal to have a suggested reclaim time of 0. It
just means reclaim immediately on idle.

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
File components/task_scheduler_util/common/variations_util.h (right):

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
components/task_scheduler_util/common/variations_util.h:23: // Builds a
TaskScheduler::InitParams from pool descriptors in |variations| that
Nit: in |variations| -> from |variations_params|

[fdoray, 2017-04-03 19:21:20]

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

[fdoray, 2017-04-03 19:21:45]

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
File components/task_scheduler_util/browser/initialization.cc (right):

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
components/task_scheduler_util/browser/initialization.cc:23: enum WorkerPoolType
: size_t {
On 2017/04/03 19:13:11, robliao wrote:
> This can be removed.

Still used in BrowserWorkerPoolIndexForTraits.

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
File components/task_scheduler_util/common/variations_util.cc (right):

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
components/task_scheduler_util/common/variations_util.cc:96: if
(params->suggested_reclaim_time() <= base::TimeDelta()) {
On 2017/04/03 19:13:11, robliao wrote:
> While undesirable, it is still legal to have a suggested reclaim time of 0. It
> just means reclaim immediately on idle.

Done.

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
File components/task_scheduler_util/common/variations_util.h (right):

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
components/task_scheduler_util/common/variations_util.h:23: // Builds a
TaskScheduler::InitParams from pool descriptors in |variations| that
On 2017/04/03 19:13:12, robliao wrote:
> Nit: in |variations| -> from |variations_params|

Done.

[commit-bot: I haz the power, 2017-04-03 19:21:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fdoray, 2017-04-03 19:22:20]

The CQ bit was unchecked by fdoray@chromium.org

[fdoray, 2017-04-03 19:22:24]

The CQ bit was checked by fdoray@chromium.org

[fdoray, 2017-04-03 19:22:25]

The patchset sent to the CQ was uploaded after l-g-t-m from robliao@chromium.org
Link to the patchset: https://codereview.chromium.org/2788223002/#ps80001
(title: "CR-robliao-14")

[commit-bot: I haz the power, 2017-04-03 19:23:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[robliao, 2017-04-03 19:23:46]

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
File components/task_scheduler_util/browser/initialization.cc (right):

https://codereview.chromium.org/2788223002/diff/60001/components/task_schedul...
components/task_scheduler_util/browser/initialization.cc:23: enum WorkerPoolType
: size_t {
On 2017/04/03 19:21:45, fdoray wrote:
> On 2017/04/03 19:13:11, robliao wrote:
> > This can be removed.
> 
> Still used in BrowserWorkerPoolIndexForTraits.

Indeed. My mistake!

[commit-bot: I haz the power, 2017-04-03 19:52:37]

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

[commit-bot: I haz the power, 2017-04-03 19:52:38]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[fdoray, 2017-04-03 21:08:02]

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

[fdoray, 2017-04-03 21:08:12]

The CQ bit was unchecked by fdoray@chromium.org

[fdoray, 2017-04-03 21:08:16]

The CQ bit was checked by fdoray@chromium.org

[fdoray, 2017-04-03 21:08:17]

The patchset sent to the CQ was uploaded after l-g-t-m from robliao@chromium.org
Link to the patchset: https://codereview.chromium.org/2788223002/#ps100001
(title: "fix-test-error")

[commit-bot: I haz the power, 2017-04-03 21:08:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 21:18:10]

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

[commit-bot: I haz the power, 2017-04-03 21:18:11]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[fdoray, 2017-04-04 12:21:59]

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

[commit-bot: I haz the power, 2017-04-04 12:22:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fdoray, 2017-04-04 12:24:16]

The CQ bit was unchecked by fdoray@chromium.org

[fdoray, 2017-04-04 12:24:16]

The CQ bit was checked by fdoray@chromium.org

[fdoray, 2017-04-04 12:24:17]

The patchset sent to the CQ was uploaded after l-g-t-m from robliao@chromium.org
Link to the patchset: https://codereview.chromium.org/2788223002/#ps120001
(title: "fix-build-error")

[commit-bot: I haz the power, 2017-04-04 12:24:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 13:45:05]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1491308656864580,
"parent_rev": "442d3691fdff217b0b34e3f59ae88c3a4ec7f093", "commit_rev":
"fd083b7d9394059c80f9131b4874fc54662ee3bc"}

[commit-bot: I haz the power, 2017-04-04 13:46:01]

Description was changed from

==========
Generate TaskScheduler::InitParams from components/task_scheduler_util/.

Initialization of TaskScheduler with a vector of
SchedulerWorkerPoolParams is being deprecated in favor of
initialization with a TaskScheduler::InitParams struct that contains
a fixed number of SchedulerWorkerPoolParams members.

BUG=690706
==========

to

==========
Generate TaskScheduler::InitParams from components/task_scheduler_util/.

Initialization of TaskScheduler with a vector of
SchedulerWorkerPoolParams is being deprecated in favor of
initialization with a TaskScheduler::InitParams struct that contains
a fixed number of SchedulerWorkerPoolParams members.

BUG=690706

Review-Url: https://codereview.chromium.org/2788223002
Cr-Commit-Position: refs/heads/master@{#461698}
Committed:
https://chromium.googlesource.com/chromium/src/+/fd083b7d9394059c80f9131b4874...
==========

[commit-bot: I haz the power, 2017-04-04 13:46:02]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/fd083b7d9394059c80f9131b4874...