Clean up type handling in make_computed_style_base.py.

Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> type_name (not really storage related)
- storage_type_path -> field_type_path (not really storage related)
- field_family -> field_role (family is vague, role is better IMO)
- keyword_only -> field_template (field_template generalises
  keyword_only to more than just enums).

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool etc. Previously, 'storage_type'
  was same as 'type_name', which means templates could not tell
  whether the field is a enum or something else.

This patch does not change behaviour.

BUG=628043
Review-Url: https://codereview.chromium.org/2692803002
Cr-Commit-Position: refs/heads/master@{#451534}
Committed: https://chromium.googlesource.com/chromium/src/+/e3c137153d7e9995c53612e071d3afe094de39e2

[shend, 2017-02-13 04:34:04]

Description was changed from

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

As of this patch:
- Every field has a 'type', which currently can only be 'keyword', but
  can be 'primitive' or 'external' when we support those in the future.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
- For 'keyword' fields, the corresponding enum is generated, but this
  behaviour can be overridden by specifying 'field_type_path' to point
  to an existing enum. This will also be used for 'external' fields in
  the future. When 'field_type_path' is specified, 'type_name' is
  automatically set based on the name of the file.

To do this, this patch:
- Changes 'keyword_only' to 'field_type' to generalise it.
- Replace instances of 'storage' to 'field', since 'storage' refers to
  the actual memory layout, not the interface.
- Use 'type_name' to hold name of enum, instead of using 'field_type'.

BUG=628043
==========

to

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> field_template
- storage_type_path -> field_type_path
- field_family -> field_role

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool. For 'external', it'll be Length
  etc.

This patch does not change behaviour.

BUG=628043
==========

[shend, 2017-02-13 04:44:29]

Description was changed from

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> field_template
- storage_type_path -> field_type_path
- field_family -> field_role

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool. For 'external', it'll be Length
  etc.

This patch does not change behaviour.

BUG=628043
==========

to

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> field_template (avoid confusion with type_name)
- storage_type_path -> field_type_path (not really storage, since everything is
stored as unsigned)
- field_family -> field_role (family is vague, role is better IMO)

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool. For 'external', it'll be Length
  etc.

This patch does not change behaviour.

BUG=628043
==========

[shend, 2017-02-13 04:46:52]

Description was changed from

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> field_template (avoid confusion with type_name)
- storage_type_path -> field_type_path (not really storage, since everything is
stored as unsigned)
- field_family -> field_role (family is vague, role is better IMO)

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool. For 'external', it'll be Length
  etc.

This patch does not change behaviour.

BUG=628043
==========

to

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> field_template (avoid confusion with type_name)
- storage_type_path -> field_type_path (not really storage, since
  everything is stored as unsigned)
- field_family -> field_role (family is vague, role is better IMO)

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool. For 'external', it'll be Length
  etc.

This patch does not change behaviour.

BUG=628043
==========

[shend, 2017-02-13 04:49:14]

Description was changed from

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> field_template (avoid confusion with type_name)
- storage_type_path -> field_type_path (not really storage, since
  everything is stored as unsigned)
- field_family -> field_role (family is vague, role is better IMO)

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool. For 'external', it'll be Length
  etc.

This patch does not change behaviour.

BUG=628043
==========

to

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> field_template (avoid confusion with type_name)
- storage_type_path -> field_type_path (not really storage, since
  everything is stored as unsigned)
- field_family -> field_role (family is vague, role is better IMO)

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool etc. Previously, 'storage_type'
  was same as 'type_name', which means templates could not tell
  whether the field is a enum or something else.

This patch does not change behaviour.

BUG=628043
==========

[shend, 2017-02-13 04:49:42]

shend@chromium.org changed reviewers:
+ nainar@chromium.org

[shend, 2017-02-13 04:49:43]

Hi Naina, PTAL. Let me know what you think of the names.

[shend, 2017-02-13 05:26:57]

shend@chromium.org changed reviewers:
+ meade@chromium.org
- nainar@chromium.org

[shend, 2017-02-13 05:26:58]

-nainar since she's not familiar with this code.
Eddy PTAL :)

[shend, 2017-02-13 06:44:35]

Description was changed from

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> field_template (avoid confusion with type_name)
- storage_type_path -> field_type_path (not really storage, since
  everything is stored as unsigned)
- field_family -> field_role (family is vague, role is better IMO)

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool etc. Previously, 'storage_type'
  was same as 'type_name', which means templates could not tell
  whether the field is a enum or something else.

This patch does not change behaviour.

BUG=628043
==========

to

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> field_template (avoid confusion with type_name)
- storage_type_path -> field_type_path (not really storage, since
  everything is stored as unsigned)
- field_family -> field_role (family is vague, role is better IMO)

and adds a new attribute 'type_name' to Field which holds the actual
name of the type,

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool etc. Previously, 'storage_type'
  was same as 'type_name', which means templates could not tell
  whether the field is a enum or something else.

This patch does not change behaviour.

BUG=628043
==========

[meade_UTC10, 2017-02-13 06:46:24]

https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/build/scripts/make_computed_style_base.py
(right):

https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_computed_style_base.py:40: #
Affects how the field is generated (keyword, primitive, external)
How does it affect it? What are the valid values? Do you assert somewhere that
it is one of the valid values?

Also where is this used? Is it only in the jinja templates (or is it a typo that
some things below are field_template)?

https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_computed_style_base.py:45:
'type_name',
Could you describe the relationship between these?

e.g.
"I f it's a generated enum, type_path is empty, and type_name is the enum name.
If it's a platform type, type_name is the name of the type, and type_path is the
path to the directory containing that type (relative to <someDirectory>)"

https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl
(right):

https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl:108:
void {{field.setter_method_name}}({{field.type_name}} v) { {{field.name}} =
static_cast<unsigned>(v); }
This might be hard given it's jinja... but is there a way to make this fit lines
better so it's easier to read?

https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSProperties.json5 (right):

https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSProperties.json5:59: // Affects how this
field is generated in terms of storage and getter/setter methods.
How is it affected? + the same question as before about 'template' vs
'field_template'

[shend, 2017-02-13 06:50:34]

Description was changed from

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> field_template (avoid confusion with type_name)
- storage_type_path -> field_type_path (not really storage, since
  everything is stored as unsigned)
- field_family -> field_role (family is vague, role is better IMO)

and adds a new attribute 'type_name' to Field which holds the actual
name of the type,

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool etc. Previously, 'storage_type'
  was same as 'type_name', which means templates could not tell
  whether the field is a enum or something else.

This patch does not change behaviour.

BUG=628043
==========

to

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> type_name (not really storage related)
- storage_type_path -> field_type_path (not really storage related)
- field_family -> field_role (family is vague, role is better IMO)
- keyword_only -> field_template (field_template generalises
  keyword_only to more than one type).

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool etc. Previously, 'storage_type'
  was same as 'type_name', which means templates could not tell
  whether the field is a enum or something else.

This patch does not change behaviour.

BUG=628043
==========

[shend, 2017-02-13 07:15:42]

On 2017/02/13 at 06:46:24, meade wrote:
>
https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/build/scripts/make_computed_style_base.py
(right):
> 
>
https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/build/scripts/make_computed_style_base.py:40: #
Affects how the field is generated (keyword, primitive, external)
> How does it affect it? What are the valid values? Do you assert somewhere that
it is one of the valid values?
> 
> Also where is this used? Is it only in the jinja templates (or is it a typo
that some things below are field_template)?
> 
>
https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/build/scripts/make_computed_style_base.py:45:
'type_name',
> Could you describe the relationship between these?
> 
> e.g.
> "I f it's a generated enum, type_path is empty, and type_name is the enum
name.
> If it's a platform type, type_name is the name of the type, and type_path is
the path to the directory containing that type (relative to <someDirectory>)"
> 
>
https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
> File
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl
(right):
> 
>
https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
>
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl:108:
void {{field.setter_method_name}}({{field.type_name}} v) { {{field.name}} =
static_cast<unsigned>(v); }
> This might be hard given it's jinja... but is there a way to make this fit
lines better so it's easier to read?
> 
>
https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/css/CSSProperties.json5 (right):
> 
>
https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/CSSProperties.json5:59: // Affects how this
field is generated in terms of storage and getter/setter methods.
> How is it affected? + the same question as before about 'template' vs
'field_template'

Re: more comments, yes I'll definite add more comments to the JSON5 since it's
"user-facing". My question is, in the generator script, can I just say "this
attribute corresponds to the field_template attribute in the JSON5" to avoid
duplicating comments?

Re: template vs field_template. Some attributes in the JSON5 are prefixed with
'field' and some are not. I omitted the field prefix in the Field class since
it'd be like 'field.field_template', but 'field.type_name'. Idk what's the best
way to make this consistent.

Also, I've updated the description. Turns out I confused myself and the actual
rename isn't too complicated.

[shend, 2017-02-13 07:15:54]

Description was changed from

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> type_name (not really storage related)
- storage_type_path -> field_type_path (not really storage related)
- field_family -> field_role (family is vague, role is better IMO)
- keyword_only -> field_template (field_template generalises
  keyword_only to more than one type).

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool etc. Previously, 'storage_type'
  was same as 'type_name', which means templates could not tell
  whether the field is a enum or something else.

This patch does not change behaviour.

BUG=628043
==========

to

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> type_name (not really storage related)
- storage_type_path -> field_type_path (not really storage related)
- field_family -> field_role (family is vague, role is better IMO)
- keyword_only -> field_template (field_template generalises
  keyword_only to more than just enums).

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool etc. Previously, 'storage_type'
  was same as 'type_name', which means templates could not tell
  whether the field is a enum or something else.

This patch does not change behaviour.

BUG=628043
==========

[meade_UTC10, 2017-02-16 00:56:29]

Re template vs field_template, it'd be better to make them consistent imo -
field.field_template is ok, or you can rename them in the json5 in a followup CL
if you think that reads better.

https://codereview.chromium.org/2692803002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/build/scripts/make_computed_style_base.py
(right):

https://codereview.chromium.org/2692803002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_computed_style_base.py:48: #
Affects how the field is generated (keyword, primitive, external)
You didn't address the comment about how does this affect the generation? At
least point to the file the reader should look in.

[meade_UTC10, 2017-02-16 00:56:45]

lgtm

Please fix the comments comment :)

[shend, 2017-02-16 00:57:30]

On 2017/02/16 at 00:56:45, meade wrote:
> lgtm
> 
> Please fix the comments comment :)

yep, will do.

[shend, 2017-02-16 01:01:30]

On 2017/02/16 at 00:56:29, meade wrote:
> Re template vs field_template, it'd be better to make them consistent imo -
field.field_template is ok, or you can rename them in the json5 in a followup CL
if you think that reads better.
> 
>
https://codereview.chromium.org/2692803002/diff/140001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/build/scripts/make_computed_style_base.py
(right):
> 
>
https://codereview.chromium.org/2692803002/diff/140001/third_party/WebKit/Sou...
> third_party/WebKit/Source/build/scripts/make_computed_style_base.py:48: #
Affects how the field is generated (keyword, primitive, external)
> You didn't address the comment about how does this affect the generation? At
least point to the file the reader should look in.

cool, I'll make it consistent so that python follows the JSON. When we implement
CSS properties with multiple fields, the JSON will probably look like:

{
  name: "vertical-align",
  fields: [
    { name: "keywords", template: "keyword", keywords: [...] },
    { name: "length", template: "external", type_path: "..." },
  ]
}

so all the names will become consistent :)

[shend, 2017-02-16 02:48:58]

Addressed comments

https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/build/scripts/make_computed_style_base.py
(right):

https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_computed_style_base.py:40: #
Affects how the field is generated (keyword, primitive, external)
On 2017/02/13 at 06:46:24, Eddy wrote:
> How does it affect it? What are the valid values? Do you assert somewhere that
it is one of the valid values?
> 
> Also where is this used? Is it only in the jinja templates (or is it a typo
that some things below are field_template)?

Added explanations in the JSON5. Made naming consistent.

https://codereview.chromium.org/2692803002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_computed_style_base.py:45:
'type_name',
On 2017/02/13 at 06:46:24, Eddy wrote:
> Could you describe the relationship between these?
> 
> e.g.
> "I f it's a generated enum, type_path is empty, and type_name is the enum
name.
> If it's a platform type, type_name is the name of the type, and type_path is
the path to the directory containing that type (relative to <someDirectory>)"

Added comment to refer readers to the JSON5 file, which has a detailed
explanation already.

https://codereview.chromium.org/2692803002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/build/scripts/make_computed_style_base.py
(right):

https://codereview.chromium.org/2692803002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_computed_style_base.py:48: #
Affects how the field is generated (keyword, primitive, external)
On 2017/02/16 at 00:56:29, Eddy wrote:
> You didn't address the comment about how does this affect the generation? At
least point to the file the reader should look in.

Pointed to the JSON5. Since the attribute names are the same as the JSON5 now,
there should be no confusion.

[shend, 2017-02-16 06:11:36]

The CQ bit was checked by shend@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-16 06:12:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-16 06:27:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-16 06:27:35]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[shend, 2017-02-16 06:36:20]

The CQ bit was checked by shend@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-16 06:37:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-16 07:39:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-16 07:39:54]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[shend, 2017-02-16 21:55:54]

The CQ bit was checked by shend@chromium.org

[shend, 2017-02-16 21:55:54]

The patchset sent to the CQ was uploaded after l-g-t-m from meade@chromium.org
Link to the patchset: https://codereview.chromium.org/2692803002/#ps200001
(title: "Rebase")

[commit-bot: I haz the power, 2017-02-16 21:56:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[shend, 2017-02-16 22:49:53]

The CQ bit was unchecked by shend@chromium.org

[shend, 2017-02-17 01:54:34]

The CQ bit was checked by shend@chromium.org

[shend, 2017-02-17 01:54:35]

The patchset sent to the CQ was uploaded after l-g-t-m from meade@chromium.org
Link to the patchset: https://codereview.chromium.org/2692803002/#ps220001
(title: "Update comments")

[commit-bot: I haz the power, 2017-02-17 01:56:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-17 03:59:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-17 03:59:39]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)

[shend, 2017-02-19 22:28:59]

The CQ bit was checked by shend@chromium.org

[commit-bot: I haz the power, 2017-02-19 22:29:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-20 00:06:47]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1487543339675030,
"parent_rev": "f0f67bb9cea082b6e8b1fdf0e392e2fdea524eda", "commit_rev":
"e3c137153d7e9995c53612e071d3afe094de39e2"}

[commit-bot: I haz the power, 2017-02-20 00:07:23]

Description was changed from

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> type_name (not really storage related)
- storage_type_path -> field_type_path (not really storage related)
- field_family -> field_role (family is vague, role is better IMO)
- keyword_only -> field_template (field_template generalises
  keyword_only to more than just enums).

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool etc. Previously, 'storage_type'
  was same as 'type_name', which means templates could not tell
  whether the field is a enum or something else.

This patch does not change behaviour.

BUG=628043
==========

to

==========
Clean up type handling in make_computed_style_base.py.

Currently, the way we specify the types of fields in ComputedStyleBase
is confusing and inconsistent. This makes it difficult to extend the
generator to support non-enum types like ints and platform types.

This patch renames:
- storage_type -> type_name (not really storage related)
- storage_type_path -> field_type_path (not really storage related)
- field_family -> field_role (family is vague, role is better IMO)
- keyword_only -> field_template (field_template generalises
  keyword_only to more than just enums).

so that, as of this patch:
- Every field picks a 'template' which for now can only be 'keyword',
  but can be 'primitive' or 'external' when we support those later.
  This affects what the generated API for that field looks like.
- Every field has a 'type_name', which is the actual C++ type used in
  the generated code. For 'keyword' fields, this is the enum name.
  For 'primitive', it'll be int/bool etc. Previously, 'storage_type'
  was same as 'type_name', which means templates could not tell
  whether the field is a enum or something else.

This patch does not change behaviour.

BUG=628043

Review-Url: https://codereview.chromium.org/2692803002
Cr-Commit-Position: refs/heads/master@{#451534}
Committed:
https://chromium.googlesource.com/chromium/src/+/e3c137153d7e9995c53612e071d3...
==========

[commit-bot: I haz the power, 2017-02-20 00:07:24]

Committed patchset #12 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/e3c137153d7e9995c53612e071d3...