Allow (non)property fields to be hardcoded in make_computed_style_base.

Allow (non)property fields to be hardcoded in make_computed_style_base.

Currently we divide ComputedStyle fields into two groups: properties
and nonproperties. Properties represent actual CSS properties and are
read from CSSProperties.json5. Nonproperties represent fields that do
not correspond to a CSS property and are read from a hardcoded list in
the generator script.

However, some fields act like properties, but do not correspond to
CSS properties (e.g. margin is a combination of four properties, but it
itself is not a CSS property). These need to be stored in the hardcoded
list. But since everything in the hardcoded list is considered a
nonproperty, this will not generate the right code.

Hence, this patch changes the hardcoded list to be rather a list of
"extra" fields which can be property or nonproperty fields. To
distinguish between properties and nonproperties, we introduce a
has_custom_compare_and_copy parameter. If true, the field is a
nonproperty.

This is prework for replacing the concept of nonproperties with
has_custom_compare_and_copy entirely.

This patch does not affect existing behaviour.

BUG=628043
Review-Url: https://codereview.chromium.org/2811273003
Cr-Commit-Position: refs/heads/master@{#466166}
Committed: https://chromium.googlesource.com/chromium/src/+/abefb698e5e6ad81853ffdd0560c9efa4740464d

[shend, 2017-04-12 07:31:27]

Description was changed from

==========
Allow nonproperty fields to  in CSSProperties.json5.

Currently we divide ComputedStyle fields into two groups: properties
and nonproperties. Properties represent actual CSS properties and are
read from CSSProperties.json5. Nonproperties represent fields that do
not correspond to a CSS property and are read from a hardcoded list in
the generator script.

However, some fields act like properties, but do not correspond to
CSS properties (e.g. margin is a combination of four properties, but it
itself is not a CSS property). These need to be stored in the hardcoded
list. But since everything in the hardcoded list is considered a
nonproperty, this will not work.

Hence, this patch changes the hardcoded list to be rather a list of
"extra" fields which can be property or nonproperty fields. To
distinguish between properties and nonproperties, we use the is_property
parameter. This allows us to add in extra fields that behave like
properties but does not directly correspond to a CSS property.

BUG=628043
==========

to

==========
Allow (non)property fields to be hardcoded in make_computed_style_base.

Currently we divide ComputedStyle fields into two groups: properties
and nonproperties. Properties represent actual CSS properties and are
read from CSSProperties.json5. Nonproperties represent fields that do
not correspond to a CSS property and are read from a hardcoded list in
the generator script.

However, some fields act like properties, but do not correspond to
CSS properties (e.g. margin is a combination of four properties, but it
itself is not a CSS property). These need to be stored in the hardcoded
list. But since everything in the hardcoded list is considered a
nonproperty, this will not generate the right code.

Hence, this patch changes the hardcoded list to be rather a list of
"extra" fields which can be property or nonproperty fields. To
distinguish between properties and nonproperties, we use the is_property
parameter. This allows us to add in extra fields that behave like
properties by specifying is_property=True.

BUG=628043
==========

[shend, 2017-04-12 07:32:16]

shend@chromium.org changed reviewers:
+ nainar@chromium.org

[shend, 2017-04-12 07:32:16]

Hi Bugs, PTAL

[nainar, 2017-04-12 18:07:40]

Not Bugs but lgtm :P

[shend, 2017-04-12 23:28:30]

shend@chromium.org changed reviewers:
+ alancutter@chromium.org

[shend, 2017-04-12 23:28:31]

Hi Alan, PTAL

[shend, 2017-04-13 05:04:29]

Description was changed from

==========
Allow (non)property fields to be hardcoded in make_computed_style_base.

Currently we divide ComputedStyle fields into two groups: properties
and nonproperties. Properties represent actual CSS properties and are
read from CSSProperties.json5. Nonproperties represent fields that do
not correspond to a CSS property and are read from a hardcoded list in
the generator script.

However, some fields act like properties, but do not correspond to
CSS properties (e.g. margin is a combination of four properties, but it
itself is not a CSS property). These need to be stored in the hardcoded
list. But since everything in the hardcoded list is considered a
nonproperty, this will not generate the right code.

Hence, this patch changes the hardcoded list to be rather a list of
"extra" fields which can be property or nonproperty fields. To
distinguish between properties and nonproperties, we use the is_property
parameter. This allows us to add in extra fields that behave like
properties by specifying is_property=True.

BUG=628043
==========

to

==========
Allow (non)property fields to be hardcoded in make_computed_style_base.

Currently we divide ComputedStyle fields into two groups: properties
and nonproperties. Properties represent actual CSS properties and are
read from CSSProperties.json5. Nonproperties represent fields that do
not correspond to a CSS property and are read from a hardcoded list in
the generator script.

However, some fields act like properties, but do not correspond to
CSS properties (e.g. margin is a combination of four properties, but it
itself is not a CSS property). These need to be stored in the hardcoded
list. But since everything in the hardcoded list is considered a
nonproperty, this will not generate the right code.

Hence, this patch changes the hardcoded list to be rather a list of
"extra" fields which can be property or nonproperty fields. To
distinguish between properties and nonproperties, we introduce a
has_custom_compare_and_copy parameter. If true, the field is a
nonproperty.

This is prework for replacing the concept of nonproperties with
has_custom_compare_and_copy entirely.

This patch does not affect existing behaviour.

BUG=628043
==========

[shend, 2017-04-13 05:06:01]

The CQ bit was checked by shend@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 05:06:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 05:10:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-13 05:10:09]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[shend, 2017-04-13 05:23:16]

The CQ bit was checked by shend@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 05:23:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 05:26:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-13 05:26:54]

Dry run: 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_presub...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[shend, 2017-04-13 05:40:12]

The CQ bit was checked by shend@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 05:40:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 07:11:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-13 07:11:02]

Dry run: This issue passed the CQ dry run.

[shend, 2017-04-14 04:28:52]

The CQ bit was checked by shend@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-14 04:29:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-14 05:50:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-14 05:50:19]

Dry run: This issue passed the CQ dry run.

[alancutter (OOO until 2018), 2017-04-19 04:40:16]

lgtm

https://codereview.chromium.org/2811273003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/build/scripts/make_computed_style_base.py
(right):

https://codereview.chromium.org/2811273003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_computed_style_base.py:321:
property_['has_custom_compare_and_copy'] = False
# All CSS properties that are generated do not have custom comparison and copy
logic.

https://codereview.chromium.org/2811273003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/build/scripts/make_computed_style_base.py
(right):

https://codereview.chromium.org/2811273003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_computed_style_base.py:327:
extra_fields = [defaultdict(lambda: None, item) for item in EXTRA_FIELDS]
I still have concerns over this from https://codereview.chromium.org/2811253002.
Treating this bit as separate for the review.

[shend, 2017-04-19 07:20:31]

https://codereview.chromium.org/2811273003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/build/scripts/make_computed_style_base.py
(right):

https://codereview.chromium.org/2811273003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_computed_style_base.py:321:
property_['has_custom_compare_and_copy'] = False
On 2017/04/19 at 04:40:15, alancutter wrote:
> # All CSS properties that are generated do not have custom comparison and copy
logic.

Done.

https://codereview.chromium.org/2811273003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/build/scripts/make_computed_style_base.py
(right):

https://codereview.chromium.org/2811273003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_computed_style_base.py:327:
extra_fields = [defaultdict(lambda: None, item) for item in EXTRA_FIELDS]
On 2017/04/19 at 04:40:15, alancutter wrote:
> I still have concerns over this from
https://codereview.chromium.org/2811253002.
> Treating this bit as separate for the review.

Fixed in parent patch.

[shend, 2017-04-20 07:35:03]

The CQ bit was checked by shend@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-20 07:35:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[shend, 2017-04-20 08:21:38]

The CQ bit was checked by shend@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-20 08:21:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-20 09:51:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-20 09:51:51]

Dry run: This issue passed the CQ dry run.

[shend, 2017-04-20 22:30:05]

The CQ bit was checked by shend@chromium.org

[shend, 2017-04-20 22:30:06]

The patchset sent to the CQ was uploaded after l-g-t-m from nainar@chromium.org,
alancutter@chromium.org
Link to the patchset: https://codereview.chromium.org/2811273003/#ps180001
(title: "Rebase")

[commit-bot: I haz the power, 2017-04-20 22:30:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-20 22:34:46]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1492727405611570,
"parent_rev": "014aec7ea81e1725b6e7fc9ae7d8654d8ee8f6a7", "commit_rev":
"abefb698e5e6ad81853ffdd0560c9efa4740464d"}

[commit-bot: I haz the power, 2017-04-20 22:34:54]

Description was changed from

==========
Allow (non)property fields to be hardcoded in make_computed_style_base.

Currently we divide ComputedStyle fields into two groups: properties
and nonproperties. Properties represent actual CSS properties and are
read from CSSProperties.json5. Nonproperties represent fields that do
not correspond to a CSS property and are read from a hardcoded list in
the generator script.

However, some fields act like properties, but do not correspond to
CSS properties (e.g. margin is a combination of four properties, but it
itself is not a CSS property). These need to be stored in the hardcoded
list. But since everything in the hardcoded list is considered a
nonproperty, this will not generate the right code.

Hence, this patch changes the hardcoded list to be rather a list of
"extra" fields which can be property or nonproperty fields. To
distinguish between properties and nonproperties, we introduce a
has_custom_compare_and_copy parameter. If true, the field is a
nonproperty.

This is prework for replacing the concept of nonproperties with
has_custom_compare_and_copy entirely.

This patch does not affect existing behaviour.

BUG=628043
==========

to

==========
Allow (non)property fields to be hardcoded in make_computed_style_base.

Currently we divide ComputedStyle fields into two groups: properties
and nonproperties. Properties represent actual CSS properties and are
read from CSSProperties.json5. Nonproperties represent fields that do
not correspond to a CSS property and are read from a hardcoded list in
the generator script.

However, some fields act like properties, but do not correspond to
CSS properties (e.g. margin is a combination of four properties, but it
itself is not a CSS property). These need to be stored in the hardcoded
list. But since everything in the hardcoded list is considered a
nonproperty, this will not generate the right code.

Hence, this patch changes the hardcoded list to be rather a list of
"extra" fields which can be property or nonproperty fields. To
distinguish between properties and nonproperties, we introduce a
has_custom_compare_and_copy parameter. If true, the field is a
nonproperty.

This is prework for replacing the concept of nonproperties with
has_custom_compare_and_copy entirely.

This patch does not affect existing behaviour.

BUG=628043

Review-Url: https://codereview.chromium.org/2811273003
Cr-Commit-Position: refs/heads/master@{#466166}
Committed:
https://chromium.googlesource.com/chromium/src/+/abefb698e5e6ad81853ffdd0560c...
==========

[commit-bot: I haz the power, 2017-04-20 22:34:56]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/abefb698e5e6ad81853ffdd0560c...