Refactor code generation to allow us to diff generic expressions and not just fields
This patch refactors the diffing generator code to allow us to diff
field getters and other expressions instead of just fields themselves in
the future.
It does so by creating a tree of groups to diff (DiffGroups)
that store the subgroups to diff as well as the expressions to diff.
For now the expressions are just the field accessors. In the future,
they will contain expressions like BorderLeftWidth() - a public getter
for border-left-width, and HasTransform() - which is not a field or
its getter.
Diff: https://gist.github.com/d14529581f729e810842254cbf7dc1f5/revisions
BUG=710938
Review-Url: https://codereview.chromium.org/2879563002
Cr-Commit-Position: refs/heads/master@{#471666}
diff --git a/third_party/WebKit/Source/build/scripts/make_computed_style_base.py b/third_party/WebKit/Source/build/scripts/make_computed_style_base.py
index 0169baf..c039ac17 100755
--- a/third_party/WebKit/Source/build/scripts/make_computed_style_base.py
+++ b/third_party/WebKit/Source/build/scripts/make_computed_style_base.py
@@ -74,6 +74,20 @@
self.all_fields = _flatten_list(subgroup.all_fields for subgroup in subgroups) + fields
+class DiffGroup(object):
+ """Represents a group of expressions and subgroups that need to be diffed
+ for a function in ComputedStyle.
+
+ Attributes:
+ subgroups: List of DiffGroup instances that are stored as subgroups under this group.
+ expressions: List of expression that are on this group that need to be diffed.
+ """
+ def __init__(self, group_name):
+ self.group_name = group_name
+ self.subgroups = []
+ self.expressions = []
+
+
class Field(object):
"""
The generated ComputedStyle object is made up of a series of Fields.
@@ -139,6 +153,10 @@
self.internal_setter_method_name = method_name(join_name(setter_method_name, 'Internal'))
self.initial_method_name = initial_method_name
self.resetter_method_name = method_name(join_name('Reset', name_for_methods))
+ if self.group_name:
+ self.getter_expression = self.group_member_name + '->' + class_member_name(self.name)
+ else:
+ self.getter_expression = class_member_name(self.name)
# If the size of the field is not None, it means it is a bit field
self.is_bit_field = self.size is not None
@@ -168,6 +186,24 @@
return Group('', subgroups=subgroups, fields=_reorder_fields(no_group))
+def _create_diff_groups_map(diff_function_inputs, root_group):
+ diff_functions_map = {}
+ for entry in diff_function_inputs:
+ diff_functions_map[entry['name']] = _create_diff_groups(entry['fields'], root_group)
+ return diff_functions_map
+
+
+def _create_diff_groups(fields_to_diff, root_group):
+ diff_group = DiffGroup(root_group.member_name)
+ for subgroup in root_group.subgroups:
+ if any(field.property_name in fields_to_diff for field in subgroup.all_fields):
+ diff_group.subgroups.append(_create_diff_groups(fields_to_diff, subgroup))
+ for field in root_group.fields:
+ if field.property_name in fields_to_diff:
+ diff_group.expressions.append(field.getter_expression)
+ return diff_group
+
+
def _create_enums(properties):
"""
Returns an OrderedDict of enums to be generated, enum name -> [list of enum values]
@@ -373,6 +409,9 @@
# Organise fields into a tree structure where the root group
# is ComputedStyleBase.
self._root_group = _group_fields(all_fields)
+ self._diff_functions_map = _create_diff_groups_map(json5_generator.Json5File.load_from_files(
+ [json5_file_paths[2]]
+ ).name_dictionaries, self._root_group)
self._include_paths = _get_include_paths(all_properties)
self._outputs = {
@@ -387,6 +426,7 @@
'enums': self._generated_enums,
'include_paths': self._include_paths,
'computed_style': self._root_group,
+ 'diff_functions_map': self._diff_functions_map,
}
@template_expander.use_jinja('ComputedStyleBaseConstants.h.tmpl')
diff --git a/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl b/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl
index 8ea284e..43f8c820 100644
--- a/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl
+++ b/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl
@@ -121,24 +121,12 @@
{% endfor %}
}
- bool ScrollAnchorDisablingPropertyChanged(
- const ComputedStyleBase& other,
- const StyleDifference& diff) const {
- {{fieldwise_diff(computed_style, computed_style.all_fields
- |selectattr("property_name", "in", ["margin-top", "margin-left", "margin-right", "margin-bottom", "left", "right", "top", "bottom", "padding-top", "padding-left", "padding-right", "padding-bottom"])
- |list
- )|indent(4)}}
+ {% for name, groups_to_diff in diff_functions_map.items() %}
+ bool {{name}}(const ComputedStyleBase& other) const {
+ {{fieldwise_diff(groups_to_diff)|indent(4)}}
return false;
}
-
- bool DiffNeedsFullLayoutAndPaintInvalidation(
- const ComputedStyleBase& other) const {
- {{fieldwise_diff(computed_style, computed_style.all_fields
- |selectattr("property_name", "in", ["padding-top", "padding-left", "padding-right", "padding-bottom"])
- |list
- )|indent(4)}}
- return false;
- }
+ {% endfor %}
// Fields.
// TODO(sashab): Remove initialFoo() static methods and update callers to
diff --git a/third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl b/third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl
index 11474ec..3519dab8 100644
--- a/third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl
+++ b/third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl
@@ -96,16 +96,14 @@
{% endfor %}
{% endmacro %}
-{% macro fieldwise_diff(group, fields_to_diff) %}
-{% for subgroup in group.subgroups %}
-{% if subgroup.all_fields|select("in", fields_to_diff)|list|length > 0 -%}
-if ({{subgroup.member_name}}.Get() != other.{{subgroup.member_name}}.Get()) {
-{{fieldwise_diff(subgroup, fields_to_diff)|indent(2, true)}}
+{% macro fieldwise_diff(group_to_diff) %}
+{% for group in group_to_diff.subgroups %}
+if ({{group.group_name}}.Get() != other.{{group.group_name}}.Get()) {
+{{fieldwise_diff(group)|indent(2, true)}}
}
-{% endif -%}
{% endfor %}
-{% for field in group.fields|select("in", fields_to_diff) %}
-if ({{getter_expression(field)}} != other.{{getter_expression(field)}})
+{% for expression in group_to_diff.expressions %}
+if ({{expression}} != other.{{expression}})
return true;
{% endfor %}
-{% endmacro %}
+{% endmacro %}
\ No newline at end of file
diff --git a/third_party/WebKit/Source/core/BUILD.gn b/third_party/WebKit/Source/core/BUILD.gn
index 84ccd79f..86067d8f 100644
--- a/third_party/WebKit/Source/core/BUILD.gn
+++ b/third_party/WebKit/Source/core/BUILD.gn
@@ -410,7 +410,10 @@
css_properties("make_core_generated_computed_style_base") {
script = "../build/scripts/make_computed_style_base.py"
- in_files = [ "css/ComputedStyleExtraFields.json5" ]
+ in_files = [
+ "css/ComputedStyleExtraFields.json5",
+ "css/ComputedStyleDiffFunctions.json5",
+ ]
other_inputs = [
"../build/scripts/templates/fields/field.tmpl",
"../build/scripts/templates/fields/group.tmpl",
diff --git a/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5 b/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
new file mode 100644
index 0000000..657b33e
--- /dev/null
+++ b/third_party/WebKit/Source/core/css/ComputedStyleDiffFunctions.json5
@@ -0,0 +1,25 @@
+{
+ // This file specifies the fields we want to diff in the various diff functions
+ // in ComputedStyle.
+
+ parameters: {
+ // The name parameter represents the name of the function to be created.
+
+ // The fields that need to be diffed.
+ fields: {
+ },
+ },
+ data: [
+ {
+ name: "ScrollAnchorDisablingPropertyChanged",
+ fields: ["margin-top", "margin-left", "margin-right", "margin-bottom",
+ "left", "right", "top", "bottom", "padding-top",
+ "padding-left", "padding-right", "padding-bottom"]
+ },
+ {
+ name: "DiffNeedsFullLayoutAndPaintInvalidation",
+ fields: ["padding-top", "padding-left", "padding-right",
+ "padding-bottom"]
+ }
+ ],
+}
diff --git a/third_party/WebKit/Source/core/style/ComputedStyle.cpp b/third_party/WebKit/Source/core/style/ComputedStyle.cpp
index 23ccc5c..a437ead 100644
--- a/third_party/WebKit/Source/core/style/ComputedStyle.cpp
+++ b/third_party/WebKit/Source/core/style/ComputedStyle.cpp
@@ -568,7 +568,7 @@
return true;
}
- if (ComputedStyleBase::ScrollAnchorDisablingPropertyChanged(other, diff))
+ if (ComputedStyleBase::ScrollAnchorDisablingPropertyChanged(other))
return true;
if (diff.TransformChanged())
