Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2841173002:
Add presubmit check against usage of BrowserThread::GetBlockingPool(). (Closed)
DescriptionAdd presubmit check against usage of BrowserThread::GetBlockingPool().
The blocking pool is deprecated. base/task_scheduler/post_task.h
should be used instead. More than 180 CLs to migrate the blocking
pool to base/task_scheduler/post_task.h have landed (ref. bug).
BUG=667892
Review-Url: https://codereview.chromium.org/2841173002
Cr-Commit-Position: refs/heads/master@{#468421}
Committed: https://chromium.googlesource.com/chromium/src/+/c4ac18d1c07180e543acbcc5308f811b5171ffd5
Patch Set 1 #
Total comments: 2
Patch Set 2 : CR-gab #
Total comments: 2
Patch Set 3 : CR-robliao-vocabulary #Patch Set 4 : fix #Messages
Total messages: 34 (22 generated)
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
fdoray@chromium.org changed reviewers: + gab@chromium.org, robliao@chromium.org
PTAL
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
fdoray@chromium.org changed reviewers: + brettw@chromium.org - gab@chromium.org
brettw@: PTAL
gab@chromium.org changed reviewers: + gab@chromium.org
https://codereview.chromium.org/2841173002/diff/1/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/2841173002/diff/1/PRESUBMIT.py#newcode326 PRESUBMIT.py:326: True, Does True here mean "block CQ" or "warning only"? I think this should still be warning only as code that needs to be in sync with yet to be migrated code can still use the BlockingPool legitimately if only to be on the right sequence (that should be mentioned in the presubmit comment as well)
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...
PTAnL https://codereview.chromium.org/2841173002/diff/1/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/2841173002/diff/1/PRESUBMIT.py#newcode326 PRESUBMIT.py:326: True, On 2017/04/28 01:58:24, gab wrote: > Does True here mean "block CQ" or "warning only"? I think this should still be > warning only as code that needs to be in sync with yet to be migrated code can > still use the BlockingPool legitimately if only to be on the right sequence > (that should be mentioned in the presubmit comment as well) Discussed offline. If a components already has code to get a SequencedTaskRunner from BrowserThread::GetBlockingPool(), this check won't prevent it from posting new tasks to it. If a component want to get a new SequencedTaskRunner or wants to post a parallel tasks, it should use the base/task_scheduler/post_task.h API.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm + nit https://codereview.chromium.org/2841173002/diff/20001/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/2841173002/diff/20001/PRESUBMIT.py#newcode327 PRESUBMIT.py:327: 'For questions, reach out to base/task_scheduler/OWNERS.', Nit: s/reach out to/contact/
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
https://codereview.chromium.org/2841173002/diff/20001/PRESUBMIT.py File PRESUBMIT.py (right): https://codereview.chromium.org/2841173002/diff/20001/PRESUBMIT.py#newcode327 PRESUBMIT.py:327: 'For questions, reach out to base/task_scheduler/OWNERS.', On 2017/05/01 18:03:01, robliao wrote: > Nit: s/reach out to/contact/ Done.
fdoray@chromium.org changed reviewers: + jam@chromium.org - brettw@chromium.org
jam@: Please take a look.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
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...
lgtm On 2017/05/01 18:05:48, fdoray wrote: > jam@: Please take a look.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by fdoray@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from robliao@chromium.org Link to the patchset: https://codereview.chromium.org/2841173002/#ps60001 (title: "fix")
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": 60001, "attempt_start_ts": 1493674280788630,
"parent_rev": "9451bd7ecb9c10fa982c98a67366b2390c9dcbd2", "commit_rev":
"c4ac18d1c07180e543acbcc5308f811b5171ffd5"}
Message was sent while issue was closed.
Description was changed from ========== Add presubmit check against usage of BrowserThread::GetBlockingPool(). The blocking pool is deprecated. base/task_scheduler/post_task.h should be used instead. More than 180 CLs to migrate the blocking pool to base/task_scheduler/post_task.h have landed (ref. bug). BUG=667892 ========== to ========== Add presubmit check against usage of BrowserThread::GetBlockingPool(). The blocking pool is deprecated. base/task_scheduler/post_task.h should be used instead. More than 180 CLs to migrate the blocking pool to base/task_scheduler/post_task.h have landed (ref. bug). BUG=667892 Review-Url: https://codereview.chromium.org/2841173002 Cr-Commit-Position: refs/heads/master@{#468421} Committed: https://chromium.googlesource.com/chromium/src/+/c4ac18d1c07180e543acbcc5308f... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/c4ac18d1c07180e543acbcc5308f... |
