Refactor code generation to allow us to diff generic expressions and not just fields

third_party/WebKit/Source/build/scripts/make_computed_style_base.py

Index: third_party/WebKit/Source/build/scripts/make_computed_style_base.py
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 a181350f4317cdd9e5071746d0333e3497525437..dadeebba4d0cafe19eabe55fde0b7bf9805a671f 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
@@ -51,7 +51,6 @@ def _num_32_bit_words_for_bit_fields(bit_fields):
         cur_bucket += field.size
     return num_buckets + (cur_bucket > 0)
 
-

[nainar, 2017/05/11 04:04:08]

oops. Added this in. 

[nainar, 2017/05/11 06:00:08]

Removed.

 class Group(object):
     """Represents a group of fields stored together in a class.
 
@@ -74,6 +73,19 @@ class Group(object):
         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 functions that are on this group that need to be diffed.

[shend, 2017/05/11 05:00:59]

"List of expressions"

[nainar, 2017/05/11 06:00:08]

Done.

+    """
+    def __init__(self, group_name, subgroups, expressions):
+        self.group_name = group_name
+        self.subgroups = subgroups
+        self.expressions = expressions
+
 class Field(object):
     """
     The generated ComputedStyle object is made up of a series of Fields.
@@ -163,11 +175,49 @@ def _group_fields(fields):
     for field in fields:
         groups[field.group_name].append(field)
 
-    no_group = groups.pop(None)
+    no_group = {}
+    if groups.get(None):
+        no_group = groups.pop(None)
     subgroups = [Group(group_name, [], _reorder_fields(fields)) for group_name, fields in groups.items()]
     return Group('', subgroups=subgroups, fields=_reorder_fields(no_group))
 
 
+def _create_field(field_name, all_properties):

[shend, 2017/05/11 05:00:59]

Hmm, I'm not quite sure why this is needed.
I think you should be able to do all the diff group logic with just root_group,
without creating any new Field instances and stuff.

[nainar, 2017/05/11 06:00:08]

Didn't realize root_group was a thing

+    for property_ in all_properties:
+        if field_name == property_['name']:
+            return _create_property_field(property_)
+
+
+def _create_expression(field):

[shend, 2017/05/11 05:00:59]

maybe _getter_expression? Or even put this as a member of Field?

[nainar, 2017/05/11 06:00:08]

Added it as a member on field

+    if field.group_name:
+        return field.group_name + '_data_->' + class_member_name(field.name)
+    else:
+        return class_member_name(field.name)
+
+
+def _match_groups_to_diff_groups(groups):
+    diff_groups_list = []
+    if not groups:
+        return
+    for group in groups:
+        list_of_expressions = []
+        for field in group.fields:
+            list_of_expressions.append(_create_expression(field))
+        diff_group = DiffGroup(group.name + '_data_', _match_groups_to_diff_groups(group.subgroups), list_of_expressions)
+        diff_groups_list.append(diff_group)
+    return diff_groups_list
+
+
+def _create_diff_groups(diff_function_inputs, all_properties):

[shend, 2017/05/11 05:00:59]

As mentioned below, I believe this code would be easier if this returned a map
of name -> DF, not name -> DFs. The code should be very similar to the original
jinja template. Something like:

_create_diff_group(root_group, fields_to_diff):
  DG = DiffGroup()
  for subgroup in root_group.subgroups:
      if subgroup.all_fields and fields_to_diff intersect
         DG.subgroups.append(_create_diff_groups(subgroup, fields_to_diff))
  for field in group.fields that are in fields_to_diff
      DG.exprs.append(getter_expression(field)).
  return DG.

[nainar, 2017/05/11 06:00:08]

Done.

+    diff_named_groups_map = {}
+    for entry in diff_function_inputs:
+        list_of_fields = []
+        for field in entry['fields']:
+            list_of_fields.append(_create_field(field, all_properties))
+        diff_named_groups_map[entry['name']] = _match_groups_to_diff_groups(_group_fields(list_of_fields).subgroups)
+    return diff_named_groups_map
+
+
 def _create_enums(properties):
     """
     Returns an OrderedDict of enums to be generated, enum name -> [list of enum values]
@@ -370,6 +420,10 @@ class ComputedStyleBaseWriter(make_style_builder.StyleBuilderWriter):
 
         all_fields = _create_fields(all_properties)
 
+        self._diff_functions_map = _create_diff_groups(json5_generator.Json5File.load_from_files(
+            [json5_file_paths[2]]
+        ).name_dictionaries, all_properties)
+
         # Organise fields into a tree structure where the root group
         # is ComputedStyleBase.
         self._root_group = _group_fields(all_fields)
@@ -396,6 +450,7 @@ class ComputedStyleBaseWriter(make_style_builder.StyleBuilderWriter):
             'properties': self._properties,
             'enums': self._generated_enums,
             'computed_style': self._root_group,
+            'diff_functions_map': self._diff_functions_map,
         }
 
     @template_expander.use_jinja('ComputedStyleBaseConstants.h.tmpl')