The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src.git/+/6fc8991a4e66031ed27261602742c9f3d6d2754f
commit 6fc8991a4e66031ed27261602742c9f3d6d2754f
Author: Anders Hartvoll Ruud <andruud@chromium.org>
Date: Tue May 22 11:14:27 2018
Let CSSProperties.json5 decide which properties gets legacy style builders.
Currently, we have the json5-field 'custom_apply_functions_all', which expands
to custom_apply_functions_[initial, inherit, value]. These fields were probably
originally intended to mean that style building functions for this property
should not be generated (for initial/inherit/value), but should instead be
provided manually (hand-written) elsewhere.
However, 'custom_apply_functions_all' has a second meaning, beyond its expansion
to _[initial, inherit, value]: it is also an indication that the property
_maybe_ (depending on hard-coded lists in Python) uses a non-default template
"fragment" for the code generation.
So 'custom_apply_functions_all' _could_ mean that all of the style building
functions really are custom (hard-written), or it could mean that the property
has generated style building functions, but that the template deviates from the
standard one. (You have to check the .py/.tmpl-files to figure out which).
Finally, 'custom_apply_functions_all' is used in a third way: the new code
generator uses custom_apply_functions_all (via its expansion) to determine
whether a CSSProperty subclass should get style building functions or not.
(The new code generator does still not handle all cases, so StyleBuilder::
ApplyProperty uses the old code paths for such properties). I.e. we use
'custom_apply_functions_all' as an indication of 'legacy' style building.
This is confusing, and source of truth is spread across several json5/tmpl/py
files.
To improve this, I propose:
* Remove custom_apply_functions_*. Instead:
* Add 'style_builder_custom_functions', which takes a list of actually custom
(i.e. always hand-written) builders. (Following the pattern of
computed_style_custom_functions).
* Add 'style_builder_template', which changes how code generation is performed.
The plan here is to eventually set this to values like 'fill_layer', 'color',
etc, instead of having hard-coded lists in Python to achieve the same thing.
For now, however (to keep this patch manageable), only the 'legacy' value
exists. Using this value suppresses the default code generation in
style_builder_functions.cc.tmpl, and assumes that the code generation of
that property is manually handled (in the same .tmpl file).
* Add 'style_builder_legacy', which explicitly says whether a property should
use the old code path or not. This avoids a hard-coded list in Python.
Note: Previously we generated some unused style builder functions in
CSSProperty subclasses even for properties that used the legacy code path.
This caused certain includes to be added to the generated headers, and
these includes are needed (i.e. assumed to be present) by some of our
_custom files. The "path of least diff" to solve this in this case, is to
simply always add those includes. (That will be the end state anyway).
R=futhark@chromium.org
with usages of 'style_builder_template'. Also we also need an additional
'style_builder_template_args' field, to pass custom data to the templates.
Next: Replacing hard-coded abomination in calculate_apply_functions_to_declare
Bug: 751354
Change-Id: I50049254c50afd739dc5385078c5252f0139dd01
Reviewed-on: https://chromium-review.googlesource.com/1066064
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Anders Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560531}
[modify] https://crrev.com/6fc8991a4e66031ed27261602742c9f3d6d2754f/third_party/blink/renderer/build/scripts/core/css/css_properties.py
[modify] https://crrev.com/6fc8991a4e66031ed27261602742c9f3d6d2754f/third_party/blink/renderer/build/scripts/core/css/properties/make_css_property_subclasses.py
[modify] https://crrev.com/6fc8991a4e66031ed27261602742c9f3d6d2754f/third_party/blink/renderer/build/scripts/core/css/properties/templates/style_builder_functions.tmpl
[modify] https://crrev.com/6fc8991a4e66031ed27261602742c9f3d6d2754f/third_party/blink/renderer/build/scripts/make_style_builder.py
[modify] https://crrev.com/6fc8991a4e66031ed27261602742c9f3d6d2754f/third_party/blink/renderer/build/scripts/templates/style_builder_functions.cc.tmpl
[modify] https://crrev.com/6fc8991a4e66031ed27261602742c9f3d6d2754f/third_party/blink/renderer/core/css/CSSProperties.json5
Comment 1 by meade@chromium.org
, Aug 2 2017