Use TaskScheduler instead of blocking pool in render_widget_host_impl.cc.

Use TaskScheduler instead of blocking pool in render_widget_host_impl.cc.

The blocking pool is being deprecated in favor of TaskScheduler.

BUG=667892
R=sadrul@chromium.org

Review-Url: https://codereview.chromium.org/2677153002
Cr-Commit-Position: refs/heads/master@{#450340}
Committed: https://chromium.googlesource.com/chromium/src/+/8ee279b6b1fbde62f6f5603bc0f8e0c7bb098072

[fdoray, 2017-02-06 10:32:04]

Hi! I'm a Python script responsible for migrating tasks from the
blocking pool to TaskScheduler. In this CL, I used these traits to post to
TaskScheduler a task that was previously posted to the blocking pool:

.WithPriority(base::TaskPriority::BACKGROUND)
  User won't notice if the task takes an arbitrarily long time to complete.
  Making your task BACKGROUND allows non-BACKGROUND tasks to run faster :)

*No* .WithShutdownBehavior()

.MayBlock()
  The task may block. This includes but is not limited to tasks that wait on
  synchronous file I/O operations: read or write a file from disk, interact
  with a pipe or a socket, rename or delete a file, enumerate files in a
  directory, etc. This trait isn't required for the mere use of locks.

*No* .WithBaseSyncPrimitives()
  The task is not allowed to use these functions:
  - base::WaitableEvent::Wait
  - base::ConditionVariable::Wait
  - base::PlatformThread::Join
  - base::PlatformThread::Sleep
  - base::Process::WaitForExit
  - base::Process::WaitForExitWithTimeout

If you think this is correct, please LGTM and CQ this CL.

Otherwise, tell me which traits I should have used and I'll automatically
upload a new patch. You can find documentation at
https://cs.chromium.org/chromium/src/base/task_scheduler/task_traits.h

WithPriority()
[default/BACKGROUND/USER_VISIBLE/USER_BLOCKING]:

WithShutdownBehavior()
[default/CONTINUE_ON_SHUTDOWN/SKIP_ON_SHUTDOWN/BLOCK_SHUTDOWN]:

MayBlock()
[yes/no]:

WithBaseSyncPrimitives()
[yes/no]:

Comments for a human:
[A human will read this]

FAQ:

What should I do if the CQ dry run didn't pass?
You can ignore this CL. A human will take a look and get back to you.

Why are you doing this?
Browser threads, the blocking pool and base::WorkerPool are being deprecated
in favor of TaskScheduler. Design doc:
https://docs.google.com/document/d/1S2AAeoo1xa_vsLbDYBsDHCqhrkfiMgoIPlyRi6kxa...

What is the default priority?
A task posted to TaskScheduler without an explicit priority inherits its
priority from the calling context (e.g. a task posted to TaskScheduler
from a BACKGROUND task without an explicit .WithPriority() will have a
BACKGROUND priority).

What is the default shutdown behavior?
It is not documented on purpose. If your task shouldn't be skipped on shutdown
(e.g. a task that persists user data on disk), it should have an explicit
BLOCK_SHUTDOWN shutdown behavior. If it's important not to wait on your task
on shutdown (e.g. it takes a long time to run and could cause a shutdown
hang), it should have an explicit CONTINUE_ON_SHUTDOWN shutdown behavior.

[fdoray, 2017-02-06 10:32:05]

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

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

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-06 11:11:55]

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

[commit-bot: I haz the power, 2017-02-06 11:11:56]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[fdoray, 2017-02-06 13:40:31]

Hi! I'm a Python script responsible for migrating tasks from the
blocking pool to TaskScheduler. In this CL, I used these traits to post to
TaskScheduler a task that was previously posted to the blocking pool:

.WithPriority(base::TaskPriority::BACKGROUND)
  User won't notice if the task takes an arbitrarily long time to complete.
  Making your task BACKGROUND allows non-BACKGROUND tasks to run faster :)

*No* .WithShutdownBehavior()

.MayBlock()
  The task may block. This includes but is not limited to tasks that wait on
  synchronous file I/O operations: read or write a file from disk, interact
  with a pipe or a socket, rename or delete a file, enumerate files in a
  directory, etc. This trait isn't required for the mere use of locks.

*No* .WithBaseSyncPrimitives()
  The task is not allowed to use these functions:
  - base::WaitableEvent::Wait
  - base::ConditionVariable::Wait
  - base::PlatformThread::Join
  - base::PlatformThread::Sleep
  - base::Process::WaitForExit
  - base::Process::WaitForExitWithTimeout

If you think this is correct, please LGTM and CQ this CL.

Otherwise, tell me which traits I should have used and I'll automatically
upload a new patch. You can find documentation at
https://cs.chromium.org/chromium/src/base/task_scheduler/task_traits.h

WithPriority()
[default/BACKGROUND/USER_VISIBLE/USER_BLOCKING]:

WithShutdownBehavior()
[default/CONTINUE_ON_SHUTDOWN/SKIP_ON_SHUTDOWN/BLOCK_SHUTDOWN]:

MayBlock()
[yes/no]:

WithBaseSyncPrimitives()
[yes/no]:

Comments for a human:
[A human will read this]

FAQ:

What should I do if the CQ dry run didn't pass?
You can ignore this CL. A human will take a look and get back to you.

Why are you doing this?
Browser threads, the blocking pool and base::WorkerPool are being deprecated
in favor of TaskScheduler. Design doc:
https://docs.google.com/document/d/1S2AAeoo1xa_vsLbDYBsDHCqhrkfiMgoIPlyRi6kxa...

What is the default priority?
A task posted to TaskScheduler without an explicit priority inherits its
priority from the calling context (e.g. a task posted to TaskScheduler
from a BACKGROUND task without an explicit .WithPriority() will have a
BACKGROUND priority).

What is the default shutdown behavior?
It is not documented on purpose. If your task shouldn't be skipped on shutdown
(e.g. a task that persists user data on disk), it should have an explicit
BLOCK_SHUTDOWN shutdown behavior. If it's important not to wait on your task
on shutdown (e.g. it takes a long time to run and could cause a shutdown
hang), it should have an explicit CONTINUE_ON_SHUTDOWN shutdown behavior.

[fdoray, 2017-02-06 13:40:32]

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

[commit-bot: I haz the power, 2017-02-06 13:40:49]

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-06 15:11:44]

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

[commit-bot: I haz the power, 2017-02-06 15:11:45]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[sadrul, 2017-02-07 05:30:00]

The try failures look relevant. Could you look into those?

[fdoray, 2017-02-07 13:11:13]

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

[commit-bot: I haz the power, 2017-02-07 13:11:24]

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-07 13:36:38]

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

[commit-bot: I haz the power, 2017-02-07 13:36:39]

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...)

[fdoray, 2017-02-07 19:59:19]

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

[commit-bot: I haz the power, 2017-02-07 19:59:53]

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-07 20:55:31]

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

[commit-bot: I haz the power, 2017-02-07 20:55:32]

Dry run: This issue passed the CQ dry run.

[fdoray, 2017-02-08 12:38:50]

On 2017/02/07 05:30:00, sadrul wrote:
> The try failures look relevant. Could you look into those?

PTAnL

[fdoray, 2017-02-13 16:53:07]

On 2017/02/08 12:38:50, fdoray wrote:
> On 2017/02/07 05:30:00, sadrul wrote:
> > The try failures look relevant. Could you look into those?
> 
> PTAnL

ping

[sadrul, 2017-02-13 17:30:22]

On 2017/02/13 16:53:07, fdoray wrote:
> On 2017/02/08 12:38:50, fdoray wrote:
> > On 2017/02/07 05:30:00, sadrul wrote:
> > > The try failures look relevant. Could you look into those?
> > 
> > PTAnL
> 
> ping

Sorry, this got lost. Change in render_widget_host_impl.cc lgtm. Note though
that I am not an owner in the rest of the files.

[fdoray, 2017-02-13 18:06:53]

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

[fdoray, 2017-02-13 18:06:54]

jochen@: Please review trivial changes in chrome/ and content/

[jochen (gone - plz use gerrit), 2017-02-14 09:04:50]

lgtm

[fdoray, 2017-02-14 12:31:25]

The CQ bit was checked by fdoray@chromium.org

[commit-bot: I haz the power, 2017-02-14 12:31:50]

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-14 13:48:59]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487075485247140,
"parent_rev": "6908e6e6a69e1c5b11a7a2227d65a913208bdb95", "commit_rev":
"8ee279b6b1fbde62f6f5603bc0f8e0c7bb098072"}

[commit-bot: I haz the power, 2017-02-14 13:49:38]

Description was changed from

==========
Use TaskScheduler instead of blocking pool in render_widget_host_impl.cc.

The blocking pool is being deprecated in favor of TaskScheduler.

BUG=667892
R=sadrul@chromium.org
==========

to

==========
Use TaskScheduler instead of blocking pool in render_widget_host_impl.cc.

The blocking pool is being deprecated in favor of TaskScheduler.

BUG=667892
R=sadrul@chromium.org

Review-Url: https://codereview.chromium.org/2677153002
Cr-Commit-Position: refs/heads/master@{#450340}
Committed:
https://chromium.googlesource.com/chromium/src/+/8ee279b6b1fbde62f6f5603bc0f8...
==========

[commit-bot: I haz the power, 2017-02-14 13:49:39]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/8ee279b6b1fbde62f6f5603bc0f8...