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

Issue 2788353002: Move hardcoded extra fields to its own JSON5 file. (Closed)

Created:
3 years, 8 months ago by shend
Modified:
3 years, 8 months ago
CC:
blink-reviews, blink-reviews-style_chromium.org, chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Move hardcoded extra fields to its own JSON5 file. For extra fields in ComputedStyle, we have been hardcoding their values in the generator script. However, this is ugly and not sustainable. This patch moves them into its own JSON file. This patch does not change generated code. BUG=628043 Review-Url: https://codereview.chromium.org/2788353002 Cr-Commit-Position: refs/heads/master@{#466178} Committed: https://chromium.googlesource.com/chromium/src/+/4187e7d068935829eb5a914b7f3f2b8144e3bb98

Patch Set 1 #

Patch Set 2 : rebase #

Patch Set 3 : Forgot file #

Patch Set 4 : rebase #

Patch Set 5 : Rebase #

Patch Set 6 : Rebase #

Patch Set 7 : Rebase #

Patch Set 8 : Rebase #

Total comments: 4

Patch Set 9 : Address comments #

Patch Set 10 : Address comments #

Patch Set 11 : Rebase #

Patch Set 12 : Rebase #

Patch Set 13 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+180 lines, -76 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 4 chunks +11 lines, -76 lines 0 comments Download
M third_party/WebKit/Source/core/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 View 1 2 3 4 5 6 7 8 9 1 chunk +168 lines, -0 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 60 (43 generated)
shend
Hi Bugs, PTAL. I'm open to suggestions for the naming of the new JSON file.
3 years, 8 months ago (2017-04-03 06:58:41 UTC) #2
shend
Wait, sorry, this patch is not ready yet.
3 years, 8 months ago (2017-04-03 07:42:28 UTC) #8
shend
Hi Bugs, second try :P PTAL
3 years, 8 months ago (2017-04-12 07:51:53 UTC) #11
Bugs Nash
On 2017/04/12 at 07:51:53, shend wrote: > Hi Bugs, second try :P PTAL My review ...
3 years, 8 months ago (2017-04-13 00:00:40 UTC) #18
shend
This should be ready for review now.
3 years, 8 months ago (2017-04-13 05:37:48 UTC) #21
Bugs Nash
lgtm
3 years, 8 months ago (2017-04-17 23:05:04 UTC) #30
shend
Hi Alan, PTAL
3 years, 8 months ago (2017-04-19 00:59:20 UTC) #32
alancutter (OOO until 2018)
https://codereview.chromium.org/2788353002/diff/140001/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 File third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 (right): https://codereview.chromium.org/2788353002/diff/140001/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5#newcode20 third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5:20: // The rest is the same as CSSProperties.json5 Full ...
3 years, 8 months ago (2017-04-19 07:13:32 UTC) #33
shend
https://codereview.chromium.org/2788353002/diff/140001/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 File third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 (right): https://codereview.chromium.org/2788353002/diff/140001/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5#newcode20 third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5:20: // The rest is the same as CSSProperties.json5 On ...
3 years, 8 months ago (2017-04-19 07:38:58 UTC) #34
shend
Listed relevant properties in JSON file as a comment. Alan, PTAL again?
3 years, 8 months ago (2017-04-19 22:50:11 UTC) #35
Bugs Nash
Oh whoops, I wrote this comment this morning and forgot to publish. https://codereview.chromium.org/2788353002/diff/140001/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 File third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 ...
3 years, 8 months ago (2017-04-19 22:56:56 UTC) #36
alancutter (OOO until 2018)
lgtm https://codereview.chromium.org/2788353002/diff/140001/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 File third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 (right): https://codereview.chromium.org/2788353002/diff/140001/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5#newcode20 third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5:20: // The rest is the same as CSSProperties.json5 ...
3 years, 8 months ago (2017-04-20 03:29:58 UTC) #37
shend
On 2017/04/20 at 03:29:58, alancutter wrote: > lgtm > > https://codereview.chromium.org/2788353002/diff/140001/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 > File third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5 (right): ...
3 years, 8 months ago (2017-04-20 03:35:56 UTC) #38
alancutter (OOO until 2018)
On 2017/04/20 at 03:35:56, shend wrote: > On 2017/04/20 at 03:29:58, alancutter wrote: > > ...
3 years, 8 months ago (2017-04-20 03:52:59 UTC) #39
shend
On 2017/04/20 at 03:52:59, alancutter wrote: > On 2017/04/20 at 03:35:56, shend wrote: > > ...
3 years, 8 months ago (2017-04-20 03:59:18 UTC) #40
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/2788353002/240001
3 years, 8 months ago (2017-04-20 22:57:29 UTC) #57
commit-bot: I haz the power
3 years, 8 months ago (2017-04-20 23:02:39 UTC) #60
Message was sent while issue was closed.
Committed patchset #13 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/4187e7d068935829eb5a914b7f3f...

Powered by Google App Engine
This is Rietveld 408576698