Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2769803002:
Use TaskScheduler instead of blocking pool in chrome_metrics_services_manager_client.cc. (Closed)
DescriptionUse TaskScheduler instead of blocking pool in chrome_metrics_services_manager_client.cc.
The blocking pool is being deprecated in favor of TaskScheduler.
BUG=667892
R=rkaplow@chromium.org
Review-Url: https://codereview.chromium.org/2769803002
Cr-Commit-Position: refs/heads/master@{#467681}
Committed: https://chromium.googlesource.com/chromium/src/+/147e49528a22fadade88e8d4ef47391944cf1668
Patch Set 1 #Patch Set 2 : CR - no BLOCK_SHUTDOWN #Patch Set 3 : rebase #Messages
Total messages: 33 (20 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...
rkaplow@chromium.org changed reviewers: + asvitkine@chromium.org
not 100% sure about this - do we want this to block shutdown?
On 2017/03/22 20:18:12, rkaplow (slow) wrote: > not 100% sure about this - do we want this to block shutdown? Usually, tasks that persist data (writing to the registry in this case) should block shutdown. Are you fine with skipping this task if the browser is closed early?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I don't think this should block shutdown. The backup of UMA client id is done just in case - e.g. if we later need to restore it due to malware messing up the prefs file. It's nice to have, but not critical. I don't think blocking shutdown on it makes sense - it's not important enough for that.
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...
On 2017/03/22 20:34:30, Alexei Svitkine (slow) wrote: > I don't think this should block shutdown. > > The backup of UMA client id is done just in case - e.g. if we later need to > restore it due to malware messing up the prefs file. > > It's nice to have, but not critical. I don't think blocking shutdown on it makes > sense - it's not important enough for that. PTAnL
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
lgtm
The CQ bit was checked by fdoray@chromium.org
The CQ bit was unchecked by fdoray@chromium.org
The CQ bit was checked by fdoray@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
The CQ bit was unchecked by fdoray@chromium.org
The CQ bit was checked by fdoray@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rkaplow@chromium.org, asvitkine@chromium.org Link to the patchset: https://codereview.chromium.org/2769803002/#ps40001 (title: "rebase")
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Posting to the UI thread is no longer needed now that TaskScheduler is initialized early.
lgtm
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493301718894080,
"parent_rev": "1982eef8e01ab5e8ce6aeba827faa54c8e88838a", "commit_rev":
"147e49528a22fadade88e8d4ef47391944cf1668"}
Message was sent while issue was closed.
Description was changed from ========== Use TaskScheduler instead of blocking pool in chrome_metrics_services_manager_client.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 chrome_metrics_services_manager_client.cc. The blocking pool is being deprecated in favor of TaskScheduler. BUG=667892 R=rkaplow@chromium.org Review-Url: https://codereview.chromium.org/2769803002 Cr-Commit-Position: refs/heads/master@{#467681} Committed: https://chromium.googlesource.com/chromium/src/+/147e49528a22fadade88e8d4ef47... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/147e49528a22fadade88e8d4ef47... |
