Skip to content

Commit

Permalink
Clean up V8 bindings constants code
Browse files Browse the repository at this point in the history 
This small refactoring makes constants handled in the same way as attributes,
extending the work in https://crrev.com/b9619673. It uses filters on a single
|constants| context variable, rather than defining new lists for each (possibly
overlapping) group of constants, and makes use of the same logic for grouping
runtime-enabled constants by feature name that attributes were previously
changed to use.

(This also includes some pylint cleanup for v8_attributes.py and
code_generator_v8.py)

Review-Url: https://codereview.chromium.org/2058443002
Cr-Commit-Position: refs/heads/master@{#399069}
  • Loading branch information
@clelland
clelland authored and Commit bot committed Jun 10, 2016
1 parent 94c6fb2 commit 370a9ea
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 60 deletions.
12 changes: 9 additions & 3 deletions third_party/WebKit/Source/bindings/scripts/code_generator_v8.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

# pylint: disable=import-error,print-statement,relative-import

"""Generate Blink V8 bindings (.h and .cpp files).
If run itself, caches Jinja templates (and creates dummy file for build,
Expand Down Expand Up @@ -80,8 +82,8 @@
import v8_interface
import v8_types
import v8_union
from v8_utilities import capitalize, cpp_name, unique_by, v8_class_name
from utilities import KNOWN_COMPONENTS, idl_filename_to_component, is_valid_component_dependency, is_testing_target, shorten_union_name
from v8_utilities import capitalize, cpp_name, unique_by
from utilities import idl_filename_to_component, is_valid_component_dependency, is_testing_target, shorten_union_name


def normalize_and_sort_includes(include_paths):
Expand Down Expand Up @@ -149,6 +151,8 @@ def find_base_type(current_type):
class TypedefResolver(Visitor):
def __init__(self, info_provider):
self.info_provider = info_provider
self.additional_header_includes = set()
self.typedefs = {}

def resolve(self, definitions, definition_name):
"""Traverse definitions and resolves typedefs with the actual types."""
Expand Down Expand Up @@ -287,6 +291,7 @@ def generate_interface_code(self, definitions, interface_name, interface):

def generate_dictionary_code(self, definitions, dictionary_name,
dictionary):
# pylint: disable=unused-argument
interfaces_info = self.info_provider.interfaces_info
header_template = self.jinja_env.get_template('dictionary_v8.h')
cpp_template = self.jinja_env.get_template('dictionary_v8.cpp')
Expand Down Expand Up @@ -424,6 +429,7 @@ def initialize_jinja_env(cache_dir):
'unique_by': unique_by,
})
jinja_env.filters.update(attribute_filters())
jinja_env.filters.update(v8_interface.constant_filters())
return jinja_env


Expand Down Expand Up @@ -456,7 +462,7 @@ def main(argv):
try:
cache_dir = argv[1]
dummy_filename = argv[2]
except IndexError as err:
except IndexError:
print 'Usage: %s CACHE_DIR DUMMY_FILENAME' % argv[0]
return 1

Expand Down
101 changes: 51 additions & 50 deletions third_party/WebKit/Source/bindings/scripts/v8_interface.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

# pylint: disable=relative-import

"""Generate template values for an interface.
Design doc: http://www.chromium.org/developers/design-documents/idl-compiler
Expand All @@ -36,19 +38,15 @@
import itertools
from operator import itemgetter

import idl_definitions
from idl_definitions import IdlOperation, IdlArgument
import idl_types
from idl_types import IdlType, inherits_interface
import v8_attributes
from v8_globals import includes
import v8_methods
import v8_types
from v8_types import cpp_ptr_type, cpp_template_type
import v8_utilities
from v8_utilities import (cpp_name_or_partial, capitalize, cpp_name,
has_extended_attribute_value, runtime_enabled_function_name,
extended_attribute_value_as_list, is_legacy_interface_type_checking)
from v8_utilities import (cpp_name_or_partial, cpp_name, has_extended_attribute_value,
runtime_enabled_function_name, is_legacy_interface_type_checking)


INTERFACE_H_INCLUDES = frozenset([
Expand All @@ -69,6 +67,32 @@
])


def filter_has_constant_configuration(constants):
return [constant for constant in constants if
not constant['measure_as'] and
not constant['deprecate_as'] and
not constant['runtime_enabled_function'] and
not constant['origin_trial_feature_name']]


def filter_has_special_getter(constants):
return [constant for constant in constants if
constant['measure_as'] or
constant['deprecate_as'] or
constant['origin_trial_enabled_function']]


def filter_runtime_enabled(constants):
return [constant for constant in constants if
constant['runtime_enabled_function']]


def constant_filters():
return {'has_constant_configuration': filter_has_constant_configuration,
'has_special_getter': filter_has_special_getter,
'runtime_enabled_constants': filter_runtime_enabled}


def interface_context(interface):
includes.clear()
includes.update(INTERFACE_CPP_INCLUDES)
Expand Down Expand Up @@ -248,40 +272,16 @@ def interface_context(interface):
'constructors': constructors,
'has_custom_constructor': bool(custom_constructors),
'interface_length':
interface_length(interface, constructors + custom_constructors),
interface_length(constructors + custom_constructors),
'is_constructor_raises_exception': extended_attributes.get('RaisesException') == 'Constructor', # [RaisesException=Constructor]
'named_constructor': named_constructor,
'unscopeables': sorted(unscopeables),
})

constants = [constant_context(constant, interface) for constant in interface.constants]

special_getter_constants = []
runtime_enabled_constants = dict()
constant_configuration_constants = []

for constant in constants:
if constant['measure_as'] or constant['deprecate_as'] or constant['origin_trial_enabled_function']:
special_getter_constants.append(constant)
continue
runtime_enabled_function = constant['runtime_enabled_function']
if runtime_enabled_function:
if runtime_enabled_function not in runtime_enabled_constants:
runtime_enabled_constants[runtime_enabled_function] = []
runtime_enabled_constants[runtime_enabled_function].append(constant)
continue
constant_configuration_constants.append(constant)

# Constants
context.update({
'constant_configuration_constants': constant_configuration_constants,
'constants': constants,
'constants': [constant_context(constant, interface) for constant in interface.constants],
'do_not_check_constants': 'DoNotCheckConstants' in extended_attributes,
'has_constant_configuration': any(
not constant['runtime_enabled_function']
for constant in constants),
'runtime_enabled_constants': sorted(runtime_enabled_constants.iteritems()),
'special_getter_constants': special_getter_constants,
})

# Attributes
Expand Down Expand Up @@ -343,9 +343,9 @@ def generated_argument(idl_type, name, is_optional=False, extended_attributes=No
# need to support iterator overloads between interface and
# partial interface definitions.
# http://heycam.github.io/webidl/#idl-overloading
if (not interface.is_partial
and (interface.iterable or interface.maplike or interface.setlike
or interface.has_indexed_elements or 'Iterable' in extended_attributes)):
if (not interface.is_partial and
(interface.iterable or interface.maplike or interface.setlike or
interface.has_indexed_elements or 'Iterable' in extended_attributes)):

used_extended_attributes = {}

Expand Down Expand Up @@ -551,8 +551,8 @@ def generated_iterator_method(name, implemented_as=None):
'has_origin_safe_method_setter': is_global and any(
method['is_check_security_for_receiver'] and not method['is_unforgeable']
for method in methods),
'has_private_script': any(attribute['is_implemented_in_private_script'] for attribute in attributes) or
any(method['is_implemented_in_private_script'] for method in methods),
'has_private_script': (any(attribute['is_implemented_in_private_script'] for attribute in attributes) or
any(method['is_implemented_in_private_script'] for method in methods)),
'iterator_method': iterator_method,
'has_array_iterator': has_array_iterator,
'method_configuration_methods': method_configuration_methods,
Expand Down Expand Up @@ -600,9 +600,11 @@ def constant_context(constant, interface):
'measure_as': v8_utilities.measure_as(constant, interface), # [MeasureAs]
'name': constant.name,
'origin_trial_enabled_function': v8_utilities.origin_trial_enabled_function_name(constant), # [OriginTrialEnabled]
'origin_trial_feature_name': v8_utilities.origin_trial_feature_name(constant), # [OriginTrialEnabled]
# FIXME: use 'reflected_name' as correct 'name'
'reflected_name': extended_attributes.get('Reflect', constant.name),
'runtime_enabled_function': runtime_enabled_function_name(constant), # [RuntimeEnabled]
'runtime_feature_name': v8_utilities.runtime_feature_name(constant), # [RuntimeEnabled]
'value': constant.value,
}

Expand Down Expand Up @@ -701,8 +703,8 @@ def overloads_context(interface, overloads):
# maximum distinguishing argument index.
longest_overloads = effective_overloads_by_length[-1][1]
if (not common_value(overloads, 'runtime_enabled_function') and
all(method.get('runtime_enabled_function')
for method, _, _ in longest_overloads)):
all(method.get('runtime_enabled_function')
for method, _, _ in longest_overloads)):
# Generate a list of (length, runtime_enabled_functions) tuples.
runtime_determined_maxargs = []
for length, effective_overloads in reversed(effective_overloads_by_length):
Expand Down Expand Up @@ -767,11 +769,11 @@ def overloads_context(interface, overloads):
'runtime_determined_lengths': runtime_determined_lengths,
'runtime_determined_maxargs': runtime_determined_maxargs,
'runtime_enabled_function_all': common_value(overloads, 'runtime_enabled_function'), # [RuntimeEnabled]
'valid_arities': lengths
# Only need to report valid arities if there is a gap in the
# sequence of possible lengths, otherwise invalid length means
# "not enough arguments".
if lengths[-1] - lengths[0] != len(lengths) - 1 else None,
'valid_arities': (lengths
# Only need to report valid arities if there is a gap in the
# sequence of possible lengths, otherwise invalid length means
# "not enough arguments".
if lengths[-1] - lengths[0] != len(lengths) - 1 else None),
'visible': has_overload_visible,
'has_partial_overloads': has_partial_overloads,
}
Expand Down Expand Up @@ -948,7 +950,7 @@ def typename_without_nullable(idl_type):
distinguishing_argument_type_names = [type_list[index]
for type_list in type_lists]
if (len(set(distinguishing_argument_type_names)) !=
len(distinguishing_argument_type_names)):
len(distinguishing_argument_type_names)):
raise ValueError('Types in distinguishing argument are not distinct:\n'
'%s' % distinguishing_argument_type_names)

Expand Down Expand Up @@ -1222,18 +1224,18 @@ def constructor_context(interface, constructor):
is_constructor_raises_exception or
any(argument for argument in constructor.arguments
if argument.idl_type.name == 'SerializedScriptValue' or
argument.idl_type.v8_conversion_needs_exception_state),
argument.idl_type.v8_conversion_needs_exception_state),
'has_optional_argument_without_default_value':
any(True for argument_context in argument_contexts
if argument_context['is_optional_without_default_value']),
'is_call_with_document':
# [ConstructorCallWith=Document]
has_extended_attribute_value(interface,
'ConstructorCallWith', 'Document'),
'ConstructorCallWith', 'Document'),
'is_call_with_execution_context':
# [ConstructorCallWith=ExecutionContext]
has_extended_attribute_value(interface,
'ConstructorCallWith', 'ExecutionContext'),
'ConstructorCallWith', 'ExecutionContext'),
'is_call_with_script_state':
# [ConstructorCallWith=ScriptState]
has_extended_attribute_value(
Expand Down Expand Up @@ -1269,7 +1271,7 @@ def number_of_required_arguments(constructor):
if not argument.is_optional])


def interface_length(interface, constructors):
def interface_length(constructors):
# Docs: http://heycam.github.io/webidl/#es-interface-call
if not constructors:
return 0
Expand Down Expand Up @@ -1375,7 +1377,6 @@ def property_deleter(deleter):
return None

extended_attributes = deleter.extended_attributes
idl_type = deleter.idl_type
is_call_with_script_state = v8_utilities.has_extended_attribute_value(deleter, 'CallWith', 'ScriptState')
is_ce_reactions = 'CEReactions' in extended_attributes
return {
Expand Down
12 changes: 6 additions & 6 deletions third_party/WebKit/Source/bindings/templates/constants.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,27 @@ static void {{constant.name}}ConstantGetterCallback(v8::Local<v8::Name>, const v

{######################################}
{% macro install_constants() %}
{% if constant_configuration_constants %}
{% if constants | has_constant_configuration %}
{# Normal constants #}
const V8DOMConfiguration::ConstantConfiguration {{v8_class}}Constants[] = {
{% for constant in constant_configuration_constants %}
{% for constant in constants | has_constant_configuration %}
{{constant_configuration(constant)}},
{% endfor %}
};
V8DOMConfiguration::installConstants(isolate, interfaceTemplate, prototypeTemplate, {{v8_class}}Constants, WTF_ARRAY_LENGTH({{v8_class}}Constants));
{% endif %}
{# Runtime-enabled constants #}
{% for constant_tuple in runtime_enabled_constants %}
{% filter runtime_enabled(constant_tuple[0]) %}
{% for constant in constant_tuple[1] %}
{% for group in constants | runtime_enabled_constants | groupby('runtime_feature_name') %}
{% filter runtime_enabled(group.list[0].runtime_enabled_function) %}
{% for constant in group.list %}
{% set constant_name = constant.name.title().replace('_', '') %}
const V8DOMConfiguration::ConstantConfiguration constant{{constant_name}}Configuration = {{constant_configuration(constant)}};
V8DOMConfiguration::installConstant(isolate, interfaceTemplate, prototypeTemplate, constant{{constant_name}}Configuration);
{% endfor %}
{% endfilter %}
{% endfor %}
{# Constants with [DeprecateAs] or [MeasureAs] or [OriginTrialEnabled] #}
{% for constant in special_getter_constants %}
{% for constant in constants | has_special_getter %}
V8DOMConfiguration::installConstantWithGetter(isolate, interfaceTemplate, prototypeTemplate, "{{constant.name}}", {{cpp_class}}V8Internal::{{constant.name}}ConstantGetterCallback);
{% endfor %}
{# Check constants #}
Expand Down
2 changes: 1 addition & 1 deletion third_party/WebKit/Source/bindings/templates/interface_base.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ static void (*{{method.name}}MethodForPartialInterface)(const v8::FunctionCallba
{# Constants #}
{% from 'constants.cpp' import constant_getter_callback
with context %}
{% for constant in special_getter_constants %}
{% for constant in constants | has_special_getter %}
{{constant_getter_callback(constant)}}
{% endfor %}
{# Attributes #}
Expand Down

0 comments on commit 370a9ea

Please sign in to comment.