|
|
Chromium Code Reviews
DescriptionChange SchedulerWorker::ShouldExit Checks Ordering
A SchedulerWorker may outlive the test environment when it's released
as a result of SingleThreadTaskRunner unregistration in the
SchedulerSingleThreadTaskRunnerManager. As a result, SchedulerWorker
should prefer checking its own fields first to validate that
it should continue to run before consulting the environment provided
TaskTracker.
BUG=705500
Review-Url: https://codereview.chromium.org/2781933003
Cr-Commit-Position: refs/heads/master@{#460475}
Committed: https://chromium.googlesource.com/chromium/src/+/624dc15f7cb6b404e607f536aef0c18d325903ee
Patch Set 1 #Patch Set 2 : Fix Wording #
Total comments: 2
Patch Set 3 : Fix Comment #Messages
Total messages: 22 (15 generated)
robliao@chromium.org changed reviewers: + fdoray@chromium.org, gab@chromium.org
The CQ bit was checked by robliao@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...
The CQ bit was checked by robliao@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by robliao@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
https://codereview.chromium.org/2781933003/diff/20001/base/task_scheduler/sch... File base/task_scheduler/scheduler_worker.cc (right): https://codereview.chromium.org/2781933003/diff/20001/base/task_scheduler/sch... base/task_scheduler/scheduler_worker.cc:335: // released and outlive task_tracker_ in unit tests. However, when the |task_tracker_|
lgtm with comment (see previous post)
lgtm
https://codereview.chromium.org/2781933003/diff/20001/base/task_scheduler/sch... File base/task_scheduler/scheduler_worker.cc (right): https://codereview.chromium.org/2781933003/diff/20001/base/task_scheduler/sch... base/task_scheduler/scheduler_worker.cc:335: // released and outlive task_tracker_ in unit tests. However, when the On 2017/03/29 12:16:32, fdoray wrote: > |task_tracker_| Done.
The CQ bit was checked by robliao@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org, fdoray@chromium.org Link to the patchset: https://codereview.chromium.org/2781933003/#ps40001 (title: "Fix Comment")
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": 40001, "attempt_start_ts": 1490808631819160,
"parent_rev": "02a0fe9ac80db197c640106ed6a36efbdec49e19", "commit_rev":
"624dc15f7cb6b404e607f536aef0c18d325903ee"}
Message was sent while issue was closed.
Description was changed from ========== Change SchedulerWorker::ShouldExit Checks Ordering A SchedulerWorker may outlive the test environment when it's released as a result of SingleThreadTaskRunner unregistration in the SchedulerSingleThreadTaskRunnerManager. As a result, SchedulerWorker should prefer checking its own fields first to validate that it should continue to run before consulting the environment provided TaskTracker. BUG=705500 ========== to ========== Change SchedulerWorker::ShouldExit Checks Ordering A SchedulerWorker may outlive the test environment when it's released as a result of SingleThreadTaskRunner unregistration in the SchedulerSingleThreadTaskRunnerManager. As a result, SchedulerWorker should prefer checking its own fields first to validate that it should continue to run before consulting the environment provided TaskTracker. BUG=705500 Review-Url: https://codereview.chromium.org/2781933003 Cr-Commit-Position: refs/heads/master@{#460475} Committed: https://chromium.googlesource.com/chromium/src/+/624dc15f7cb6b404e607f536aef0... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/624dc15f7cb6b404e607f536aef0... |
