Skip to content

Commit

Permalink
Move fully generated StyleBuilderFunction methods to corresponding CS…
Browse files Browse the repository at this point in the history
…SPropertyAPIs.

This is a resubmission of an earlier cl (crrev.com/c/646928), which was 
reverted because it caused a Chrome Win Build error. The linker error suggest the
size of static lib was too large, exceeding win limit. In this cl, we increase
split_count from 5 to 10 for all systems.

Bug: 751354
Change-Id: I536bf5e8adbdd3da58418e77cbc882432e4b3c7f
Reviewed-on: https://chromium-review.googlesource.com/648536
Reviewed-by: meade_UTC10 <meade@chromium.org>
Commit-Queue: Jia Meng <jiameng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500221}
  • Loading branch information
jm318 authored and Commit Bot committed Sep 7, 2017
1 parent 5b4ce66 commit 73b2912
Show file tree
Hide file tree
Showing 10 changed files with 183 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,63 @@
import os
import sys
sys.path.append(os.path.join(os.path.dirname(__file__), '../../..'))
import types

import json5_generator
import template_expander

from collections import namedtuple, Counter
from make_css_property_api_base import CSSPropertyAPIWriter
from name_utilities import upper_camel_case


class ApiMethod(namedtuple('ApiMethod', 'name,return_type,parameters')):
pass


def apply_computed_style_builder_function_parameters(property_):
"""Generate function parameters for generated implementations of Apply*
"""
def set_if_none(property_, key, value):
if property_[key] is None:
property_[key] = value

# Functions should only be declared on the API classes if they are
# implemented and not shared (denoted by api_class = true. Shared classes
# are denoted by api_class = "some string").
property_['should_declare_application_functions'] = \
property_['api_class'] \
and isinstance(property_['api_class'], types.BooleanType) \
and property_['is_property'] \
and not property_['use_handlers_for'] \
and not property_['longhands'] \
and not property_['direction_aware'] \
and not property_['builder_skip']
if not property_['should_declare_application_functions']:
return

if property_['custom_all']:
property_['custom_initial'] = True
property_['custom_inherit'] = True
property_['custom_value'] = True

name = property_['name_for_methods']
if not name:
name = upper_camel_case(property_['name']).replace('Webkit', '')
simple_type_name = str(property_['type_name']).split('::')[-1]

set_if_none(property_, 'name_for_methods', name)
set_if_none(property_, 'type_name', 'E' + name)
set_if_none(
property_, 'getter', name if simple_type_name != name else 'Get' + name)
set_if_none(property_, 'setter', 'Set' + name)
set_if_none(property_, 'inherited', False)
set_if_none(property_, 'initial', 'Initial' + name)

if property_['inherited']:
property_['is_inherited_setter'] = 'Set' + name + 'IsInherited'


class CSSPropertyAPIHeadersWriter(CSSPropertyAPIWriter):
def __init__(self, json5_file_paths):
super(CSSPropertyAPIHeadersWriter, self).__init__([json5_file_paths[0]])
Expand Down Expand Up @@ -47,6 +92,7 @@ def __init__(self, json5_file_paths):
property_['api_methods'] = methods
classname = self.get_classname(property_)
assert classname is not None
apply_computed_style_builder_function_parameters(property_)
self._outputs[classname + '.h'] = (
self.generate_property_api_h_builder(classname, property_))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class CSSParserContext;
class CSSParserLocalContext;
class CSSProperty;
class CSSValue;
class StyleResolverState;

class CSSPropertyAPI {
public:
Expand Down Expand Up @@ -53,6 +54,11 @@ class CSSPropertyAPI {
virtual bool IsDescriptor() const { return false; }
virtual bool SupportsPercentage() const { return false; }
virtual bool IsProperty() const { return true; }
virtual void ApplyInitial(StyleResolverState&) const { NOTREACHED(); }
virtual void ApplyInherit(StyleResolverState&) const { NOTREACHED(); }
virtual void ApplyValue(StyleResolverState&, const CSSValue&) const {
NOTREACHED();
}
};

{% for api_class_data in api_classes_by_property_id %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,27 @@
// found in the LICENSE file.

{% from 'templates/macros.tmpl' import source_files_for_generated_file %}
{% from 'core/css/properties/templates/application_macros.tmpl' import set_value, convert_and_set_value %}
{{source_files_for_generated_file(template_file, input_files)}}

#ifndef {{api_classname}}_h
#define {{api_classname}}_h

// TODO(jiameng): every property API header file needs to include a subset of
// headers below. Ideally each property header should include required headers
// only. Once all properties have their StyleBuilderFunctions moved to APIs,
// we will need to revisit this issue.
#include "core/CSSPropertyNames.h"
#include "core/css/CSSIdentifierValue.h"
#include "core/css/CSSPrimitiveValue.h"
#include "core/css/CSSPrimitiveValueMappings.h"
#include "core/css/properties/CSSPropertyAPI.h"
#include "core/css/resolver/StyleBuilderConverter.h"
#include "core/css/resolver/StyleResolverState.h"
#include "core/style/ComputedStyle.h"
#include "core/style/SVGComputedStyle.h"
#include "platform/heap/HeapAllocator.h"
#include "core/css/resolver/StyleResolverState.h"

namespace blink {

Expand Down Expand Up @@ -39,6 +52,50 @@ class {{api_classname}} : public CSSPropertyAPI {
{% if property.is_descriptor %}
bool IsDescriptor() const override { return true; }
{% endif %}

{% if property.should_declare_application_functions %}
{% if not property.custom_initial %}
void ApplyInitial(StyleResolverState& state) const override {
{% if property.svg %}
{{set_value(property)}}(SVGComputedStyle::{{property.initial}}());
{% elif property.font %}
{{set_value(property)}}(FontBuilder::{{property.initial}}());
{% else %}
{{set_value(property)}}(ComputedStyle::{{property.initial}}());
{% endif %}
{% if property.independent %}
state.Style()->{{property.is_inherited_setter}}(false);
{% endif %}
}
{% endif %}
{# TODO(meade): else emit function declaration only #}

{% if not property.custom_inherit %}
void ApplyInherit(StyleResolverState& state) const override {
{% if property.svg %}
{{set_value(property)}}(state.ParentStyle()->SvgStyle().{{property.getter}}());
{% elif property.font %}
{{set_value(property)}}(state.ParentFontDescription().{{property.getter}}());
{% else %}
{{set_value(property)}}(state.ParentStyle()->{{property.getter}}());
{% endif %}
{% if property.independent %}
state.Style()->{{property.is_inherited_setter}}(true);
{% endif %}
}
{% endif %}
{# TODO(meade): else emit function declaration only #}

{% if not property.custom_value %}
void ApplyValue(StyleResolverState& state, const CSSValue& value) const override {
{{convert_and_set_value(property)}}
{% if property.independent %}
state.Style()->{{property.is_inherited_setter}}(false);
{% endif %}
}
{% endif %}
{# TODO(meade): else emit function declaration only #}
{% endif %}
{% if 'Percent' in property.typedom_types %}
bool SupportsPercentage() const override { return true; }
{% endif %}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// 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.

{% macro set_value(property) %}
{% if property.svg %}
state.Style()->AccessSVGStyle().{{property.setter}}
{%- elif property.font %}
state.GetFontBuilder().{{property.setter}}
{%- else %}
state.Style()->{{property.setter}}
{%- endif %}
{% endmacro %}

{% macro convert_and_set_value(property) %}
{% if property.converter %}
{{set_value(property)}}(StyleBuilderConverter::{{property.converter}}(state, value));
{# TODO(sashab): Remove this list from this file. #}
{% elif property.type_name in ['short', 'unsigned short', 'int', 'unsigned int', 'unsigned', 'float', 'LineClampValue'] %}
{{set_value(property)}}(ToCSSPrimitiveValue(value).ConvertTo<{{property.type_name}}>());
{%- else %}
{{set_value(property)}}(ToCSSIdentifierValue(value).ConvertTo<{{property.type_name}}>());
{%- endif %}
{% endmacro %}

{% macro set_is_inherited(property) %}
state.Style()->{{property.is_inherited_setter}}
{% endmacro %}
27 changes: 21 additions & 6 deletions third_party/WebKit/Source/build/scripts/make_style_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

import sys
import types

from core.css import css_properties
import json5_generator
Expand All @@ -40,14 +41,15 @@ def set_if_none(property_, key, value):
if property_[key] is None:
property_[key] = value

# TODO(shend): Use name_utilities for manipulating names.
# TODO(shend): Rearrange the code below to separate assignment and set_if_none
# TODO(meade): Delete this once all methods are moved to CSSPropertyAPIs.
upper_camel = upper_camel_case(property_['name'])
set_if_none(property_, 'name_for_methods', upper_camel.replace('Webkit', ''))
set_if_none(
property_, 'name_for_methods', upper_camel.replace('Webkit', ''))
name = property_['name_for_methods']
simple_type_name = str(property_['type_name']).split('::')[-1]
set_if_none(property_, 'type_name', 'E' + name)
set_if_none(property_, 'getter', name if simple_type_name != name else 'Get' + name)
set_if_none(
property_, 'getter', name if simple_type_name != name else 'Get' + name)
set_if_none(property_, 'setter', 'Set' + name)
set_if_none(property_, 'inherited', False)
set_if_none(property_, 'initial', 'Initial' + name)
Expand All @@ -58,9 +60,22 @@ def set_if_none(property_, key, value):
property_['custom_value'] = True
if property_['inherited']:
property_['is_inherited_setter'] = 'Set' + name + 'IsInherited'
property_['should_declare_functions'] = not property_['use_handlers_for'] and not property_['longhands'] \
and not property_['direction_aware'] and not property_['builder_skip'] \
property_['should_declare_functions'] = \
not property_['use_handlers_for'] \
and not property_['longhands'] \
and not property_['direction_aware'] \
and not property_['builder_skip'] \
and property_['is_property']
# Functions should only be used in StyleBuilder if the CSSPropertyAPI
# class is shared or not implemented yet (shared classes are denoted by
# api_class = "some string").
property_['use_api_in_stylebuilder'] = \
property_['should_declare_functions'] \
and not (property_['custom_initial'] or
property_['custom_inherit'] or
property_['custom_value']) \
and property_['api_class'] \
and isinstance(property_['api_class'], types.BooleanType)


class StyleBuilderWriter(css_properties.CSSProperties):
Expand Down
1 change: 1 addition & 0 deletions third_party/WebKit/Source/build/scripts/scripts.gni
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ scripts_for_json5_files = [
"$_scripts_dir/name_utilities.py",
"$_scripts_dir/template_expander.py",
"$_scripts_dir/templates/macros.tmpl",
"$_scripts_dir/core/css/properties/templates/application_macros.tmpl",
]

css_properties_files =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "StyleBuilderFunctions.h"
#include "core/css/CSSProperty.h"
#include "core/css/properties/CSSPropertyAPI.h"
#include "core/css/resolver/StyleResolverState.h"

// FIXME: currently we're just generating a switch statement, but we should
Expand All @@ -21,9 +22,19 @@ void StyleBuilder::ApplyProperty(CSSPropertyID property,
bool isInherit) {
switch (property) {
{% for property_id, property in properties.items()
if property.should_declare_functions or property.use_handlers_for %}
if property.should_declare_functions or property.use_handlers_for %}
{% set used_property = properties[property.use_handlers_for] or property %}
{% set used_property_id = used_property.property_id %}
{% if used_property.use_api_in_stylebuilder %}
case {{property_id}}:
if (isInitial)
Get{{used_property_id}}API().ApplyInitial(state);
else if (isInherit)
Get{{used_property_id}}API().ApplyInherit(state);
else
Get{{used_property_id}}API().ApplyValue(state, value);
return;
{% else %}
case {{property_id}}:
if (isInitial)
StyleBuilderFunctions::applyInitial{{used_property_id}}(state);
Expand All @@ -32,6 +43,7 @@ void StyleBuilder::ApplyProperty(CSSPropertyID property,
else
StyleBuilderFunctions::applyValue{{used_property_id}}(state, value);
return;
{% endif %}

{% endfor %}
case CSSPropertyVariable:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
{% from 'templates/macros.tmpl' import license, source_files_for_generated_file %}
{% from 'core/css/properties/templates/application_macros.tmpl' import set_value, convert_and_set_value %}

{#
This file is for property handlers which use the templating engine to
reduce (handwritten) code duplication.

The `properties' dict can be used to access a property's parameters in
jinja2 templates (i.e. setter, getter, initial, type_name)

TODO(meade): Delete this file once all StyleBuilderFunction generation
is moved to the CSSPropertyAPI.
#}

{{source_files_for_generated_file(template_file, input_files)}}
Expand All @@ -30,33 +35,15 @@ void StyleBuilderFunctions::applyInherit{{property_id}}(StyleResolverState& stat
{% macro declare_value_function(property_id) %}
void StyleBuilderFunctions::applyValue{{property_id}}(StyleResolverState& state, const CSSValue& value)
{%- endmacro %}
{% macro set_value(property) %}
{% if property.svg %}
state.Style()->AccessSVGStyle().{{property.setter}}
{%- elif property.font %}
state.GetFontBuilder().{{property.setter}}
{%- else %}
state.Style()->{{property.setter}}
{%- endif %}
{% endmacro %}
{% macro convert_and_set_value(property) %}
{% if property.converter %}
{{set_value(property)}}(StyleBuilderConverter::{{property.converter}}(state, value));
{# TODO(sashab): Remove this list from this file. #}
{% elif property.type_name in ['short', 'unsigned short', 'int', 'unsigned int', 'unsigned', 'float', 'LineClampValue'] %}
{{set_value(property)}}(ToCSSPrimitiveValue(value).ConvertTo<{{property.type_name}}>());
{%- else %}
{{set_value(property)}}(ToCSSIdentifierValue(value).ConvertTo<{{property.type_name}}>());
{%- endif %}
{% endmacro %}
{% macro set_is_inherited(property) %}
state.Style()->{{property.is_inherited_setter}}
{% endmacro %}

namespace blink {

{% for property_id, property in properties.items() if property.should_declare_functions %}
{% set apply_type = property.apply_type %}
{% for property_id, property in properties.items()
if property.should_declare_functions
and not property.use_api_in_stylebuilder %}
{% if not property.custom_initial %}
{{declare_initial_function(property_id)}} {
{% if property.svg %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ class StyleBuilderFunctions {
public:

{% for property_id, property in properties.items()
if property.should_declare_functions %}
if property.should_declare_functions
and not property.use_api_in_stylebuilder %}
static void applyInitial{{property_id}}(StyleResolverState&);
static void applyInherit{{property_id}}(StyleResolverState&);
static void applyValue{{property_id}}(StyleResolverState&, const CSSValue&);
Expand Down
2 changes: 1 addition & 1 deletion third_party/WebKit/Source/core/css/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import("//third_party/WebKit/Source/core/core.gni")

blink_core_sources("css") {
split_count = 5
split_count = 10
sources = [
"ActiveStyleSheets.cpp",
"ActiveStyleSheets.h",
Expand Down

0 comments on commit 73b2912

Please sign in to comment.