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

Issue 2829083002: Add constexpr TaskTraits constructor. (Closed)

Created:
3 years, 8 months ago by fdoray
Modified:
3 years, 7 months 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 Review-Url: https://codereview.chromium.org/2829083002 Cr-Commit-Position: refs/heads/master@{#468417} Committed: https://chromium.googlesource.com/chromium/src/+/879b2fcdbf4efc613abf3ab094ca11d53e74498c

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: 6

Patch Set 21 : CR-gab #

Total comments: 2

Patch Set 22 : remove-comma #

Unified diffs Side-by-side diffs Delta from patch set Stats (+357 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 20 2 chunks +114 lines, -66 lines 0 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 20 21 1 chunk +128 lines, -0 lines 0 comments 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

Messages

Total messages: 119 (89 generated)
fdoray
PTAL
3 years, 8 months 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 ...
3 years, 8 months 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: > ...
3 years, 8 months 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: > ...
3 years, 8 months 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, ...
3 years, 8 months 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 ...
3 years, 8 months 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 ...
3 years, 7 months 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 years, 7 months 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 years, 7 months 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 years, 7 months 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 years, 7 months 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: > ...
3 years, 7 months 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 ...
3 years, 7 months 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. ...
3 years, 7 months 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 ...
3 years, 7 months 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 ...
3 years, 7 months 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> ...
3 years, 7 months ago (2017-04-28 15:00:36 UTC) #84
etipdoray
lgtm
3 years, 7 months 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 ...
3 years, 7 months 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
3 years, 7 months 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)
3 years, 7 months 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 ...
3 years, 7 months 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()). ...
3 years, 7 months 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 ...
3 years, 7 months ago (2017-04-28 18:40:36 UTC) #100
gab
https://codereview.chromium.org/2829083002/diff/360001/base/task_scheduler/task_traits.h File base/task_scheduler/task_traits.h (right): https://codereview.chromium.org/2829083002/diff/360001/base/task_scheduler/task_traits.h#newcode122 base/task_scheduler/task_traits.h:122: ValidTrait(base::WithBaseSyncPrimitives) {} Don't need base::? https://codereview.chromium.org/2829083002/diff/360001/base/task_scheduler/task_traits.h#newcode135 base/task_scheduler/task_traits.h:135: // TaskPriority, ...
3 years, 7 months ago (2017-04-28 19:00:32 UTC) #101
fdoray
gab@: PTAnL https://codereview.chromium.org/2829083002/diff/360001/base/task_scheduler/task_traits.h File base/task_scheduler/task_traits.h (right): https://codereview.chromium.org/2829083002/diff/360001/base/task_scheduler/task_traits.h#newcode122 base/task_scheduler/task_traits.h:122: ValidTrait(base::WithBaseSyncPrimitives) {} On 2017/04/28 19:00:31, gab wrote: ...
3 years, 7 months ago (2017-05-01 17:13:10 UTC) #105
gab
LGTM w/ nit, this is awesome :)! https://codereview.chromium.org/2829083002/diff/380001/base/task_scheduler/task_traits_details.h File base/task_scheduler/task_traits_details.h (right): https://codereview.chromium.org/2829083002/diff/380001/base/task_scheduler/task_traits_details.h#newcode28 base/task_scheduler/task_traits_details.h:28: // prefers ...
3 years, 7 months ago (2017-05-01 18:56:17 UTC) #109
fdoray
https://codereview.chromium.org/2829083002/diff/380001/base/task_scheduler/task_traits_details.h File base/task_scheduler/task_traits_details.h (right): https://codereview.chromium.org/2829083002/diff/380001/base/task_scheduler/task_traits_details.h#newcode28 base/task_scheduler/task_traits_details.h:28: // prefers using the second overload, because the type ...
3 years, 7 months ago (2017-05-01 19:29:31 UTC) #110
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/400001
3 years, 7 months ago (2017-05-01 19:31:14 UTC) #116
commit-bot: I haz the power
3 years, 7 months ago (2017-05-01 21:34:38 UTC) #119
Message was sent while issue was closed.
Committed patchset #22 (id:400001) as
https://chromium.googlesource.com/chromium/src/+/879b2fcdbf4efc613abf3ab094ca...

Powered by Google App Engine
This is Rietveld 408576698