Chromium Code Reviews
Help | Chromium Project | Sign in
(32)

Issue 2829083002: Add constexpr TaskTraits constructor.

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 2 days ago by fdoray
Modified:
18 hours, 12 minutes ago
Reviewers:
robliao, etipdoray, gab, awong, danakj
CC:
chromium-reviews, gab+watch_chromium.org, danakj+watch_chromium.org, fdoray+watch_chromium.org, robliao+watch_chromium.org, vmpstr+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add constexpr TaskTraits constructor. The new constructor accepts any number of TaskTraits in any order. E.g. constexpr TaskTraits default_traits; constexpr TaskTraits user_visible_traits = {TaskPriority::USER_VISIBLE}; constexpr TaskTraits user_visible_may_block_traits = { TaskPriority::USER_VISIBLE, MayBlock()}; constexpr TaskTraits other_user_visible_may_block_traits = { MayBlock(), TaskPriority::USER_VISIBLE}; BUG=713683

Patch Set 1 #

Patch Set 2 : self-review #

Patch Set 3 : rebase #

Total comments: 2

Patch Set 4 : fix android build error #

Patch Set 5 : no-compile #

Patch Set 6 : do-not-return-void-from-constexpr #

Patch Set 7 : fix unused warning #

Total comments: 10

Patch Set 8 : CR-robliao-28-add-details-file #

Total comments: 24

Patch Set 9 : rebase #

Patch Set 10 : CR-etipdoray-gab-36-38 #

Total comments: 4

Patch Set 11 : CR-robliao-48 #

Total comments: 2

Patch Set 12 : tag-dispatching-first #

Patch Set 13 : tag-dispatching-first #

Patch Set 14 : fix build error #

Patch Set 15 : by-value #

Patch Set 16 : delete-move-constructor #

Patch Set 17 : fix-android-build-error #

Total comments: 1

Patch Set 18 : unretained #

Total comments: 4

Patch Set 19 : sfinae #

Total comments: 8

Patch Set 20 : CR gab #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+351 lines, -69 lines) Patch
M base/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 3 chunks +3 lines, -0 lines 0 comments Download
M base/task_scheduler/task_traits.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 2 chunks +109 lines, -66 lines 2 comments Download
M base/task_scheduler/task_traits.cc View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -3 lines 0 comments Download
A base/task_scheduler/task_traits_details.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +127 lines, -0 lines 1 comment Download
A base/task_scheduler/task_traits_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +81 lines, -0 lines 0 comments Download
A base/task_scheduler/task_traits_unittest.nc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +31 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 103 (78 generated)
fdoray
PTAL
1 week, 1 day ago (2017-04-21 16:53:15 UTC) #26
robliao
Pretty cool! A quick light pass while we get the file structure worked out (and ...
1 week, 1 day ago (2017-04-21 18:02:06 UTC) #27
robliao
https://codereview.chromium.org/2829083002/diff/110001/base/task_scheduler/task_traits.h File base/task_scheduler/task_traits.h (right): https://codereview.chromium.org/2829083002/diff/110001/base/task_scheduler/task_traits.h#newcode121 base/task_scheduler/task_traits.h:121: namespace internal { On 2017/04/21 18:02:06, robliao wrote: > ...
1 week, 1 day ago (2017-04-21 18:03:13 UTC) #28
fdoray
PTAnL https://codereview.chromium.org/2829083002/diff/110001/base/task_scheduler/task_traits.h File base/task_scheduler/task_traits.h (right): https://codereview.chromium.org/2829083002/diff/110001/base/task_scheduler/task_traits.h#newcode87 base/task_scheduler/task_traits.h:87: // WithBaseSyncPrimitives(). On 2017/04/21 18:02:06, robliao wrote: > ...
1 week, 1 day ago (2017-04-21 20:35:20 UTC) #31
fdoray
https://codereview.chromium.org/2829083002/diff/110001/base/task_scheduler/task_traits.h File base/task_scheduler/task_traits.h (right): https://codereview.chromium.org/2829083002/diff/110001/base/task_scheduler/task_traits.h#newcode107 base/task_scheduler/task_traits.h:107: // Avoid creating threads. Instead, use On 2017/04/21 20:35:20, ...
1 week, 1 day ago (2017-04-21 20:43:18 UTC) #32
etipdoray
I'm adding my opinion on CheckNoArgOfType. https://codereview.chromium.org/2829083002/diff/130001/base/task_scheduler/task_traits.h File base/task_scheduler/task_traits.h (right): https://codereview.chromium.org/2829083002/diff/130001/base/task_scheduler/task_traits.h#newcode175 base/task_scheduler/task_traits.h:175: bool may_block() const ...
5 days, 3 hours ago (2017-04-24 20:22:03 UTC) #36
gab
This is awesome, thanks :)! +ajwong https://codereview.chromium.org/2829083002/diff/40001/base/task_scheduler/task_traits.h File base/task_scheduler/task_traits.h (right): https://codereview.chromium.org/2829083002/diff/40001/base/task_scheduler/task_traits.h#newcode131 base/task_scheduler/task_traits.h:131: // calls back ...
4 days, 5 hours ago (2017-04-25 17:48:15 UTC) #38
fdoray
PTAnL https://codereview.chromium.org/2829083002/diff/40001/base/task_scheduler/task_traits.h File base/task_scheduler/task_traits.h (right): https://codereview.chromium.org/2829083002/diff/40001/base/task_scheduler/task_traits.h#newcode131 base/task_scheduler/task_traits.h:131: // calls back GetTraitFromArgList() with CallFirstTag() as first ...
3 days, 6 hours ago (2017-04-26 17:20:33 UTC) #45
robliao
https://codereview.chromium.org/2829083002/diff/170001/base/task_scheduler/task_traits.h File base/task_scheduler/task_traits.h (right): https://codereview.chromium.org/2829083002/diff/170001/base/task_scheduler/task_traits.h#newcode196 base/task_scheduler/task_traits.h:196: // The constructor only accepts types that are convertible ...
3 days, 3 hours ago (2017-04-26 19:45:54 UTC) #48
fdoray
please take another look https://codereview.chromium.org/2829083002/diff/170001/base/task_scheduler/task_traits.h File base/task_scheduler/task_traits.h (right): https://codereview.chromium.org/2829083002/diff/170001/base/task_scheduler/task_traits.h#newcode196 base/task_scheduler/task_traits.h:196: // The constructor only accepts ...
3 days, 2 hours ago (2017-04-26 20:25:14 UTC) #51
robliao
lgtm % fixes for compiler. https://codereview.chromium.org/2829083002/diff/190001/base/task_scheduler/task_traits_details.h File base/task_scheduler/task_traits_details.h (right): https://codereview.chromium.org/2829083002/diff/190001/base/task_scheduler/task_traits_details.h#newcode74 base/task_scheduler/task_traits_details.h:74: struct CallSecondTag {}; These ...
3 days, 2 hours ago (2017-04-26 21:15:54 UTC) #54
fdoray
https://codereview.chromium.org/2829083002/diff/190001/base/task_scheduler/task_traits_details.h File base/task_scheduler/task_traits_details.h (right): https://codereview.chromium.org/2829083002/diff/190001/base/task_scheduler/task_traits_details.h#newcode74 base/task_scheduler/task_traits_details.h:74: struct CallSecondTag {}; On 2017/04/26 21:15:54, robliao wrote: > ...
2 days, 9 hours ago (2017-04-27 13:26:00 UTC) #57
etipdoray
There seems to be some problems with overloading constructors. Here's a possible solution! https://codereview.chromium.org/2829083002/diff/310001/base/task_scheduler/task_traits.h File ...
2 days ago (2017-04-27 22:27:39 UTC) #76
robliao
On 2017/04/27 22:27:39, etipdoray wrote: > There seems to be some problems with overloading constructors. ...
2 days ago (2017-04-27 22:29:37 UTC) #77
etipdoray
On 2017/04/27 22:29:37, robliao wrote: > On 2017/04/27 22:27:39, etipdoray wrote: > > There seems ...
2 days ago (2017-04-27 22:49:11 UTC) #78
danakj
On Thu, Apr 27, 2017 at 6:49 PM, <etipdoray@gmail.com> wrote: > On 2017/04/27 22:29:37, robliao ...
2 days ago (2017-04-27 22:56:35 UTC) #79
etipdoray
On 2017/04/27 22:56:35, danakj wrote: > On Thu, Apr 27, 2017 at 6:49 PM, <mailto:etipdoray@gmail.com> ...
1 day, 8 hours ago (2017-04-28 15:00:36 UTC) #84
etipdoray
lgtm
1 day, 8 hours ago (2017-04-28 15:01:02 UTC) #85
fdoray
After investigation, it seems like the problem is that on iOS and Android, copying an ...
1 day, 6 hours ago (2017-04-28 16:58:03 UTC) #86
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2829083002/320001
1 day, 6 hours ago (2017-04-28 16:59:02 UTC) #89
commit-bot: I haz the power
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_presubmit/builds/423395)
1 day, 6 hours ago (2017-04-28 17:09:16 UTC) #91
gab
nits on impl, looking at latest version now after discussion to make Bind() work. https://codereview.chromium.org/2829083002/diff/320001/base/task_scheduler/task_traits.h ...
1 day, 5 hours ago (2017-04-28 18:03:41 UTC) #93
gab
Nice, I like that approach even better to check param types (since it allows Bind()). ...
1 day, 5 hours ago (2017-04-28 18:15:43 UTC) #97
fdoray
ptanl https://codereview.chromium.org/2829083002/diff/320001/base/task_scheduler/task_traits.h File base/task_scheduler/task_traits.h (right): https://codereview.chromium.org/2829083002/diff/320001/base/task_scheduler/task_traits.h#newcode192 base/task_scheduler/task_traits.h:192: // ValidTrait ensures that we only accept types ...
1 day, 4 hours ago (2017-04-28 18:40:36 UTC) #100
gab
1 day, 4 hours ago (2017-04-28 19:00:32 UTC) #101
https://codereview.chromium.org/2829083002/diff/360001/base/task_scheduler/ta...
File base/task_scheduler/task_traits.h (right):

https://codereview.chromium.org/2829083002/diff/360001/base/task_scheduler/ta...
base/task_scheduler/task_traits.h:122: ValidTrait(base::WithBaseSyncPrimitives)
{}
Don't need base::?

https://codereview.chromium.org/2829083002/diff/360001/base/task_scheduler/ta...
base/task_scheduler/task_traits.h:135: // TaskPriority, TaskShutdownBehavior,
MayBlock and/or WithBaseSyncPrimitives
, before "and/or"

https://codereview.chromium.org/2829083002/diff/360001/base/task_scheduler/ta...
File base/task_scheduler/task_traits_details.h (right):

https://codereview.chromium.org/2829083002/diff/360001/base/task_scheduler/ta...
base/task_scheduler/task_traits_details.h:71: CallFirstTag,
I meant to reorder this to be Overload 1 so that logic and comment flows as :

1) Is list empty -> return default
2) Is first arg of right type -> return value
3) Discard first arg and go back to (1)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46