Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(247)

Issue 1911493002: TaskScheduler: Move TaskFactory out of scheduler_thread_pool_unittest.cc (Closed)

Created:
4 years, 8 months ago by fdoray
Modified:
4 years, 8 months ago
Reviewers:
robliao, danakj
CC:
chromium-reviews, gab+watch_chromium.org, robliao+watch_chromium.org, fdoray+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

TaskScheduler: Move TaskFactory out of scheduler_thread_pool_unittest.cc This will allow the factory to be reused to test TaskSchedulerImpl. TBR=danakj@chromium.org BUG=553459 Committed: https://crrev.com/44fda214399b68ac4a4d7cc07feaf52253c59e7e Cr-Commit-Position: refs/heads/master@{#389491}

Patch Set 1 #

Total comments: 8

Patch Set 2 : CR robliao #3 (rename enum, expect in test file rather than util file, namespace test) #

Patch Set 3 : added comments #

Patch Set 4 : self review #

Patch Set 5 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+235 lines, -134 lines) Patch
M base/BUILD.gn View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M base/base.gyp View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M base/task_scheduler/scheduler_thread_pool_unittest.cc View 1 10 chunks +56 lines, -134 lines 0 comments Download
A base/task_scheduler/test_task_factory.h View 1 2 3 1 chunk +93 lines, -0 lines 0 comments Download
A base/task_scheduler/test_task_factory.cc View 1 2 1 chunk +82 lines, -0 lines 0 comments Download

Messages

Total messages: 30 (12 generated)
fdoray
Can you review this CL? Thanks.
4 years, 8 months ago (2016-04-20 17:29:07 UTC) #2
robliao
https://codereview.chromium.org/1911493002/diff/1/base/task_scheduler/scheduler_thread_pool_unittest.cc File base/task_scheduler/scheduler_thread_pool_unittest.cc (right): https://codereview.chromium.org/1911493002/diff/1/base/task_scheduler/scheduler_thread_pool_unittest.cc#newcode80 base/task_scheduler/scheduler_thread_pool_unittest.cc:80: YES, Maybe this can be enum class ThreadIdleBehavior { ...
4 years, 8 months ago (2016-04-21 01:38:18 UTC) #3
fdoray
robliao@: Can you take another look? Note: I prefer to get reviews on these CLs ...
4 years, 8 months ago (2016-04-22 15:54:03 UTC) #4
gab
One reviewer is enough for this one? Leaving it to Rob unless requested otherwise :-)
4 years, 8 months ago (2016-04-22 16:53:54 UTC) #6
robliao
On 2016/04/22 16:53:54, gab wrote: > One reviewer is enough for this one? Leaving it ...
4 years, 8 months ago (2016-04-22 17:09:42 UTC) #7
robliao
lgtm https://codereview.chromium.org/1911493002/diff/1/base/task_scheduler/test_task_factory.cc File base/task_scheduler/test_task_factory.cc (right): https://codereview.chromium.org/1911493002/diff/1/base/task_scheduler/test_task_factory.cc#newcode33 base/task_scheduler/test_task_factory.cc:33: EXPECT_TRUE(task_runner_->PostTask( On 2016/04/22 15:54:03, fdoray wrote: > On ...
4 years, 8 months ago (2016-04-22 23:04:19 UTC) #8
fdoray
https://codereview.chromium.org/1911493002/diff/1/base/task_scheduler/test_task_factory.cc File base/task_scheduler/test_task_factory.cc (right): https://codereview.chromium.org/1911493002/diff/1/base/task_scheduler/test_task_factory.cc#newcode33 base/task_scheduler/test_task_factory.cc:33: EXPECT_TRUE(task_runner_->PostTask( On 2016/04/22 23:04:19, robliao wrote: > On 2016/04/22 ...
4 years, 8 months ago (2016-04-25 14:25:02 UTC) #9
fdoray
Can you review this CL? Thanks.
4 years, 8 months ago (2016-04-25 14:27:52 UTC) #14
fdoray
danakj@: Can you review this CL? Thanks.
4 years, 8 months ago (2016-04-25 14:28:08 UTC) #15
gab
On 2016/04/25 14:28:08, fdoray wrote: > danakj@: Can you review this CL? Thanks. I think ...
4 years, 8 months ago (2016-04-25 15:12:21 UTC) #16
fdoray
On 2016/04/25 15:12:21, gab wrote: > On 2016/04/25 14:28:08, fdoray wrote: > > danakj@: Can ...
4 years, 8 months ago (2016-04-25 15:13:10 UTC) #17
gab
On 2016/04/25 15:13:10, fdoray wrote: > On 2016/04/25 15:12:21, gab wrote: > > On 2016/04/25 ...
4 years, 8 months ago (2016-04-25 15:16:33 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1911493002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1911493002/80001
4 years, 8 months ago (2016-04-25 15:17:35 UTC) #21
commit-bot: I haz the power
Try jobs failed on following builders: chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-generic_chromium_compile_only_ng/builds/127113)
4 years, 8 months ago (2016-04-25 15:24:02 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1911493002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1911493002/80001
4 years, 8 months ago (2016-04-25 16:02:13 UTC) #25
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years, 8 months ago (2016-04-25 16:25:00 UTC) #27
commit-bot: I haz the power
Patchset 5 (id:??) landed as https://crrev.com/44fda214399b68ac4a4d7cc07feaf52253c59e7e Cr-Commit-Position: refs/heads/master@{#389491}
4 years, 8 months ago (2016-04-25 16:27:10 UTC) #29
danakj
4 years, 8 months ago (2016-04-25 18:49:10 UTC) #30
Message was sent while issue was closed.
LGTM

Powered by Google App Engine
This is Rietveld 408576698