Use TaskScheduler instead of blocking pool in printer_backend_proxy_chromeos.cc.

Use TaskScheduler instead of blocking pool in printer_backend_proxy_chromeos.cc.

The blocking pool is being deprecated in favor of TaskScheduler.

BUG=667892
R=vandebo@chromium.org

Review-Url: https://codereview.chromium.org/2675773004
Cr-Commit-Position: refs/heads/master@{#466831}
Committed: https://chromium.googlesource.com/chromium/src/+/b46b4c70b81ba49928b1a0eb5502d89bcfba2956

[fdoray, 2017-02-03 17:58:45]

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-03 17:58:46]

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

[commit-bot: I haz the power, 2017-02-03 17:59: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-03 18:14:22]

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

[commit-bot: I haz the power, 2017-02-03 18:14:23]

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

[fdoray, 2017-02-22 20:17:55]

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

[commit-bot: I haz the power, 2017-02-22 20:18: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-22 20:23:50]

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

[commit-bot: I haz the power, 2017-02-22 20:23:51]

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...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  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...)
  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_...)

[fdoray, 2017-02-24 20:03:10]

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

[commit-bot: I haz the power, 2017-02-24 20:04:02]

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

[fdoray, 2017-02-24 20:04:43]

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

[commit-bot: I haz the power, 2017-02-24 20:05:14]

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-24 20:24:20]

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

[commit-bot: I haz the power, 2017-02-24 20:24:21]

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

[fdoray, 2017-04-24 13:08:03]

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

[commit-bot: I haz the power, 2017-04-24 13:08:36]

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-24 13:24:42]

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

[commit-bot: I haz the power, 2017-04-24 13:24:43]

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

[fdoray, 2017-04-24 13:31:51]

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

[commit-bot: I haz the power, 2017-04-24 13:32:21]

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-24 13:59:33]

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

[commit-bot: I haz the power, 2017-04-24 13:59:34]

Dry run: This issue passed the CQ dry run.

[fdoray, 2017-04-24 15:02:54]

Description was changed from

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

The blocking pool is being deprecated in favor of TaskScheduler.

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

to

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

The blocking pool is being deprecated in favor of TaskScheduler.

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

[fdoray, 2017-04-24 15:02:54]

fdoray@chromium.org changed reviewers:
+ gene@chromium.org
- vandebo@chromium.org

[fdoray, 2017-04-24 15:03:08]

+gene@chromium.org
PTAL

On 2017/02/03 17:58:45, fdoray wrote:
> 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.

[chromium-reviews, 2017-04-24 17:44:27]

could you please add vitalybuka@, he might be a better reviewer here

On Mon, Apr 24, 2017 at 8:03 AM, <fdoray@chromium.org> wrote:

> +gene@chromium.org
> PTAL
>
> On 2017/02/03 17:58:45, fdoray wrote:
> > 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_
> vsLbDYBsDHCqhrkfiMgoIPlyRi6kxa5k/edit?usp=sharing
> >
> > 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.
>
>
>
> https://codereview.chromium.org/2675773004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[fdoray, 2017-04-24 17:48:14]

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

[fdoray, 2017-04-24 17:48:15]

vitalybuka@: PTAL

[Vitaly Buka corp, 2017-04-24 23:45:37]

vitalybuka@google.com changed reviewers:
+ vitalybuka@google.com

[Vitaly Buka corp, 2017-04-24 23:45:37]

The CQ bit was checked by vitalybuka@google.com

[Vitaly Buka corp, 2017-04-24 23:45:38]

lgtm

[commit-bot: I haz the power, 2017-04-24 23:46:03]

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-24 23:46:04]

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

[commit-bot: I haz the power, 2017-04-24 23:46:05]

No L-G-T-M from a valid reviewer yet.
CQ run can only be started once the patch has received an L-G-T-M from a full
committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[Vitaly Buka (NO REVIEWS), 2017-04-24 23:55:18]

The CQ bit was checked by vitalybuka@chromium.org

[Vitaly Buka (NO REVIEWS), 2017-04-24 23:55:19]

lgtm

[commit-bot: I haz the power, 2017-04-24 23:55:58]

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-25 00:00:14]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1493078118551190,
"parent_rev": "27ae42407b0effd81dd43d7542305f5cc1aa1d58", "commit_rev":
"b46b4c70b81ba49928b1a0eb5502d89bcfba2956"}

[commit-bot: I haz the power, 2017-04-25 00:00:23]

Description was changed from

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

The blocking pool is being deprecated in favor of TaskScheduler.

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

to

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

The blocking pool is being deprecated in favor of TaskScheduler.

BUG=667892
R=vandebo@chromium.org

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

[commit-bot: I haz the power, 2017-04-25 00:00:30]

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