Use TaskScheduler instead of blocking pool in google_update_metrics_provider_win.cc.

Use TaskScheduler instead of blocking pool in google_update_metrics_provider_win.cc.

The blocking pool is being deprecated in favor of TaskScheduler.

BUG=667892
R=rkaplow@chromium.org

Review-Url: https://codereview.chromium.org/2669923004
Cr-Commit-Position: refs/heads/master@{#448072}
Committed: https://chromium.googlesource.com/chromium/src/+/0d37fb09c9d1bf3657f67cd0c5ca916b8d122fe6

[fdoray, 2017-02-03 17:32:15]

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:32:15]

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:33:10]

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

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

[commit-bot: I haz the power, 2017-02-03 18:20:35]

Dry run: This issue passed the CQ dry run.

[rkaplow, 2017-02-03 20:59:55]

lgtm

[fdoray, 2017-02-03 21:02:40]

The CQ bit was checked by fdoray@chromium.org

[commit-bot: I haz the power, 2017-02-03 21:03:09]

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

[Alexei Svitkine (slow), 2017-02-03 21:05:16]

asvitkine@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Alexei Svitkine (slow), 2017-02-03 21:05:16]

https://codereview.chromium.org/2669923004/diff/20001/chrome/browser/metrics/...
File chrome/browser/metrics/google_update_metrics_provider_win.h (right):

https://codereview.chromium.org/2669923004/diff/20001/chrome/browser/metrics/...
chrome/browser/metrics/google_update_metrics_provider_win.h:53: static
GoogleUpdateMetrics GetGoogleUpdateDataBlocking();
Nit: This function name doesn't make sense to me anymore.

Previous name emphasized it was on blocking pool, i.e. where it should be run.
New name just says "Blocking" which I don't think makes sense. Also, comment is
still as before.

Please name it in a way that makes more sense.

[commit-bot: I haz the power, 2017-02-03 21:34:26]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486155760001640,
"parent_rev": "e5a44cc82a0be4276ade85df2b05cd130e9fd9dc", "commit_rev":
"0d37fb09c9d1bf3657f67cd0c5ca916b8d122fe6"}

[commit-bot: I haz the power, 2017-02-03 21:35:05]

Description was changed from

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

The blocking pool is being deprecated in favor of TaskScheduler.

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

to

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

The blocking pool is being deprecated in favor of TaskScheduler.

BUG=667892
R=rkaplow@chromium.org

Review-Url: https://codereview.chromium.org/2669923004
Cr-Commit-Position: refs/heads/master@{#448072}
Committed:
https://chromium.googlesource.com/chromium/src/+/0d37fb09c9d1bf3657f67cd0c5ca...
==========

[commit-bot: I haz the power, 2017-02-03 21:35:06]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/0d37fb09c9d1bf3657f67cd0c5ca...