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

Issue 2811253002: Extract default naming logic in StyleBuilderWriter to a method. (Closed)

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

Description

Extract default naming logic in StyleBuilderWriter to a method. Currently, StyleBuilderWriter augments its input with attributes related to naming, which are used in its subclasses. However, this logic is in the StyleBuilderWriter constructor, which makes it difficult to invok . For example, it's difficult for a subclass to read a separate JSON file and apply this naming logic on it. This patch moves the naming logic out into a separate method that subclasses can invoke at anytime. The separate method assumes the existence of keys like 'getter' and 'custom_all', which do not exist in NONPROPERTIES. Instead of going through all those keys and setting their value to None, we just use a defaultdict so that any access to missing keys returns None. To show that it's working, we apply the same naming logic on both properties and nonproperties in make_computed_style_base, despite the two being stored separately. BUG=628043 Review-Url: https://codereview.chromium.org/2811253002 Cr-Commit-Position: refs/heads/master@{#465944} Committed: https://chromium.googlesource.com/chromium/src/+/efefb24c1c0d9c7af36c2af86933e5e6e312c5a8

Patch Set 1 #

Total comments: 4

Patch Set 2 : Rebase #

Patch Set 3 : Rebase #

Patch Set 4 : Rebase #

Total comments: 4

Patch Set 5 : Address comments #

Total comments: 2

Patch Set 6 : Add TODO #

Unified diffs Side-by-side diffs Delta from patch set Stats (+36 lines, -31 lines) Patch
M third_party/WebKit/Source/build/scripts/make_computed_style_base.py View 1 2 3 4 1 chunk +6 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/make_style_builder.py View 1 2 3 4 5 2 chunks +30 lines, -23 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 36 (23 generated)
shend
Hi Bugs, PTAL
3 years, 8 months ago (2017-04-12 03:26:47 UTC) #2
Bugs Nash
https://codereview.chromium.org/2811253002/diff/1/third_party/WebKit/Source/build/scripts/make_computed_style_base.py File third_party/WebKit/Source/build/scripts/make_computed_style_base.py (left): https://codereview.chromium.org/2811253002/diff/1/third_party/WebKit/Source/build/scripts/make_computed_style_base.py#oldcode312 third_party/WebKit/Source/build/scripts/make_computed_style_base.py:312: if 'field_type_path' not in property_: this field_type_path logic does ...
3 years, 8 months ago (2017-04-12 23:31:46 UTC) #3
shend
https://codereview.chromium.org/2811253002/diff/1/third_party/WebKit/Source/build/scripts/make_computed_style_base.py File third_party/WebKit/Source/build/scripts/make_computed_style_base.py (left): https://codereview.chromium.org/2811253002/diff/1/third_party/WebKit/Source/build/scripts/make_computed_style_base.py#oldcode312 third_party/WebKit/Source/build/scripts/make_computed_style_base.py:312: if 'field_type_path' not in property_: On 2017/04/12 at 23:31:45, ...
3 years, 8 months ago (2017-04-12 23:41:03 UTC) #6
Bugs Nash
https://codereview.chromium.org/2811253002/diff/1/third_party/WebKit/Source/build/scripts/make_computed_style_base.py File third_party/WebKit/Source/build/scripts/make_computed_style_base.py (left): https://codereview.chromium.org/2811253002/diff/1/third_party/WebKit/Source/build/scripts/make_computed_style_base.py#oldcode312 third_party/WebKit/Source/build/scripts/make_computed_style_base.py:312: if 'field_type_path' not in property_: On 2017/04/12 at 23:41:03, ...
3 years, 8 months ago (2017-04-12 23:52:37 UTC) #7
shend
https://codereview.chromium.org/2811253002/diff/1/third_party/WebKit/Source/build/scripts/make_computed_style_base.py File third_party/WebKit/Source/build/scripts/make_computed_style_base.py (left): https://codereview.chromium.org/2811253002/diff/1/third_party/WebKit/Source/build/scripts/make_computed_style_base.py#oldcode312 third_party/WebKit/Source/build/scripts/make_computed_style_base.py:312: if 'field_type_path' not in property_: On 2017/04/12 at 23:52:37, ...
3 years, 8 months ago (2017-04-12 23:57:28 UTC) #8
Bugs Nash
lgtm
3 years, 8 months ago (2017-04-13 00:15:30 UTC) #9
shend
Hi Alan, PTAL
3 years, 8 months ago (2017-04-13 00:45:12 UTC) #12
alancutter (OOO until 2018)
https://codereview.chromium.org/2811253002/diff/60001/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/2811253002/diff/60001/third_party/WebKit/Source/build/scripts/make_computed_style_base.py#newcode314 third_party/WebKit/Source/build/scripts/make_computed_style_base.py:314: nonproperties = [defaultdict(lambda: None, item) for item in NONPROPERTIES] ...
3 years, 8 months ago (2017-04-19 04:36:35 UTC) #23
shend
Hi Alan, PTAL again :) https://codereview.chromium.org/2811253002/diff/60001/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/2811253002/diff/60001/third_party/WebKit/Source/build/scripts/make_computed_style_base.py#newcode314 third_party/WebKit/Source/build/scripts/make_computed_style_base.py:314: nonproperties = [defaultdict(lambda: None, ...
3 years, 8 months ago (2017-04-19 05:26:05 UTC) #24
alancutter (OOO until 2018)
lgtm https://codereview.chromium.org/2811253002/diff/80001/third_party/WebKit/Source/build/scripts/make_style_builder.py File third_party/WebKit/Source/build/scripts/make_style_builder.py (right): https://codereview.chromium.org/2811253002/diff/80001/third_party/WebKit/Source/build/scripts/make_style_builder.py#newcode47 third_party/WebKit/Source/build/scripts/make_style_builder.py:47: simple_type_name = str(property_['type_name']).split('::')[-1] Nit: The interlacing of these ...
3 years, 8 months ago (2017-04-20 04:39:49 UTC) #25
shend
https://codereview.chromium.org/2811253002/diff/80001/third_party/WebKit/Source/build/scripts/make_style_builder.py File third_party/WebKit/Source/build/scripts/make_style_builder.py (right): https://codereview.chromium.org/2811253002/diff/80001/third_party/WebKit/Source/build/scripts/make_style_builder.py#newcode47 third_party/WebKit/Source/build/scripts/make_style_builder.py:47: simple_type_name = str(property_['type_name']).split('::')[-1] On 2017/04/20 at 04:39:48, alancutter wrote: ...
3 years, 8 months ago (2017-04-20 06:32:53 UTC) #28
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/2811253002/100001
3 years, 8 months ago (2017-04-20 08:07:52 UTC) #33
commit-bot: I haz the power
3 years, 8 months ago (2017-04-20 08:12:10 UTC) #36
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/efefb24c1c0d9c7af36c2af86933...

Powered by Google App Engine
This is Rietveld 408576698