Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2837863003:
Use TaskScheduler instead of blocking pool in plugin_service_impl.cc. (Closed)
DescriptionUse TaskScheduler instead of blocking pool in plugin_service_impl.cc.
The blocking pool is being deprecated in favor of TaskScheduler.
BUG=667892
R=boliu@chromium.org
Review-Url: https://codereview.chromium.org/2837863003
Cr-Commit-Position: refs/heads/master@{#467611}
Committed: https://chromium.googlesource.com/chromium/src/+/cc22a38d799e48411390446a9655a0ece8f5948a
Patch Set 1 #
Total comments: 2
Patch Set 2 : USER_VISIBLE #Messages
Total messages: 20 (12 generated)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
PTAL! This CL was generated automatically. Please make sure that the appropriate TaskTraits are used to post tasks. https://cs.chromium.org/chromium/src/base/task_scheduler/task_traits.h If everything looks good, please lgtm and CQ this CL. Otherwise, tell us what's wrong. Thanks! - FAQ - What if bots are red? Ignore this CL. A human will look at errors shortly. What if the task priority is not set explicitly (no .WithTaskPriority())? When there is no explicit priority, the priority is inherited from the calling context (e.g. a task posted from a BACKGROUND task without an explicit priority will have a BACKGROUND priority). What if the shutdown behavior is not set explicitly (no .WithShutdownBehavior())? If shutdown behavior is important for a task, it should be set explicitly. It's not necessary to specify it if you're fine with either BLOCK_SHUTDOWN or SKIP_ON_SHUTDOWN.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
can you tweak your owner picking logic to skip me for content/browser stuff? I'm only technically supposed to own android things.. https://codereview.chromium.org/2837863003/diff/1/content/browser/plugin_serv... File content/browser/plugin_service_impl.cc (right): https://codereview.chromium.org/2837863003/diff/1/content/browser/plugin_serv... content/browser/plugin_service_impl.cc:103: .WithPriority(base::TaskPriority::BACKGROUND) probably should be user visible, I assume this is a request to populate the plugins page, in which case that's a user waiting for the request
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: 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_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
+kinuko@chromium.org PTAL On 2017/04/24 18:41:25, boliu wrote: > can you tweak your owner picking logic to skip me for content/browser stuff? I'm > only technically supposed to own android things.. Done https://codereview.chromium.org/2837863003/diff/1/content/browser/plugin_serv... File content/browser/plugin_service_impl.cc (right): https://codereview.chromium.org/2837863003/diff/1/content/browser/plugin_serv... content/browser/plugin_service_impl.cc:103: .WithPriority(base::TaskPriority::BACKGROUND) On 2017/04/24 18:41:25, boliu wrote: > probably should be user visible, I assume this is a request to populate the > plugins page, in which case that's a user waiting for the request Done.
fdoray@chromium.org changed reviewers: + kinuko@chromium.org
PTAL! This CL was generated automatically. Please make sure that the appropriate TaskTraits are used to post tasks. https://cs.chromium.org/chromium/src/base/task_scheduler/task_traits.h If everything looks good, please lgtm and CQ this CL. Otherwise, tell us what's wrong. Thanks! - FAQ - What if bots are red? Ignore this CL. A human will look at errors shortly. What if the task priority is not set explicitly (no .WithTaskPriority())? When there is no explicit priority, the priority is inherited from the calling context (e.g. a task posted from a BACKGROUND task without an explicit priority will have a BACKGROUND priority). What if the shutdown behavior is not set explicitly (no .WithShutdownBehavior())? If shutdown behavior is important for a task, it should be set explicitly. It's not necessary to specify it if you're fine with either BLOCK_SHUTDOWN or SKIP_ON_SHUTDOWN.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm, I think
The CQ bit was checked by kinuko@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493275963682820,
"parent_rev": "2dd2610dc05678f182b268412a63150b5fd8eacb", "commit_rev":
"cc22a38d799e48411390446a9655a0ece8f5948a"}
Message was sent while issue was closed.
Description was changed from ========== Use TaskScheduler instead of blocking pool in plugin_service_impl.cc. The blocking pool is being deprecated in favor of TaskScheduler. BUG=667892 R=boliu@chromium.org ========== to ========== Use TaskScheduler instead of blocking pool in plugin_service_impl.cc. The blocking pool is being deprecated in favor of TaskScheduler. BUG=667892 R=boliu@chromium.org Review-Url: https://codereview.chromium.org/2837863003 Cr-Commit-Position: refs/heads/master@{#467611} Committed: https://chromium.googlesource.com/chromium/src/+/cc22a38d799e48411390446a9655... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/cc22a38d799e48411390446a9655... |
