Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(17)

Issue 2786883002: Generate subgroup StyleSurroundData in ComputedStyle. (Closed)

Created:
2 years, 2 months ago by shend
Modified:
2 years, 1 month ago
Reviewers:
meade_UTC10
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-style_chromium.org, chromium-reviews, dglazkov+blink, rwlbuis
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Generate subgroup StyleSurroundData in ComputedStyle. StyleSurroundData is a class that contains the values of the offset, margin, padding and border properties. This patch generates this class as a nested class inside ComputedStyle. We add an extra key in CSSProperties.json5 called 'field_group' that specifies the name of the group that the field is in. Fields with the same field_group are grouped together into a nested class. A few caveats: - Causes perf regression because it doesn't exploit sharing of groups. The fix is in a dependent patch (to avoid bloating this CL). We will land the fix right after this patch is landed. https://codereview.chromium.org/2826633002 - We generate 'border' as 'storage_only', because it is a shorthand and not a CSS property. - Subgroups are currently generated as nested classes of ComputedStyle, but this makes the code difficult to read. This may change in the future (such as putting subgroups in a ::detail namespace). - We don't ASSERT_SIZE on subgroups yet. Diff of generated files: https://gist.github.com/darrnshn/876c8171fd134b373f28ca23e821d28f/revisions BUG=628043 Review-Url: https://codereview.chromium.org/2786883002 Cr-Commit-Position: refs/heads/master@{#467239} Committed: https://chromium.googlesource.com/chromium/src/+/2692eeb5bf7dd77e68141dd9d80ff40f24c170f2

Patch Set 1 #

Patch Set 2 : Fix things #

Patch Set 3 : Rebase #

Patch Set 4 : Move some code out to parent patch #

Patch Set 5 : Fix size asserts #

Patch Set 6 : Fix size assert #

Patch Set 7 : Add init #

Patch Set 8 : Forgot file #

Patch Set 9 : Correct bit fields size calculation #

Patch Set 10 : Fix bugs? #

Patch Set 11 : Use storage_only for shorthands #

Patch Set 12 : Rebase #

Patch Set 13 : Address comments #

Patch Set 14 : Rebase #

Total comments: 24

Patch Set 15 : Address comments #

Patch Set 16 : Address comments #

Patch Set 17 : Fix things #

Patch Set 18 : Fix more things #

Patch Set 19 : Fix a bit more #

Patch Set 20 : Rebase #

Patch Set 21 : Fix things #

Total comments: 6

Patch Set 22 : Addressed comments #

Patch Set 23 : Rebase #

Patch Set 24 : Rebase #

Patch Set 25 : Rebase #

Patch Set 26 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+263 lines, -308 lines) Patch
M third_party/WebKit/Source/build/scripts/make_computed_style_base.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 12 chunks +77 lines, -47 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/name_utilities.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 24 3 chunks +13 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 24 6 chunks +30 lines, -17 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/templates/fields/base.tmpl View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 2 chunks +7 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/templates/fields/external.tmpl View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 24 2 chunks +24 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/build/scripts/templates/fields/group.tmpl View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +40 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/templates/fields/monotonic_flag.tmpl View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSProperties.json5 View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 13 chunks +56 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +7 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/style/BUILD.gn View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/style/ComputedStyle.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 5 chunks +0 lines, -85 lines 0 comments Download
M third_party/WebKit/Source/core/style/ComputedStyle.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 4 chunks +0 lines, -4 lines 0 comments Download
D third_party/WebKit/Source/core/style/StyleSurroundData.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +0 lines, -71 lines 0 comments Download
D third_party/WebKit/Source/core/style/StyleSurroundData.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +0 lines, -64 lines 0 comments Download

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 98 (90 generated)
shend
Hi Eddy, PTAL.
2 years, 2 months ago (2017-04-03 22:25:11 UTC) #25
meade_UTC10
Mostly small comments... https://codereview.chromium.org/2786883002/diff/260001/third_party/WebKit/Source/build/scripts/make_computed_style_base.py File third_party/WebKit/Source/build/scripts/make_computed_style_base.py (right): https://codereview.chromium.org/2786883002/diff/260001/third_party/WebKit/Source/build/scripts/make_computed_style_base.py#newcode78 third_party/WebKit/Source/build/scripts/make_computed_style_base.py:78: def _num_bit_field_buckets(bit_fields): would this be better ...
2 years, 2 months ago (2017-04-06 03:29:58 UTC) #43
shend
Thanks for the comments! I've fixed a few, but had a few questions in the ...
2 years, 2 months ago (2017-04-06 04:39:40 UTC) #44
nainar
Just as an FYI. I need the group.tmpl file committed so that I can start ...
2 years, 2 months ago (2017-04-15 00:39:04 UTC) #46
meade_UTC10
lgtm, but it looks like not all the comments in group.tmpl have been addressed... https://codereview.chromium.org/2786883002/diff/260001/third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl ...
2 years, 2 months ago (2017-04-20 03:08:41 UTC) #67
shend
Thanks Eddy! https://codereview.chromium.org/2786883002/diff/260001/third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl File third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl (right): https://codereview.chromium.org/2786883002/diff/260001/third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl#newcode17 third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl:17: {% macro get_expr(field) %} On 2017/04/20 at ...
2 years, 2 months ago (2017-04-20 03:22:34 UTC) #68
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/2786883002/500001
2 years, 1 month ago (2017-04-26 05:09:38 UTC) #95
commit-bot: I haz the power
2 years, 1 month ago (2017-04-26 05:17:08 UTC) #98
Message was sent while issue was closed.
Committed patchset #26 (id:500001) as
https://chromium.googlesource.com/chromium/src/+/2692eeb5bf7dd77e68141dd9d80f...

Powered by Google App Engine
This is Rietveld 408576698