Clean up naming in ComputedStyle

Clean up naming in ComputedStyle

Clean up some naming in ComputedStyle, as pre-work to adding a fast-path
for inherited properties (which breaks up some of these functions even
further, and relies on accurate naming of members and is confusing if
they both use the term 'flags').

- Rename Non/InheritedFlags to Non/InheritedData (so as not to conflict
  with actual flags landing in the next patch)
- Renamed inheritedNotEqual() methods on ComputedStyle and
  SVGComputedStyle to be inheritedEqual() instead, as this will be less
  confusing later
- Changed operator== on ComputedStyle and SVGComputedStyle to use
  inheritedEqual() and nonInheritedEqual() internally, to reduce code
  repeats of fields
- Renamed a bunch of fields in ComputedStyle to begin with m_ and use
  proper camelcase from style guide
- Changed some ASSERTs to DCHECKs in ComputedStyle

BUG=622138
Committed: https://crrev.com/2274fc8876bc69737f0092c44a6b80add1c7736e
Cr-Commit-Position: refs/heads/master@{#403850}

[sashab, 2016-07-01 06:59:49]

sashab@chromium.org changed reviewers:
+ rune@opera.com

[sashab, 2016-07-01 06:59:51]

Sorry; the diff came out a little funny. But it's all mechanical changes with
find/replace except one thing which I commented. :)

https://codereview.chromium.org/2115803002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2115803002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:349: {
This method is identical (all comparisons are in the other methods) *except* for
the loadingCustomFonts() check in inheritedEqual(). Although this technically is
a behavior change, I doubt this actually changes anything -- two styles should
not be equal if their loadingCustomFonts() are not equal. So if anything this
fixes a bug we haven't found yet. :^)

https://codereview.chromium.org/2115803002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:420: bool
ComputedStyle::inheritedEqual(const ComputedStyle& other) const
It's less confusing if it's an inheritedEqual() method, since
!oldStyle->inheritedNotEqual(newStyle) is used later and reads better as
oldStyle->inheritedEqual(newStyle)

https://codereview.chromium.org/2115803002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.h (right):

https://codereview.chromium.org/2115803002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.h:160: // !START SYNC!: Keep
this in sync with the copy constructor in ComputedStyle.cpp and
implicitlyInherited() in StyleResolver.cpp
Unless commented specifically, everything in this patch is just renaming

https://codereview.chromium.org/2115803002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/SVGComputedStyle.cpp (right):

https://codereview.chromium.org/2115803002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/SVGComputedStyle.cpp:83: return
inheritedEqual(other)
This is identical to the comparisons before, I just moved them around. :)

[rune, 2016-07-01 08:45:41]

https://codereview.chromium.org/2115803002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2115803002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:349: {
On 2016/07/01 06:59:50, sashab wrote:
> This method is identical (all comparisons are in the other methods) *except*
for
> the loadingCustomFonts() check in inheritedEqual(). Although this technically
is
> a behavior change, I doubt this actually changes anything -- two styles should
> not be equal if their loadingCustomFonts() are not equal. So if anything this
> fixes a bug we haven't found yet. :^)

That will cause an assert in the baseComputedStyle optimization for animations
(the bots found it). We explicitly check for loadingCustomFonts() in addition to
operator== where we need to.

[sashab, 2016-07-05 02:01:14]

On 2016/07/01 at 08:45:41, rune wrote:
>
https://codereview.chromium.org/2115803002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):
> 
>
https://codereview.chromium.org/2115803002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/style/ComputedStyle.cpp:349: {
> On 2016/07/01 06:59:50, sashab wrote:
> > This method is identical (all comparisons are in the other methods) *except*
for
> > the loadingCustomFonts() check in inheritedEqual(). Although this
technically is
> > a behavior change, I doubt this actually changes anything -- two styles
should
> > not be equal if their loadingCustomFonts() are not equal. So if anything
this
> > fixes a bug we haven't found yet. :^)
> 
> That will cause an assert in the baseComputedStyle optimization for animations
(the bots found it). We explicitly check for loadingCustomFonts() in addition to
operator== where we need to.

In that case it should be in a separate loadingCustomFontsEqual() method. Moved
it to there and updated stylePropagationDiff() (the only callsite) to call it
explicitly.

[sashab, 2016-07-05 02:01:54]

sashab@chromium.org changed reviewers:
+ timloh@chromium.org

[sashab, 2016-07-05 02:01:55]

timloh - mechanical change, ptal

[sashab, 2016-07-05 02:38:13]

sashab@chromium.org changed reviewers:
+ dstockwell@chromium.org

[sashab, 2016-07-05 02:38:14]

Doug pls review, mechanical change

[dstockwell, 2016-07-05 23:33:20]

I think then general change is OK. But I'm not sure about renaming flags to
data, there's already a StyleRareInheritedData etc... and they are still
flags... You'll need someone else to sign off on that one :/

[sashab, 2016-07-05 23:36:55]

sashab@chromium.org changed reviewers:
+ esprehn@chromium.org

[sashab, 2016-07-05 23:36:56]

elliot wdyt? :)

[esprehn, 2016-07-06 02:13:25]

On 2016/07/05 at 23:36:56, sashab wrote:
> elliot wdyt? :)

If the external thing is called RareInheritedData it doesn't seem too crazy to
call the internal thing Data too. I don't think we need to worry too much about
the shed either way. This code has been ugly and unloved for so many years.
Progress is great!

LGTM

[sashab, 2016-07-06 04:25:48]

The CQ bit was checked by sashab@chromium.org

[commit-bot: I haz the power, 2016-07-06 04:25:53]

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, 2016-07-06 04:30:16]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-06 04:30:20]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-06 04:32:42]

Description was changed from

==========
Clean up naming in ComputedStyle

Clean up some naming in ComputedStyle, as pre-work to adding a fast-path
for inherited properties (which breaks up some of these functions even
further, and relies on accurate naming of members and is confusing if
they both use the term 'flags').

- Rename Non/InheritedFlags to Non/InheritedData (so as not to conflict
  with actual flags landing in the next patch)
- Renamed inheritedNotEqual() methods on ComputedStyle and
  SVGComputedStyle to be inheritedEqual() instead, as this will be less
  confusing later
- Changed operator== on ComputedStyle and SVGComputedStyle to use
  inheritedEqual() and nonInheritedEqual() internally, to reduce code
  repeats of fields
- Renamed a bunch of fields in ComputedStyle to begin with m_ and use
  proper camelcase from style guide
- Changed some ASSERTs to DCHECKs in ComputedStyle

BUG=622138
==========

to

==========
Clean up naming in ComputedStyle

Clean up some naming in ComputedStyle, as pre-work to adding a fast-path
for inherited properties (which breaks up some of these functions even
further, and relies on accurate naming of members and is confusing if
they both use the term 'flags').

- Rename Non/InheritedFlags to Non/InheritedData (so as not to conflict
  with actual flags landing in the next patch)
- Renamed inheritedNotEqual() methods on ComputedStyle and
  SVGComputedStyle to be inheritedEqual() instead, as this will be less
  confusing later
- Changed operator== on ComputedStyle and SVGComputedStyle to use
  inheritedEqual() and nonInheritedEqual() internally, to reduce code
  repeats of fields
- Renamed a bunch of fields in ComputedStyle to begin with m_ and use
  proper camelcase from style guide
- Changed some ASSERTs to DCHECKs in ComputedStyle

BUG=622138

Committed: https://crrev.com/2274fc8876bc69737f0092c44a6b80add1c7736e
Cr-Commit-Position: refs/heads/master@{#403850}
==========

[commit-bot: I haz the power, 2016-07-06 04:32:43]

Patchset 3 (id:??) landed as
https://crrev.com/2274fc8876bc69737f0092c44a6b80add1c7736e
Cr-Commit-Position: refs/heads/master@{#403850}