Add TaskScheduler initialization arguments to ChildProcess.

Add TaskScheduler initialization arguments to ChildProcess.

Previously, the constructor of ChildProcess called
ChildProcess::InitializeTaskScheduler() to initialize TaskScheduler.
RenderProcessImpl overrode this method to initialize TaskScheduler
with custom arguments. Unfortunately, the constructor of a base
class cannot call methods from a derived class.

This CL removes the virtual ChildProcess::InitializeTaskScheduler()
method. RnederProcessImpl passes its custom TaskScheduler
initialization arguments to the constructor of ChildProcess.

BUG=664996
Review-Url: https://codereview.chromium.org/2687903003
Cr-Commit-Position: refs/heads/master@{#449785}
Committed: https://chromium.googlesource.com/chromium/src/+/31cc6f8573abb454414ab2bdd26b07980eac2f8c

[fdoray, 2017-02-09 17:22:05]

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

[fdoray, 2017-02-09 17:22:07]

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

[fdoray, 2017-02-09 17:22:08]

PTAL

[fdoray, 2017-02-09 17:22:19]

Description was changed from

==========
# Enter a description of the change.
Add TaskScheduler initialization arguments to ChildProcess.

Previously, the constructor of ChildProcess called
ChildProcess::InitializeTaskScheduler() to initialize TaskScheduler.
RenderProcessImpl overrode this method to initialize TaskScheduler
with custom arguments. Unfortunately, the constructor of a base
class cannot call methods from a derived class.

This CL removes the virtual ChildProcess::InitializeTaskScheduler()
method. RnederProcessImpl passes its custom TaskScheduler
initialization arguments to the constructor of ChildProcess.

BUG=664996
==========

to

==========
Add TaskScheduler initialization arguments to ChildProcess.

Previously, the constructor of ChildProcess called
ChildProcess::InitializeTaskScheduler() to initialize TaskScheduler.
RenderProcessImpl overrode this method to initialize TaskScheduler
with custom arguments. Unfortunately, the constructor of a base
class cannot call methods from a derived class.

This CL removes the virtual ChildProcess::InitializeTaskScheduler()
method. RnederProcessImpl passes its custom TaskScheduler
initialization arguments to the constructor of ChildProcess.

BUG=664996
==========

[commit-bot: I haz the power, 2017-02-09 17:22:44]

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-02-09 17:33:12]

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

[commit-bot: I haz the power, 2017-02-09 17:33:12]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[gab, 2017-02-09 17:43:45]

Nice catch!

lgtm % comments

https://codereview.chromium.org/2687903003/diff/1/content/renderer/render_pro...
File content/renderer/render_process_impl.cc (right):

https://codereview.chromium.org/2687903003/diff/1/content/renderer/render_pro...
content/renderer/render_process_impl.cc:138: : RenderProcess(worker_pool_params,
worker_pool_index_for_traits_callback),
std::move callback

https://codereview.chromium.org/2687903003/diff/1/content/renderer/render_pro...
File content/renderer/render_process_impl.h (right):

https://codereview.chromium.org/2687903003/diff/1/content/renderer/render_pro...
content/renderer/render_process_impl.h:25: static std::unique_ptr<RenderProcess>
Create();
Explain here why a static Create is required (i.e. similar explanation to your
CL desc).

[fdoray, 2017-02-10 14:08:40]

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

[commit-bot: I haz the power, 2017-02-10 14:08:52]

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-02-10 14:32:50]

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

[commit-bot: I haz the power, 2017-02-10 14:32:51]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fdoray, 2017-02-10 16:04:42]

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

[commit-bot: I haz the power, 2017-02-10 16:05:17]

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-02-10 17:20:00]

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

[commit-bot: I haz the power, 2017-02-10 17:20:02]

Dry run: This issue passed the CQ dry run.

[fdoray, 2017-02-10 18:47:11]

fdoray@chromium.org changed reviewers:
+ jam@chromium.org

[fdoray, 2017-02-10 18:47:12]

jam@: PTAL

https://codereview.chromium.org/2687903003/diff/1/content/renderer/render_pro...
File content/renderer/render_process_impl.cc (right):

https://codereview.chromium.org/2687903003/diff/1/content/renderer/render_pro...
content/renderer/render_process_impl.cc:138: : RenderProcess(worker_pool_params,
worker_pool_index_for_traits_callback),
On 2017/02/09 17:43:45, gab wrote:
> std::move callback

Done.

https://codereview.chromium.org/2687903003/diff/1/content/renderer/render_pro...
File content/renderer/render_process_impl.h (right):

https://codereview.chromium.org/2687903003/diff/1/content/renderer/render_pro...
content/renderer/render_process_impl.h:25: static std::unique_ptr<RenderProcess>
Create();
On 2017/02/09 17:43:45, gab wrote:
> Explain here why a static Create is required (i.e. similar explanation to your
> CL desc).

Done.

[jam, 2017-02-10 21:34:47]

lgtm

are you going to experiment with changing the IO thread priority?

[fdoray, 2017-02-10 23:22:53]

On 2017/02/10 21:34:47, jam wrote:
> lgtm
> 
> are you going to experiment with changing the IO thread priority?

No, that's not a short-term plan.

[fdoray, 2017-02-10 23:23:01]

The CQ bit was checked by fdoray@chromium.org

[fdoray, 2017-02-10 23:23:01]

The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org
Link to the patchset: https://codereview.chromium.org/2687903003/#ps40001
(title: "default")

[commit-bot: I haz the power, 2017-02-10 23:23:42]

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-02-10 23:31:14]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486768981726180,
"parent_rev": "a25f4dfb17a80bf34678a29bee5f107fba37a84d", "commit_rev":
"31cc6f8573abb454414ab2bdd26b07980eac2f8c"}

[commit-bot: I haz the power, 2017-02-10 23:36:33]

Description was changed from

==========
Add TaskScheduler initialization arguments to ChildProcess.

Previously, the constructor of ChildProcess called
ChildProcess::InitializeTaskScheduler() to initialize TaskScheduler.
RenderProcessImpl overrode this method to initialize TaskScheduler
with custom arguments. Unfortunately, the constructor of a base
class cannot call methods from a derived class.

This CL removes the virtual ChildProcess::InitializeTaskScheduler()
method. RnederProcessImpl passes its custom TaskScheduler
initialization arguments to the constructor of ChildProcess.

BUG=664996
==========

to

==========
Add TaskScheduler initialization arguments to ChildProcess.

Previously, the constructor of ChildProcess called
ChildProcess::InitializeTaskScheduler() to initialize TaskScheduler.
RenderProcessImpl overrode this method to initialize TaskScheduler
with custom arguments. Unfortunately, the constructor of a base
class cannot call methods from a derived class.

This CL removes the virtual ChildProcess::InitializeTaskScheduler()
method. RnederProcessImpl passes its custom TaskScheduler
initialization arguments to the constructor of ChildProcess.

BUG=664996

Review-Url: https://codereview.chromium.org/2687903003
Cr-Commit-Position: refs/heads/master@{#449785}
Committed:
https://chromium.googlesource.com/chromium/src/+/31cc6f8573abb454414ab2bdd26b...
==========

[commit-bot: I haz the power, 2017-02-10 23:36:38]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/31cc6f8573abb454414ab2bdd26b...

[jam, 2017-02-11 00:16:31]

On 2017/02/10 23:22:53, fdoray wrote:
> On 2017/02/10 21:34:47, jam wrote:
> > lgtm
> > 
> > are you going to experiment with changing the IO thread priority?
> 
> No, that's not a short-term plan.

so why is it an argument to ChildProcess?

[gab, 2017-02-13 15:01:36]

On 2017/02/11 00:16:31, jam wrote:
> On 2017/02/10 23:22:53, fdoray wrote:
> > On 2017/02/10 21:34:47, jam wrote:
> > > lgtm
> > > 
> > > are you going to experiment with changing the IO thread priority?
> > 
> > No, that's not a short-term plan.
> 
> so why is it an argument to ChildProcess?

@jam: I also initially read the change that way but note that that parameter was
already there in previous version of the constructor so this change is merely
keeping it and adding a few more