Add support for generating external types in ComputedStyleBase.

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

 #!/usr/bin/env python
 # Copyright 2016 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import math
 import sys
 
 import json5_generator
 import template_expander
@@ skipping 44 matching lines @@
         # Default value for field
         'default_value',
         # Method names
         'getter_method_name',
         'setter_method_name',
         'initial_method_name',
         'resetter_method_name',
     ])
 
     def __init__(self, field_role, **kwargs):
+        # TODO(shend): get rid of this kwargs thing by creating separate classes
+        # for each field template and taking in normal arguments, then remove the
+        # line below
+        # pylint: disable=no-member

[meade_UTC10, 2017/02/17 06:56:05]

Is this a new lint that got enabled? o.O

[shend, 2017/02/19 23:10:17]

Urgh, the comment doesn't explain anything, I updated it. Basically, the line
'self.is_packed = self.size ...' gives a lint error because we dynamically set
all the attributes and pylint is like "what's self.size?". In a later patch, I'm
hoping to reduce the number of attributes in the Field class so I can just pass
all the arguments as normal arguments instead of kwargs.

+
         # Values common to all fields
         # Set attributes from the keyword arguments
         for attrib in Field.REQUIRED_ATTRIBUTES:
             setattr(self, attrib, kwargs.pop(attrib))
 
         # Field role: one of these must be true
         self.is_property = field_role == 'property'
         self.is_inherited_flag = field_role == 'inherited_flag'
         self.is_nonproperty = field_role == 'nonproperty'
         assert (self.is_property, self.is_inherited_flag, self.is_nonproperty).count(True) == 1, \
             'Field role has to be exactly one of: property, inherited_flag, nonproperty'
 
         if self.is_property:
             self.is_inherited = kwargs.pop('inherited')
             self.is_independent = kwargs.pop('independent')
             assert self.is_inherited or not self.is_independent, 'Only inherited fields can be independent'
 
             self.is_inherited_method_name = kwargs.pop('is_inherited_method_name')
         elif self.is_inherited_flag:
             # Inherited flag-only fields
             pass
 
+        # Only pack the field if the field size is not None

[meade_UTC10, 2017/02/17 06:56:05]

nit: Does this mean when the field is used as input to ComputedStyleBaseWriter's
constructor (where the packing happens)?

Perhaps it's better to refer to the reason field_size might be none (i.e. it's
an external type)

random brainstormy ideas (some reverse the logic):
self.external_type
self.is_bitfield
self.is_class_object
self.is_class_instance

[shend, 2017/02/19 23:10:17]

Yeah "packed" is ambiguous. I changed it to 'is_bitfield' since keyword fields
can either be bitfields or not (for speed), so the name has to be generic across
different field templates.

+        self.is_packed = self.size is not None
+
         assert len(kwargs) == 0, 'Unexpected arguments provided to Field: ' + str(kwargs)
 
 
 def _get_include_paths(properties):
     """
     Get a list of paths that need to be included for ComputedStyleBase.
     """
     include_paths = set()
     for property_ in properties:
         if property_['field_type_path'] is not None:
@@ skipping 27 matching lines @@
 
 def _create_property_field(property_):
     """
     Create a property field from a CSS property and return the Field object.
     """
     property_name = property_['name_for_methods']
     property_name_lower = lower_first(property_name)
 
     # From the Blink style guide: Other data members should be prefixed by "m_". [names-data-members]
     field_name = 'm_' + property_name_lower
-    bits_needed = math.log(len(property_['keywords']), 2)  # TODO: implement for non-enums
 
-    # Separate the type path from the type name, if specified.
+    # Override the type_name if field_type_path is specified
     if property_['field_type_path']:
         type_name = property_['field_type_path'].split('/')[-1]
     else:
         type_name = property_['type_name']
 
     # For now, the getter name should match the field name. Later, getter names
     # will start with an uppercase letter, so if they conflict with the type name,
     # add 'get' to the front.
     getter_method_name = property_name_lower
     if type_name == property_name:
         getter_method_name = 'get' + property_name
 
-    assert property_['initial_keyword'] is not None, \
-        ('MakeComputedStyleBase requires an initial keyword for keyword fields, none specified '
-         'for property ' + property_['name'])
-    default_value = type_name + '::k' + camel_case(property_['initial_keyword'])
+    if property_['field_template'] == 'keyword':
+        assert property_['default_value'] is not None, \
+            ('MakeComputedStyleBase requires an initial keyword for keyword fields, none specified '
+             'for property ' + property_['name'])
+        bits_needed = int(math.ceil(math.log(len(property_['keywords']), 2)))
+        default_value = type_name + '::k' + camel_case(property_['default_value'])
+    else:
+        bits_needed = None
+        default_value = property_['default_value']
 
     return Field(
         'property',
         name=field_name,
         property_name=property_['name'],
         inherited=property_['inherited'],
         independent=property_['independent'],
         type_name=type_name,
         field_template=property_['field_template'],
-        size=int(math.ceil(bits_needed)),
+        size=bits_needed,
         default_value=default_value,
         getter_method_name=getter_method_name,
         setter_method_name='set' + property_name,
         initial_method_name='initial' + property_name,
         resetter_method_name='reset' + property_name,
         is_inherited_method_name=property_name_lower + 'IsInherited',
     )
 
 
 def _create_inherited_flag_field(property_):
@@ skipping 106 matching lines @@
             'ComputedStyleBase.cpp': self.generate_base_computed_style_cpp,
             'ComputedStyleBaseConstants.h': self.generate_base_computed_style_constants,
         }
 
         # Create all the enums used by properties
         self._generated_enums = _create_enums(self._properties.values())
 
         # Create all the fields
         all_fields = _create_fields(self._properties.values())
 
-        # Group fields into buckets
-        field_buckets = _pack_fields(all_fields)
+        # Separate the normal data members from the bit fields
+        packed_fields = [field for field in all_fields if field.is_packed]
+        unpacked_fields = [field for field in all_fields if not field.is_packed]
+
+        # Pack fields into buckets
+        field_buckets = _pack_fields(packed_fields)
 
         # The expected size of ComputedStyleBase is equivalent to as many words
         # as the total number of buckets.
-        self._expected_total_field_bytes = len(field_buckets)
+        self._expected_bitfield_bytes = len(field_buckets)
 
         # The most optimal size of ComputedStyleBase is the total sum of all the
         # field sizes, rounded up to the nearest word. If this produces the
         # incorrect value, either the packing algorithm is not optimal or there
         # is no way to pack the fields such that excess padding space is not
         # added.
         # If this fails, increase extra_padding_bytes by 1, but be aware that
         # this also increases ComputedStyleBase by 1 word.
         # We should be able to bring extra_padding_bytes back to 0 from time to
         # time.
         extra_padding_bytes = 0
-        optimal_total_field_bytes = int(math.ceil(sum(f.size for f in all_fields) / 32.0))
-        real_total_field_bytes = optimal_total_field_bytes + extra_padding_bytes
-        assert self._expected_total_field_bytes == real_total_field_bytes, \
+        optimal_bitfield_bytes = int(math.ceil(sum(f.size for f in packed_fields) / 32.0))
+        real_bitfield_bytes = optimal_bitfield_bytes + extra_padding_bytes
+        assert self._expected_bitfield_bytes == real_bitfield_bytes, \
             ('The field packing algorithm produced %s bytes, optimal is %s bytes' %
-             (self._expected_total_field_bytes, real_total_field_bytes))
+             (self._expected_bitfield_bytes, real_bitfield_bytes))
+
+        # Unpacked fields go first, then the packed fields.
+        self._fields = list(unpacked_fields)
 
         # Order the fields so fields in each bucket are adjacent.
-        self._fields = []
         for bucket in field_buckets:
             for field in bucket:
                 self._fields.append(field)
 
 
     @template_expander.use_jinja('ComputedStyleBase.h.tmpl')
     def generate_base_computed_style_h(self):
         return {
             'properties': self._properties,
             'enums': self._generated_enums,
             'include_paths': _get_include_paths(self._properties.values()),
             'fields': self._fields,
-            'expected_total_field_bytes': self._expected_total_field_bytes,
         }
 
     @template_expander.use_jinja('ComputedStyleBase.cpp.tmpl')
     def generate_base_computed_style_cpp(self):
         return {
             'properties': self._properties,
             'enums': self._generated_enums,
             'fields': self._fields,
-            'expected_total_field_bytes': self._expected_total_field_bytes,
+            'expected_bitfield_bytes': self._expected_bitfield_bytes,
         }
 
     @template_expander.use_jinja('ComputedStyleBaseConstants.h.tmpl')
     def generate_base_computed_style_constants(self):
         return {
             'properties': self._properties,
             'enums': self._generated_enums,
             'fields': self._fields,
-            'expected_total_field_bytes': self._expected_total_field_bytes,
         }
 
 if __name__ == '__main__':
     json5_generator.Maker(ComputedStyleBaseWriter).main()