Generate inherited nonproperties insideLink and hasSimpleUnderline.

Generate inherited nonproperties insideLink and hasSimpleUnderline.

This patch moves insideLink and hasSimpleUnderline to be generated in
ComputedStyleBase. Both are inherited nonproperties. To generate them,
this patch allows nonproperties to be inherited and have arbitrary
field_templates.

Since these are the last two remaining inherited fields stored directly
in ComputedStyle, this patch also removes the InheritedData struct from
ComputedStyle.

Diff of generated code:
https://gist.github.com/darrnshn/81e60b88825691200d681a1927040167/revisions

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

[shend, 2017-03-22 01:49:58]

Description was changed from

==========
Generate inherited nonproperties insideLink and hasSimpleUnderline.

This patch moves insideLink and hasSimpleUnderline to be generated in
ComputedStyleBase. Both are inherited nonproperties. To generate them,
this patch allows nonproperties to be inherited and have arbitrary
field_templates.

Since these are the last two remaining inherited fields stored directly
in ComputedStyle, this patch also removes the InheritedData struct from
ComputedStyle.

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

to

==========
Generate inherited nonproperties insideLink and hasSimpleUnderline.

This patch moves insideLink and hasSimpleUnderline to be generated in
ComputedStyleBase. Both are inherited nonproperties. To generate them,
this patch allows nonproperties to be inherited and have arbitrary
field_templates.

Since these are the last two remaining inherited fields stored directly
in ComputedStyle, this patch also removes the InheritedData struct from
ComputedStyle.

Diff of generated code:
https://gist.github.com/darrnshn/81e60b88825691200d681a1927040167/revisions

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

[shend, 2017-03-22 01:59:59]

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

[shend, 2017-03-22 01:59:59]

Hi Alan, PTAL

[alancutter (OOO until 2018), 2017-03-22 06:25:19]

lgtm

https://codereview.chromium.org/2759343002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl
(left):

https://codereview.chromium.org/2759343002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl:19:
{% for field in fields if field.is_property and field.is_inherited %}
Good to see is_property checks being removed. Would it make sense to add a TODO
to remove is_property from Field entirely?

https://codereview.chromium.org/2759343002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl
(right):

https://codereview.chromium.org/2759343002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl:24:
void ComputedStyleBase::copyNonInheritedFromCached(
Unrelated to this patch: This method might be simpler if it were just
copyNonInherited() and didn't care about whether the parameter is a cache or
not. The cache logic should exist at the call site and then this may be able to
be reused in the copy constructor etc. A similar thing may be done with
inheritFrom() (renamed as copyInherited() for consistency).

Just random ideas, feel free to disregard if it doesn't work out.

[shend, 2017-03-22 22:01:37]

On 2017/03/22 at 06:25:19, alancutter wrote:
>
https://codereview.chromium.org/2759343002/diff/40001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl:24:
void ComputedStyleBase::copyNonInheritedFromCached(
> Unrelated to this patch: This method might be simpler if it were just
copyNonInherited() and didn't care about whether the parameter is a cache or
not. The cache logic should exist at the call site and then this may be able to
be reused in the copy constructor etc. A similar thing may be done with
inheritFrom() (renamed as copyInherited() for consistency).

I like the idea of reusing the copy constructor. What is a cache?

[shend, 2017-03-22 22:02:00]

https://codereview.chromium.org/2759343002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl
(left):

https://codereview.chromium.org/2759343002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl:19:
{% for field in fields if field.is_property and field.is_inherited %}
On 2017/03/22 at 06:25:19, alancutter wrote:
> Good to see is_property checks being removed. Would it make sense to add a
TODO to remove is_property from Field entirely?

It is unlikely we'll be able to remove is_property entirely, since there are
still behaviour differences between properties and nonproperties (e.g.
copyNonInheritedFromCached).

[alancutter (OOO until 2018), 2017-03-23 02:04:33]

On 2017/03/22 at 22:02:00, shend wrote:
>
https://codereview.chromium.org/2759343002/diff/40001/third_party/WebKit/Sour...
> File
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl
(left):
> 
>
https://codereview.chromium.org/2759343002/diff/40001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl:19:
{% for field in fields if field.is_property and field.is_inherited %}
> On 2017/03/22 at 06:25:19, alancutter wrote:
> > Good to see is_property checks being removed. Would it make sense to add a
TODO to remove is_property from Field entirely?
> 
> It is unlikely we'll be able to remove is_property entirely, since there are
still behaviour differences between properties and nonproperties (e.g.
copyNonInheritedFromCached).

Can these differences be captured more directly with additional Field members
instead of referring to property/nonproperty?

[shend, 2017-03-23 02:21:50]

On 2017/03/23 at 02:04:33, alancutter wrote:
> On 2017/03/22 at 22:02:00, shend wrote:
> >
https://codereview.chromium.org/2759343002/diff/40001/third_party/WebKit/Sour...
> > File
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl
(left):
> > 
> >
https://codereview.chromium.org/2759343002/diff/40001/third_party/WebKit/Sour...
> >
third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl:19:
{% for field in fields if field.is_property and field.is_inherited %}
> > On 2017/03/22 at 06:25:19, alancutter wrote:
> > > Good to see is_property checks being removed. Would it make sense to add a
TODO to remove is_property from Field entirely?
> > 
> > It is unlikely we'll be able to remove is_property entirely, since there are
still behaviour differences between properties and nonproperties (e.g.
copyNonInheritedFromCached).
> 
> Can these differences be captured more directly with additional Field members
instead of referring to property/nonproperty?

I believe they can. So something like "is_flag" outta do it.

[shend, 2017-03-23 03:00:04]

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

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

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-03-23 03:14:48]

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

[commit-bot: I haz the power, 2017-03-23 03:14:49]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[shend, 2017-03-23 21:55:30]

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

[commit-bot: I haz the power, 2017-03-23 21:56:02]

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-03-23 22:16:30]

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

[commit-bot: I haz the power, 2017-03-23 22:16:31]

Dry run: This issue passed the CQ dry run.

[shend, 2017-03-24 00:20:00]

The CQ bit was checked by shend@chromium.org

[commit-bot: I haz the power, 2017-03-24 00:20:29]

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-03-24 00:31:08]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490314800269110,
"parent_rev": "df86c4558553e33a10eaa267d7bc1eea16c8905f", "commit_rev":
"8a7ed1bc6c96fa8ce4da4c7d80eb064a74265aff"}

[commit-bot: I haz the power, 2017-03-24 00:35:43]

Description was changed from

==========
Generate inherited nonproperties insideLink and hasSimpleUnderline.

This patch moves insideLink and hasSimpleUnderline to be generated in
ComputedStyleBase. Both are inherited nonproperties. To generate them,
this patch allows nonproperties to be inherited and have arbitrary
field_templates.

Since these are the last two remaining inherited fields stored directly
in ComputedStyle, this patch also removes the InheritedData struct from
ComputedStyle.

Diff of generated code:
https://gist.github.com/darrnshn/81e60b88825691200d681a1927040167/revisions

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

to

==========
Generate inherited nonproperties insideLink and hasSimpleUnderline.

This patch moves insideLink and hasSimpleUnderline to be generated in
ComputedStyleBase. Both are inherited nonproperties. To generate them,
this patch allows nonproperties to be inherited and have arbitrary
field_templates.

Since these are the last two remaining inherited fields stored directly
in ComputedStyle, this patch also removes the InheritedData struct from
ComputedStyle.

Diff of generated code:
https://gist.github.com/darrnshn/81e60b88825691200d681a1927040167/revisions

BUG=628043

Review-Url: https://codereview.chromium.org/2759343002
Cr-Commit-Position: refs/heads/master@{#459301}
Committed:
https://chromium.googlesource.com/chromium/src/+/8a7ed1bc6c96fa8ce4da4c7d80eb...
==========

[commit-bot: I haz the power, 2017-03-24 00:35:44]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/8a7ed1bc6c96fa8ce4da4c7d80eb...