Added api_methods flag to CSSProperties.json5.

Added api_methods flag to CSSProperties.json5.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. 

This patch adds an api_methods=["method1", "method2"] flag to
CSSProperties.json5, which specifies which methods have been
implemented in the API implementation of that property. This is so
that when API .h files are generated, only methods implemented for
that property are defined.

It also adds "parseSingleValue" to the list of api_methods for all
existing properties with an API that implements parseSingleValue.

Before, when a method was added to the API, this method had to be
implemented in all API files - however, not every API should hold
each method. In this new design, to add a method to the API:
- Add the method name to the valid_values array in CSSProperties.json5.
- Add the method to CSSPropertyAPIFiles.h.tmpl
- Add the method name to the api_methods list in CSSProperties.json5
  for each property where it is implemented.

BUG=668012
Review-Url: https://codereview.chromium.org/2654403003
Cr-Original-Commit-Position: refs/heads/master@{#447427}
Committed: https://chromium.googlesource.com/chromium/src/+/a252f0fa5f780f176eb3388df35ec67abaf8ca79
Review-Url: https://codereview.chromium.org/2654403003
Cr-Commit-Position: refs/heads/master@{#447459}
Committed: https://chromium.googlesource.com/chromium/src/+/d965a330d470c634f1010dd90bc20cfb1e81971f

[aazzam, 2017-01-27 05:17:13]

The CQ bit was checked by aazzam@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-27 05:17:37]

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-01-27 06:57:23]

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

[commit-bot: I haz the power, 2017-01-27 06:57:24]

Dry run: This issue passed the CQ dry run.

[aazzam, 2017-01-30 00:10:52]

The CQ bit was checked by aazzam@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 00:11:10]

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-01-30 01:45:06]

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

[commit-bot: I haz the power, 2017-01-30 01:45:07]

Dry run: This issue passed the CQ dry run.

[aazzam, 2017-01-31 00:09:29]

Description was changed from

==========
Fixed generation so that only implemented methods are in .h file
==========

to

==========
Modified generator so that only implemented methods are in .h file.
==========

[aazzam, 2017-01-31 02:15:44]

Description was changed from

==========
Modified generator so that only implemented methods are in .h file.
==========

to

==========
Modified generator so that only implemented methods are in .h file.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. This patch modifies
make_css_rpoperty_apis.py and CSSPropertyAPIFiles.h.tmpl which
generate the .h files for implementing properties to allow for the
addition of new methods to the API.

This patch:
- Adds api_methods flag to CSSProperties.json5 which lists the
  functions implemented for this property.
- Passes implementedMethods into CSSPropertyAPIFiles.h.tmpl from
  make_css_property_apis.py.
- Wraps each function in the template in an if statement, so that
  CSSPropertyAPI<Property>.h file only declares the functions
  that are implemented in the corresponding .cpp file.
==========

[aazzam, 2017-01-31 02:20:09]

Description was changed from

==========
Modified generator so that only implemented methods are in .h file.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. This patch modifies
make_css_rpoperty_apis.py and CSSPropertyAPIFiles.h.tmpl which
generate the .h files for implementing properties to allow for the
addition of new methods to the API.

This patch:
- Adds api_methods flag to CSSProperties.json5 which lists the
  functions implemented for this property.
- Passes implementedMethods into CSSPropertyAPIFiles.h.tmpl from
  make_css_property_apis.py.
- Wraps each function in the template in an if statement, so that
  CSSPropertyAPI<Property>.h file only declares the functions
  that are implemented in the corresponding .cpp file.
==========

to

==========
Modified generator so that only implemented methods are in .h file.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. This patch modifies
make_css_rpoperty_apis.py and CSSPropertyAPIFiles.h.tmpl which
generate the .h files for implementing properties to allow for the
addition of new methods to the API.

This patch:
- Adds api_methods flag to CSSProperties.json5 which lists the
  names of functions implemented for this property.
- Modifies make_css_property_apis.py so that method names
  listed in CSSProperties.json5 are checked to be valid, and passed
  into CSSPropertyAPIFiles.h.tmpl.
- Wraps each function in the CSSPropertyAPIFiles.h template in an
  if statement, so that CSSPropertyAPI<Property>.h file only
  declares the functions that are implemented in the corresponding
  .cpp file.
==========

[aazzam, 2017-01-31 02:20:34]

Description was changed from

==========
Modified generator so that only implemented methods are in .h file.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. This patch modifies
make_css_rpoperty_apis.py and CSSPropertyAPIFiles.h.tmpl which
generate the .h files for implementing properties to allow for the
addition of new methods to the API.

This patch:
- Adds api_methods flag to CSSProperties.json5 which lists the
  names of functions implemented for this property.
- Modifies make_css_property_apis.py so that method names
  listed in CSSProperties.json5 are checked to be valid, and passed
  into CSSPropertyAPIFiles.h.tmpl.
- Wraps each function in the CSSPropertyAPIFiles.h template in an
  if statement, so that CSSPropertyAPI<Property>.h file only
  declares the functions that are implemented in the corresponding
  .cpp file.
==========

to

==========
Modified generator so that only implemented methods are in .h file.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. This patch modifies
make_css_property_apis.py and CSSPropertyAPIFiles.h.tmpl which
generate the .h files for implementing properties to allow for the
addition of new methods to the API.

This patch:
- Adds api_methods flag to CSSProperties.json5 which lists the
  names of functions implemented for this property.
- Modifies make_css_property_apis.py so that method names
  listed in CSSProperties.json5 are checked to be valid, and passed
  into CSSPropertyAPIFiles.h.tmpl.
- Wraps each function in the CSSPropertyAPIFiles.h template in an
  if statement, so that CSSPropertyAPI<Property>.h file only
  declares the functions that are implemented in the corresponding
  .cpp file.
==========

[aazzam, 2017-01-31 02:20:58]

Description was changed from

==========
Modified generator so that only implemented methods are in .h file.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. This patch modifies
make_css_property_apis.py and CSSPropertyAPIFiles.h.tmpl which
generate the .h files for implementing properties to allow for the
addition of new methods to the API.

This patch:
- Adds api_methods flag to CSSProperties.json5 which lists the
  names of functions implemented for this property.
- Modifies make_css_property_apis.py so that method names
  listed in CSSProperties.json5 are checked to be valid, and passed
  into CSSPropertyAPIFiles.h.tmpl.
- Wraps each function in the CSSPropertyAPIFiles.h template in an
  if statement, so that CSSPropertyAPI<Property>.h file only
  declares the functions that are implemented in the corresponding
  .cpp file.
==========

to

==========
Modified generator so that only implemented methods are in .h file.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. This patch modifies
make_css_property_apis.py and CSSPropertyAPIFiles.h.tmpl which
generate the .h files, to allow for the addition of new methods to
the API.

This patch:
- Adds api_methods flag to CSSProperties.json5 which lists the
  names of functions implemented for this property.
- Modifies make_css_property_apis.py so that method names
  listed in CSSProperties.json5 are checked to be valid, and passed
  into CSSPropertyAPIFiles.h.tmpl.
- Wraps each function in the CSSPropertyAPIFiles.h template in an
  if statement, so that CSSPropertyAPI<Property>.h file only
  declares the functions that are implemented in the corresponding
  .cpp file.
==========

[aazzam, 2017-01-31 02:22:55]

aazzam@google.com changed reviewers:
+ shend@chromium.org

[aazzam, 2017-01-31 02:22:56]

hey darren! please take a look at the CL description and let me know what you
think :)

[shend, 2017-01-31 02:47:42]

On 2017/01/31 at 02:22:56, aazzam wrote:
> hey darren! please take a look at the CL description and let me know what you
think :)

hey Anna, there's a few things which you might consider:
- The title is a bit confusing, but that's probably because I'm not familiar
with the project. I would do something like "Add api_methods flag to
CSSProperties.json5", which is the main "meat" of this patch right?
- I would avoid referring to Project ribbon without some context of what it
means (e.g. link to a bug).
- Explain the motivation. Why are you adding the api_methods flag? I usually do
something like "Currently, <status quo>. However <problem>. This patch <how this
fixes it>", but up to you.
- There's no need to mention everything you did in a list. I would only do it
for non-obvious changes. For example, you left parseSingleValue as a stub, so
you should mention that a future patch will actually put stuff in it. It's
basically the same principle as "when should you put comments in code".
- Think about what questions the reviewer might ask and answer them in the
description.

Sorry it's a bit vague, but I hope this helps. Let me know when you've edited it
and I can have a look :)

[aazzam, 2017-01-31 03:03:12]

Description was changed from

==========
Modified generator so that only implemented methods are in .h file.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. This patch modifies
make_css_property_apis.py and CSSPropertyAPIFiles.h.tmpl which
generate the .h files, to allow for the addition of new methods to
the API.

This patch:
- Adds api_methods flag to CSSProperties.json5 which lists the
  names of functions implemented for this property.
- Modifies make_css_property_apis.py so that method names
  listed in CSSProperties.json5 are checked to be valid, and passed
  into CSSPropertyAPIFiles.h.tmpl.
- Wraps each function in the CSSPropertyAPIFiles.h template in an
  if statement, so that CSSPropertyAPI<Property>.h file only
  declares the functions that are implemented in the corresponding
  .cpp file.
==========

to

==========
Added api_methods flag to CSSProperties.json5.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. 

This patch adds an api_methods=["method1", "method2"] flag to
CSSProperties.json5, which specifies which methods have been implemented in the
API implementation of that property. This is so that when API .h files are
generated, only methods implemented for that property are defined.

Before, when a method was added to the API, this method had to be implemented in
all API files - however, not every API should hold each method. In this new
design, to add a method to the API:
- Add the method name to the valid_methods array in make_css_property_apis.py
- Add the method to CSSPropertyAPIFiles.h.tmpl
- Add the method name to the api_methods list in CSSProperties.json5 for each
property where it is implemented.

BUG=668012
==========

[aazzam, 2017-01-31 03:04:00]

Description was changed from

==========
Added api_methods flag to CSSProperties.json5.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. 

This patch adds an api_methods=["method1", "method2"] flag to
CSSProperties.json5, which specifies which methods have been implemented in the
API implementation of that property. This is so that when API .h files are
generated, only methods implemented for that property are defined.

Before, when a method was added to the API, this method had to be implemented in
all API files - however, not every API should hold each method. In this new
design, to add a method to the API:
- Add the method name to the valid_methods array in make_css_property_apis.py
- Add the method to CSSPropertyAPIFiles.h.tmpl
- Add the method name to the api_methods list in CSSProperties.json5 for each
property where it is implemented.

BUG=668012
==========

to

==========
Added api_methods flag to CSSProperties.json5.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. 

This patch adds an api_methods=["method1", "method2"] flag to
CSSProperties.json5, which specifies which methods have been
implemented in the API implementation of that property. This is so
that when API .h files are generated, only methods implemented for
that property are defined.

Before, when a method was added to the API, this method had to be
implemented in all API files - however, not every API should hold
each method. In this new design, to add a method to the API:
- Add the method name to the valid_methods array in
  make_css_property_apis.py
- Add the method to CSSPropertyAPIFiles.h.tmpl
- Add the method name to the api_methods list in CSSProperties.json5
  for each property where it is implemented.

BUG=668012
==========

[aazzam, 2017-01-31 03:08:41]

On 2017/01/31 at 02:47:42, shend wrote:
> On 2017/01/31 at 02:22:56, aazzam wrote:
> > hey darren! please take a look at the CL description and let me know what
you think :)
> 
> hey Anna, there's a few things which you might consider:
> - The title is a bit confusing, but that's probably because I'm not familiar
with the project. I would do something like "Add api_methods flag to
CSSProperties.json5", which is the main "meat" of this patch right?
> - I would avoid referring to Project ribbon without some context of what it
means (e.g. link to a bug).
> - Explain the motivation. Why are you adding the api_methods flag? I usually
do something like "Currently, <status quo>. However <problem>. This patch <how
this fixes it>", but up to you.
> - There's no need to mention everything you did in a list. I would only do it
for non-obvious changes. For example, you left parseSingleValue as a stub, so
you should mention that a future patch will actually put stuff in it. It's
basically the same principle as "when should you put comments in code".
> - Think about what questions the reviewer might ask and answer them in the
description.
> 
> Sorry it's a bit vague, but I hope this helps. Let me know when you've edited
it and I can have a look :)

hey Darren! thanks so much for reviewing, I really appreciate the feedback! I've
updated the description - added a bug#, changed the title, and explained the
before/after state, what do you think of this?

[aazzam, 2017-01-31 03:10:03]

aazzam@google.com changed reviewers:
+ nainar@chromium.org

[aazzam, 2017-01-31 03:10:04]

please take a look naina! :) and ptal at CL description darren :)

[shend, 2017-01-31 03:20:08]

On 2017/01/31 at 03:10:04, aazzam wrote:
> please take a look naina! :) and ptal at CL description darren :)

lgtm CL description. Awesome work!

[nainar, 2017-01-31 03:21:19]

lgtm

https://codereview.chromium.org/2654403003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/make_css_property_apis.py (right):

https://codereview.chromium.org/2654403003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/make_css_property_apis.py:45: '. Methods
added to the API must also be added to list of valid methods.'
valid methods -> valid_methods

[aazzam, 2017-01-31 03:46:22]

aazzam@google.com changed reviewers:
+ sashab@chromium.org

[aazzam, 2017-01-31 03:46:23]

Fixed the variable names and comments, ptal sasha :)

[sashab, 2017-01-31 20:06:30]

This is one of the most amazing back-and-forth reviews I've ever seen :) Well
done both anna & darren, CL description looks amazing.

Please just add to CL description that this also adds "parseSingleValue" to the
list of api_methods for all existing properties with an API that implements
parseSingleValue.

Just had one question on valid_values that may remove the need to add that
python variable :) +ktyliu to help

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

https://codereview.chromium.org/2654403003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_css_property_apis.py:29:
valid_methods = ["parseSingleValue"]
Can we store this in CSSproperties.json5? There is a 'valid_values' field. Maybe
talk to ktyliu@ and see if this can be used to store the valid values in a list,
since that's pretty much all we would use it for.

Also rename to valid_api_methods (keep naming consistent)

https://codereview.chromium.org/2654403003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_css_property_apis.py:43: for method
in property['api_methods']:
for 'api_method' in (keep naming consistent)

https://codereview.chromium.org/2654403003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_css_property_apis.py:72:
'implemented_functions': implemented_functions,
Keep naming consistent; call this api_methods

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

https://codereview.chromium.org/2654403003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSProperties.json5:46: // List of methods
that are implemented in the implementation of CSSPropertyAPI
'implemented in the implementation of CSSpropertyAPI for...' -> 'implemented in
the CSSPropertyAPi for...'

https://codereview.chromium.org/2654403003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSProperties.json5:48: // functions are
defined in the .h file for this property.
, and also used in CSSPropertyAPIFiles.h.tmpl to generate declarations for only
the methods this property has implementations for.

[ktyliu, 2017-01-31 22:14:15]

ktyliu@chromium.org changed reviewers:
+ ktyliu@chromium.org

[ktyliu, 2017-01-31 22:14:16]

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

https://codereview.chromium.org/2654403003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_css_property_apis.py:29:
valid_methods = ["parseSingleValue"]
On 2017/01/31 at 20:06:30, sashab wrote:
> Can we store this in CSSproperties.json5? There is a 'valid_values' field.
Maybe talk to ktyliu@ and see if this can be used to store the valid values in a
list, since that's pretty much all we would use it for.
> 
> Also rename to valid_api_methods (keep naming consistent)

yes you can put this inside CSSProperties.json5 to get validation for free, and
also so that others can see the valid method names in the json5 directly without
referring to here

I've pointed out how to do it in CSSProperties.json5

https://codereview.chromium.org/2654403003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_css_property_apis.py:44: assert
method in valid_methods, 'Invalid method ' + method + \
this becomes unnecessary if you specify valid_values within CSSProperties.json5

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

https://codereview.chromium.org/2654403003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSProperties.json5:50: default: [],
just add this to get validation for free

  valid_values: ["parseSingleValue"],

[aazzam, 2017-01-31 22:38:00]

Description was changed from

==========
Added api_methods flag to CSSProperties.json5.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. 

This patch adds an api_methods=["method1", "method2"] flag to
CSSProperties.json5, which specifies which methods have been
implemented in the API implementation of that property. This is so
that when API .h files are generated, only methods implemented for
that property are defined.

Before, when a method was added to the API, this method had to be
implemented in all API files - however, not every API should hold
each method. In this new design, to add a method to the API:
- Add the method name to the valid_methods array in
  make_css_property_apis.py
- Add the method to CSSPropertyAPIFiles.h.tmpl
- Add the method name to the api_methods list in CSSProperties.json5
  for each property where it is implemented.

BUG=668012
==========

to

==========
Added api_methods flag to CSSProperties.json5.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. 

This patch adds an api_methods=["method1", "method2"] flag to
CSSProperties.json5, which specifies which methods have been
implemented in the API implementation of that property. This is so
that when API .h files are generated, only methods implemented for
that property are defined.

It also adds "parseSingleValue" to the list of api_methods for all
existing properties with an API that implements parseSingleValue.

Before, when a method was added to the API, this method had to be
implemented in all API files - however, not every API should hold
each method. In this new design, to add a method to the API:
- Add the method name to the valid_values array in CSSProperties.json5.
- Add the method to CSSPropertyAPIFiles.h.tmpl
- Add the method name to the api_methods list in CSSProperties.json5
  for each property where it is implemented.

BUG=668012
==========

[aazzam, 2017-01-31 22:38:38]

done (addressed comments and moved validation to CSSProperties.json5), ptal! :)

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

https://codereview.chromium.org/2654403003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_css_property_apis.py:29:
valid_methods = ["parseSingleValue"]
On 2017/01/31 at 20:06:30, sashab wrote:
> Can we store this in CSSproperties.json5? There is a 'valid_values' field.
Maybe talk to ktyliu@ and see if this can be used to store the valid values in a
list, since that's pretty much all we would use it for.
> 
> Also rename to valid_api_methods (keep naming consistent)

awesome idea!! much better than asking people to edit the python file :) had a
chat to kevin, he said that for now I can have a list of valid lists, i.e.
valid_values: [["parseSingleValue"]],
and he's already working on a CL which he will send to me shortly that allows
you to specify valid values in the list :)

https://codereview.chromium.org/2654403003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_css_property_apis.py:43: for method
in property['api_methods']:
On 2017/01/31 at 20:06:30, sashab wrote:
> for 'api_method' in (keep naming consistent)

I think i can get rid of this if i'm validating the data in CSSProperties.json5?

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

https://codereview.chromium.org/2654403003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSProperties.json5:46: // List of methods
that are implemented in the implementation of CSSPropertyAPI
On 2017/01/31 at 20:06:30, sashab wrote:
> 'implemented in the implementation of CSSpropertyAPI for...' -> 'implemented
in the CSSPropertyAPi for...'

fixed! :)

[sashab, 2017-01-31 22:50:58]

Nice :) LGTM, might need to rebase over kevins change

https://codereview.chromium.org/2654403003/diff/90005/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSProperties.json5 (right):

https://codereview.chromium.org/2654403003/diff/90005/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSProperties.json5:53: valid_values:
[["parseSingleValue"]],
Is this right? I think with https://codereview.chromium.org/2666173002 you won't
need the double braces :)

Maybe let ktyliu land first and rebase over his change :)

https://codereview.chromium.org/2654403003/diff/90005/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/properties/CSSPropertyAPI.h (right):

https://codereview.chromium.org/2654403003/diff/90005/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/properties/CSSPropertyAPI.h:35: //  
CSSPropertyDescriptor.
Ahh I'm so happy to see that we are maintaining this :) Awesome awesome

https://codereview.chromium.org/2654403003/diff/90005/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/properties/CSSPropertyAPI.h:44: NOTREACHED();
Add comment above this line like
// No code should reach here, since properties either have their own
implementations of this method or store nullptr in their descriptor.

[aazzam, 2017-01-31 22:55:44]

done, will wait for kevin's patch https://codereview.chromium.org/2666173002 to
land and will rebase! :)

https://codereview.chromium.org/2654403003/diff/90005/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSProperties.json5 (right):

https://codereview.chromium.org/2654403003/diff/90005/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSProperties.json5:53: valid_values:
[["parseSingleValue"]],
On 2017/01/31 at 22:50:58, sashab wrote:
> Is this right? I think with https://codereview.chromium.org/2666173002 you
won't need the double braces :)
> 
> Maybe let ktyliu land first and rebase over his change :)

Yeah fixed for kevins change :)

[ktyliu, 2017-01-31 23:02:46]

LGTM nice

https://codereview.chromium.org/2654403003/diff/90005/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSProperties.json5 (right):

https://codereview.chromium.org/2654403003/diff/90005/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSProperties.json5:53: valid_values:
[["parseSingleValue"]],
On 2017/01/31 at 22:55:43, aazzam wrote:
> On 2017/01/31 at 22:50:58, sashab wrote:
> > Is this right? I think with https://codereview.chromium.org/2666173002 you
won't need the double braces :)
> > 
> > Maybe let ktyliu land first and rebase over his change :)
> 
> Yeah fixed for kevins change :)

committing my change now
also feel free to use  [["parseSingleValue"]] for now if you want to commit
immediately, since you'll have to modify this and add more methods very soon :)

https://codereview.chromium.org/2654403003/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/build/scripts/make_css_property_apis.py (left):

https://codereview.chromium.org/2654403003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_css_property_apis.py:14: 
you probably still need double blank line to pass python style checking?

[aazzam, 2017-02-01 00:47:08]

The CQ bit was checked by aazzam@google.com

[aazzam, 2017-02-01 00:47:08]

The patchset sent to the CQ was uploaded after l-g-t-m from nainar@chromium.org,
shend@chromium.org, sashab@chromium.org, ktyliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2654403003/#ps160001
(title: "rebase")

[commit-bot: I haz the power, 2017-02-01 00:47:57]

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-01 02:43:52]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1485910028232640,
"parent_rev": "05d2f9d809d9082c4447bafe1e1806fdbbc9e3a1", "commit_rev":
"a252f0fa5f780f176eb3388df35ec67abaf8ca79"}

[commit-bot: I haz the power, 2017-02-01 02:44:31]

Description was changed from

==========
Added api_methods flag to CSSProperties.json5.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. 

This patch adds an api_methods=["method1", "method2"] flag to
CSSProperties.json5, which specifies which methods have been
implemented in the API implementation of that property. This is so
that when API .h files are generated, only methods implemented for
that property are defined.

It also adds "parseSingleValue" to the list of api_methods for all
existing properties with an API that implements parseSingleValue.

Before, when a method was added to the API, this method had to be
implemented in all API files - however, not every API should hold
each method. In this new design, to add a method to the API:
- Add the method name to the valid_values array in CSSProperties.json5.
- Add the method to CSSPropertyAPIFiles.h.tmpl
- Add the method name to the api_methods list in CSSProperties.json5
  for each property where it is implemented.

BUG=668012
==========

to

==========
Added api_methods flag to CSSProperties.json5.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. 

This patch adds an api_methods=["method1", "method2"] flag to
CSSProperties.json5, which specifies which methods have been
implemented in the API implementation of that property. This is so
that when API .h files are generated, only methods implemented for
that property are defined.

It also adds "parseSingleValue" to the list of api_methods for all
existing properties with an API that implements parseSingleValue.

Before, when a method was added to the API, this method had to be
implemented in all API files - however, not every API should hold
each method. In this new design, to add a method to the API:
- Add the method name to the valid_values array in CSSProperties.json5.
- Add the method to CSSPropertyAPIFiles.h.tmpl
- Add the method name to the api_methods list in CSSProperties.json5
  for each property where it is implemented.

BUG=668012

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

[commit-bot: I haz the power, 2017-02-01 02:44:32]

Committed patchset #10 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/a252f0fa5f780f176eb3388df35e...

[tsergeant, 2017-02-01 03:07:04]

A revert of this CL (patchset #10 id:160001) has been created in
https://codereview.chromium.org/2668093003/ by tsergeant@chromium.org.

The reason for reverting is: This CL causes compile to fail on GPU Mac Builder:

[1856/2060] CXX
obj/third_party/WebKit/Source/core/css/css_3/CSSPropertyAPIMargin.o
../../third_party/WebKit/Source/core/css/properties/CSSPropertyAPIMargin.cpp:13:39:
error: out-of-line definition of 'parseSingleValue' does not match any
declaration in 'blink::CSSPropertyAPIMargin'
const CSSValue* CSSPropertyAPIMargin::parseSingleValue(
                                      ^~~~~~~~~~~~~~~~
1 error generated.

See:
https://build.chromium.org/p/chromium.gpu/builders/GPU%20Mac%20Builder/builds....

[aazzam, 2017-02-01 04:16:27]

The CQ bit was checked by aazzam@google.com

[aazzam, 2017-02-01 04:16:27]

The patchset sent to the CQ was uploaded after l-g-t-m from nainar@chromium.org,
sashab@chromium.org, ktyliu@chromium.org, shend@chromium.org
Link to the patchset: https://codereview.chromium.org/2654403003/#ps200001
(title: "rebase")

[commit-bot: I haz the power, 2017-02-01 04:16:40]

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-01 05:54:56]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1485922587132710,
"parent_rev": "f00d3c2b8f462741503a9da7881129658ebcbc09", "commit_rev":
"d965a330d470c634f1010dd90bc20cfb1e81971f"}

[commit-bot: I haz the power, 2017-02-01 05:55:33]

Description was changed from

==========
Added api_methods flag to CSSProperties.json5.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. 

This patch adds an api_methods=["method1", "method2"] flag to
CSSProperties.json5, which specifies which methods have been
implemented in the API implementation of that property. This is so
that when API .h files are generated, only methods implemented for
that property are defined.

It also adds "parseSingleValue" to the list of api_methods for all
existing properties with an API that implements parseSingleValue.

Before, when a method was added to the API, this method had to be
implemented in all API files - however, not every API should hold
each method. In this new design, to add a method to the API:
- Add the method name to the valid_values array in CSSProperties.json5.
- Add the method to CSSPropertyAPIFiles.h.tmpl
- Add the method name to the api_methods list in CSSProperties.json5
  for each property where it is implemented.

BUG=668012

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

to

==========
Added api_methods flag to CSSProperties.json5.

A part of Project Ribbon, separating the parsing logic for CSS
properties from the parser into an API. 

This patch adds an api_methods=["method1", "method2"] flag to
CSSProperties.json5, which specifies which methods have been
implemented in the API implementation of that property. This is so
that when API .h files are generated, only methods implemented for
that property are defined.

It also adds "parseSingleValue" to the list of api_methods for all
existing properties with an API that implements parseSingleValue.

Before, when a method was added to the API, this method had to be
implemented in all API files - however, not every API should hold
each method. In this new design, to add a method to the API:
- Add the method name to the valid_values array in CSSProperties.json5.
- Add the method to CSSPropertyAPIFiles.h.tmpl
- Add the method name to the api_methods list in CSSProperties.json5
  for each property where it is implemented.

BUG=668012

Review-Url: https://codereview.chromium.org/2654403003
Cr-Original-Commit-Position: refs/heads/master@{#447427}
Committed:
https://chromium.googlesource.com/chromium/src/+/a252f0fa5f780f176eb3388df35e...
Review-Url: https://codereview.chromium.org/2654403003
Cr-Commit-Position: refs/heads/master@{#447459}
Committed:
https://chromium.googlesource.com/chromium/src/+/d965a330d470c634f1010dd90bc2...
==========

[commit-bot: I haz the power, 2017-02-01 05:55:35]

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