Skip to content

Commit

Permalink
Clean up V8 bindings template code
Browse files Browse the repository at this point in the history 
This patch makes some of the V8 bindings templates a bit cleaner. Specifically:
 - Uses custom filters for the attribute list to duplicating complicated logic
   between for loops in the template and extra booleans defined in python.
 - Adds a unique_by filter to simplify the uniqueness logic in the templates.
 - Makes consistent use of the 'runtime_enabled' filter.
 - Combines for/endfor and if/endif tags into single for-if/endfor tags.
 - Makes use of the jinja2 'groupby' filter to clarify the complex logic for
   collecting and grouping runtime features by name.

It has a couple of visible side effects; mostly removing some extra generated
blank lines, and removing an unneeded extra brace pair in v8_dictionary.cpp (and
adjusting indentation appropriately). The text expectations have been updated to
match.

[email protected]

Review-Url: https://codereview.chromium.org/2000483002
Cr-Commit-Position: refs/heads/master@{#395067}
(cherry picked from commit b961967)

BUG=584367

Review URL: https://codereview.chromium.org/2029423002 .

Cr-Commit-Position: refs/branch-heads/2743@{crosswalk-project#186}
Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}
  • Loading branch information
@clelland
clelland committed Jun 2, 2016
1 parent c31f015 commit 891d66a
Show file tree
Hide file tree
Showing 22 changed files with 517 additions and 637 deletions.
5 changes: 4 additions & 1 deletion third_party/WebKit/Source/bindings/scripts/code_generator_v8.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,14 @@
from idl_definitions import Visitor
import idl_types
from idl_types import IdlType
from v8_attributes import attribute_filters
import v8_callback_interface
import v8_dictionary
from v8_globals import includes, interfaces
import v8_interface
import v8_types
import v8_union
from v8_utilities import capitalize, cpp_name, v8_class_name
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


Expand Down Expand Up @@ -420,7 +421,9 @@ def initialize_jinja_env(cache_dir):
'blink_capitalize': capitalize,
'exposed': exposed_if,
'runtime_enabled': runtime_enabled_if,
'unique_by': unique_by,
})
jinja_env.filters.update(attribute_filters())
return jinja_env


Expand Down
1 change: 1 addition & 0 deletions third_party/WebKit/Source/bindings/scripts/v8_attributes.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ def attribute_context(interface, attribute):
'reflect_missing': extended_attributes.get('ReflectMissing'),
'reflect_only': extended_attribute_value_as_list(attribute, 'ReflectOnly'),
'runtime_enabled_function': v8_utilities.runtime_enabled_function_name(attribute), # [RuntimeEnabled]
'runtime_feature_name': v8_utilities.runtime_feature_name(attribute), # [RuntimeEnabled]
'should_be_exposed_to_script': not (is_implemented_in_private_script and is_only_exposed_to_private_script),
'world_suffixes': (
['', 'ForMainWorld']
Expand Down
19 changes: 0 additions & 19 deletions third_party/WebKit/Source/bindings/scripts/v8_interface.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,25 +282,6 @@ def interface_context(interface):

context.update({
'attributes': attributes,
'has_accessor_configuration': any(
not (attribute['exposed_test'] or
attribute['runtime_enabled_function']) and
not attribute['is_data_type_property'] and
attribute['should_be_exposed_to_script']
for attribute in attributes),
'has_attribute_configuration': any(
not (attribute['exposed_test'] or
attribute['runtime_enabled_function']) and
attribute['is_data_type_property'] and
attribute['should_be_exposed_to_script']
for attribute in attributes),
'has_constructor_attributes': any(
attribute['constructor_type']
for attribute in attributes),
'has_replaceable_attributes': any(
attribute['is_replaceable'] and
not attribute['is_data_type_property']
for attribute in attributes),
})

# Methods
Expand Down
11 changes: 11 additions & 0 deletions third_party/WebKit/Source/bindings/scripts/v8_utilities.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,17 @@ def uncapitalize(name):
return name[0].lower() + name[1:]


def unique_by(dict_list, key):
"""Returns elements from a list of dictionaries with unique values for the named key."""
keys_seen = set()
filtered_list = []
for item in dict_list:
if item.get(key) not in keys_seen:
filtered_list.append(item)
keys_seen.add(item.get(key))
return filtered_list


################################################################################
# C++
################################################################################
Expand Down
74 changes: 35 additions & 39 deletions third_party/WebKit/Source/bindings/templates/dictionary_v8.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,51 +43,47 @@ void {{v8_class}}::toImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value, {{
}
{% endif %}
{% for member in members %}
{% if member.runtime_enabled_function %}
if ({{member.runtime_enabled_function}}()) {
{% else %}
{
{% filter runtime_enabled(member.runtime_enabled_function) %}
v8::Local<v8::Value> {{member.name}}Value;
if (!v8Object->Get(isolate->GetCurrentContext(), v8String(isolate, "{{member.name}}")).ToLocal(&{{member.name}}Value)) {
exceptionState.rethrowV8Exception(block.Exception());
return;
}
if ({{member.name}}Value.IsEmpty() || {{member.name}}Value->IsUndefined()) {
{% if member.is_required %}
exceptionState.throwTypeError("required member {{member.name}} is undefined.");
return;
{% else %}
// Do nothing.
{% endif %}
{% if member.is_nullable %}
} else if ({{member.name}}Value->IsNull()) {
impl.{{member.null_setter_name}}();
{% endif %}
v8::Local<v8::Value> {{member.name}}Value;
if (!v8Object->Get(isolate->GetCurrentContext(), v8String(isolate, "{{member.name}}")).ToLocal(&{{member.name}}Value)) {
exceptionState.rethrowV8Exception(block.Exception());
} else {
{% if member.deprecate_as %}
Deprecation::countDeprecationIfNotPrivateScript(isolate, currentExecutionContext(isolate), UseCounter::{{member.deprecate_as}});
{% endif %}
{{v8_value_to_local_cpp_value(member) | indent(8)}}
{% if member.is_interface_type %}
if (!{{member.name}} && !{{member.name}}Value->IsNull()) {
exceptionState.throwTypeError("member {{member.name}} is not of type {{member.idl_type}}.");
return;
}
if ({{member.name}}Value.IsEmpty() || {{member.name}}Value->IsUndefined()) {
{% if member.is_required %}
exceptionState.throwTypeError("required member {{member.name}} is undefined.");
return;
{% else %}
// Do nothing.
{% endif %}
{% if member.is_nullable %}
} else if ({{member.name}}Value->IsNull()) {
impl.{{member.null_setter_name}}();
{% endif %}
} else {
{% if member.deprecate_as %}
Deprecation::countDeprecationIfNotPrivateScript(isolate, currentExecutionContext(isolate), UseCounter::{{member.deprecate_as}});
{% endif %}
{{v8_value_to_local_cpp_value(member) | indent(12)}}
{% if member.is_interface_type %}
if (!{{member.name}} && !{{member.name}}Value->IsNull()) {
exceptionState.throwTypeError("member {{member.name}} is not of type {{member.idl_type}}.");
return;
}
{% endif %}
{% if member.enum_values %}
{{declare_enum_validation_variable(member.enum_values) | indent(12)}}
if (!isValidEnum({{member.name}}, validValues, WTF_ARRAY_LENGTH(validValues), "{{member.enum_type}}", exceptionState))
return;
{% elif member.is_object %}
if (!{{member.name}}.isObject()) {
exceptionState.throwTypeError("member {{member.name}} is not an object.");
return;
}
{% endif %}
impl.{{member.setter_name}}({{member.name}});
{% if member.enum_values %}
{{declare_enum_validation_variable(member.enum_values) | indent(8)}}
if (!isValidEnum({{member.name}}, validValues, WTF_ARRAY_LENGTH(validValues), "{{member.enum_type}}", exceptionState))
return;
{% elif member.is_object %}
if (!{{member.name}}.isObject()) {
exceptionState.throwTypeError("member {{member.name}} is not an object.");
return;
}
{% endif %}
impl.{{member.setter_name}}({{member.name}});
}
{% endfilter %}

{% endfor %}
}
Expand Down
53 changes: 12 additions & 41 deletions third_party/WebKit/Source/bindings/templates/interface_base.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,7 @@ bool securityCheck(v8::Local<v8::Context> accessingContext, v8::Local<v8::Object
method_implemented_in_private_script, generate_post_message_impl,
runtime_determined_length_method, runtime_determined_maxarg_method
with context %}
{% for method in methods %}
{% if method.should_be_exposed_to_script %}
{% for method in methods if method.should_be_exposed_to_script %}
{% for world_suffix in method.world_suffixes %}
{% if not method.is_custom and not method.is_post_message and method.visible %}
{{generate_method(method, world_suffix)}}
Expand Down Expand Up @@ -153,7 +152,6 @@ bool securityCheck(v8::Local<v8::Context> accessingContext, v8::Local<v8::Object
{{origin_safe_method_getter(method, world_suffix)}}
{% endif %}
{% endfor %}
{% endif %}
{% endfor %}
{% if iterator_method %}
{{generate_method(iterator_method)}}
Expand Down Expand Up @@ -189,19 +187,15 @@ bool securityCheck(v8::Local<v8::Context> accessingContext, v8::Local<v8::Object
{##############################################################################}
{% block install_attributes %}
{% from 'attributes.cpp' import attribute_configuration with context %}
{% if has_attribute_configuration %}
{% if attributes | has_attribute_configuration %}
// Suppress warning: global constructors, because AttributeConfiguration is trivial
// and does not depend on another global objects.
#if defined(COMPONENT_BUILD) && defined(WIN32) && COMPILER(CLANG)
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wglobal-constructors"
#endif
const V8DOMConfiguration::AttributeConfiguration {{v8_class}}Attributes[] = {
{% for attribute in attributes
if not (attribute.exposed_test or
attribute.runtime_enabled_function) and
attribute.is_data_type_property and
attribute.should_be_exposed_to_script %}
{% for attribute in attributes | has_attribute_configuration %}
{{attribute_configuration(attribute)}},
{% endfor %}
};
Expand All @@ -214,13 +208,9 @@ const V8DOMConfiguration::AttributeConfiguration {{v8_class}}Attributes[] = {
{##############################################################################}
{% block install_accessors %}
{% from 'attributes.cpp' import attribute_configuration with context %}
{% if has_accessor_configuration %}
{% if attributes | has_accessor_configuration %}
const V8DOMConfiguration::AccessorConfiguration {{v8_class}}Accessors[] = {
{% for attribute in attributes
if not (attribute.exposed_test or
attribute.runtime_enabled_function) and
not attribute.is_data_type_property and
attribute.should_be_exposed_to_script %}
{% for attribute in attributes | has_accessor_configuration %}
{{attribute_configuration(attribute)}},
{% endfor %}
};
Expand Down Expand Up @@ -284,27 +274,20 @@ static void install{{v8_class}}Template(v8::Isolate* isolate, const DOMWrapperWo
{% endif %}

// Register DOM constants, attributes and operations.
{% if runtime_enabled_function %}
if ({{runtime_enabled_function}}()) {
{% endif %}
{% filter indent(4 if runtime_enabled_function else 0, true) %}
{% filter runtime_enabled(runtime_enabled_function) %}
{% if constants %}
{{install_constants() | indent}}
{% endif %}
{% if has_attribute_configuration %}
{% if attributes | has_attribute_configuration %}
V8DOMConfiguration::installAttributes(isolate, world, instanceTemplate, prototypeTemplate, {{'%sAttributes' % v8_class}}, {{'WTF_ARRAY_LENGTH(%sAttributes)' % v8_class}});
{% endif %}
{% if has_accessor_configuration %}
{% if attributes | has_accessor_configuration %}
V8DOMConfiguration::installAccessors(isolate, world, instanceTemplate, prototypeTemplate, interfaceTemplate, signature, {{'%sAccessors' % v8_class}}, {{'WTF_ARRAY_LENGTH(%sAccessors)' % v8_class}});
{% endif %}
{% if method_configuration_methods %}
V8DOMConfiguration::installMethods(isolate, world, instanceTemplate, prototypeTemplate, interfaceTemplate, signature, {{'%sMethods' % v8_class}}, {{'WTF_ARRAY_LENGTH(%sMethods)' % v8_class}});
{% endif %}
{% endfilter %}{{newline}}
{% if runtime_enabled_function %}
} // if ({{runtime_enabled_function}}())
{% endif %}

{% endfilter %}
{%- if has_access_check_callbacks and not is_partial %}{{newline}}
// Cross-origin access check
instanceTemplate->SetAccessCheckCallback({{cpp_class}}V8Internal::securityCheck, v8::External::New(isolate, const_cast<WrapperTypeInfo*>(&{{v8_class}}::wrapperTypeInfo)));
Expand All @@ -315,21 +298,9 @@ static void install{{v8_class}}Template(v8::Isolate* isolate, const DOMWrapperWo
prototypeTemplate->SetIntrinsicDataProperty(v8::Symbol::GetIterator(isolate), v8::kArrayProto_values, v8::DontEnum);
{% endif %}

{%- set runtime_enabled_features = dict() %}
{% for attribute in attributes
if attribute.runtime_enabled_function and
not attribute.exposed_test %}
{% if attribute.runtime_enabled_function not in runtime_enabled_features %}
{% set unused = runtime_enabled_features.update({attribute.runtime_enabled_function: []}) %}
{% endif %}
{% set unused = runtime_enabled_features.get(attribute.runtime_enabled_function).append(attribute) %}
{% endfor %}
{% for runtime_enabled_feature in runtime_enabled_features | sort %}{{newline}}
if ({{runtime_enabled_feature}}()) {
{% set distinct_attributes = [] %}
{% for attribute in runtime_enabled_features.get(runtime_enabled_feature) | sort
if attribute.name not in distinct_attributes %}
{% set unused = distinct_attributes.append(attribute.name) %}
{%- for group in attributes | runtime_enabled_attributes | groupby('runtime_feature_name') %}{{newline}}
if ({{group.list[0].runtime_enabled_function}}()) {
{% for attribute in group.list | unique_by('name') | sort %}
{% if attribute.is_data_type_property %}
const V8DOMConfiguration::AttributeConfiguration attribute{{attribute.name}}Configuration = \
{{attribute_configuration(attribute)}};
Expand Down
Loading

0 comments on commit 891d66a

Please sign in to comment.