Adding operator== for ChainedTimingFunction

Adding operator== for ChainedTimingFunction.

While this operator is not needed for CSS, it is very useful for testing.

DEPENDS=https://codereview.chromium.org/52923003/ - Fixing reflectivity of operator== for timing functions.
DEPENDS=https://codereview.chromium.org/39223002/ - TimingFunction test helper.

[mithro-old, 2013-11-04 03:24:03]

Hi Steve,

In reworking https://codereview.chromium.org/54743002/ to have the reverse
functions external to the TimingFunction object I relised I had confused two
changes.

This CL makes ::create understand presets so that operator== actually works
correctly. It also addes an operator== for ChainedTimingFunction.

Tim

[mithro-old, 2013-11-04 03:35:02]

Replying to your review to this code at https://codereview.chromium.org/54743002
initially here.

>
https://codereview.chromium.org/54743002/diff/1/Source/core/platform/animatio...
> Source/core/platform/animation/TimingFunction.h:131: 
> This introduces a change in behavior for CSS. The operator==() methods are
> currently used for comparing render styles to determine when things change.
> Currently, 'animation-timing-function: cubic-bezier(0.25, 0.1, 0.25, 1.0)' and
> 'animation-timing-function: ease'  are considered to be different values, but
> with your change, they will be considered equal. Are you sure this is OK?

Didn't realise operator== was used by CSS.

Guess there are two conflicting needs for ==.

>
https://codereview.chromium.org/54743002/diff/1/Source/core/platform/animatio...
> Source/core/platform/animation/TimingFunction.h:179: 
> Why don't you test for 'this == &other' here, like in the other classes?

Comparing 4 doubles is pretty easy compared to some of the other operator== so
didn't think it was needed.

Tim

[Steve Block, 2013-11-04 04:47:47]

As I mentioned, I think we need to check that changing these operator==()
methods is OK for their uses by CSS.

Also, can you upload or at least explain your new code that will use them, so I
can understand what kind of 'equality' you need?

[mithro-old, 2013-11-04 04:55:35]

>
https://codereview.chromium.org/54743002/diff/1/Source/core/platform/animatio...
> > Source/core/platform/animation/TimingFunction.h:131: 
> > This introduces a change in behavior for CSS. The operator==() methods are
> > currently used for comparing render styles to determine when things change.
> > Currently, 'animation-timing-function: cubic-bezier(0.25, 0.1, 0.25, 1.0)'
and
> > 'animation-timing-function: ease'  are considered to be different values,
but
> > with your change, they will be considered equal. Are you sure this is OK?
> 
> Didn't realise operator== was used by CSS.
> 
> Guess there are two conflicting needs for ==.

I had a read of the specification and looked at what Firefox does and what we
currently do.
 * Firefox always converts the getComputedStyle to a cubic-bezier, including
linear!
 * Chrome seems to keep the getComputedStyle in the original form.

From https://bugs.webkit.org/show_bug.cgi?id=105442
   [16:17] <smfr> darktears: at some point css-wg talked about returning the
"shorthand" functions
   [16:17] <smfr> darktears: does the spec say anything about this?
   [16:17] <darktears> smfr: not a word 
   [16:17] <darktears> smfr: the serialization is not really documented, at
least I made them consistent 
   [16:18] <darktears>  /s/made/make
   [16:18] <smfr> TabAtkins: any opinion on ^^
   [16:20] <TabAtkins> smfr: Serialization is undefined, obviously.  I'm fine
with always returning a cubic-bezier().

[mithro-old, 2013-11-04 05:05:04]

On 2013/11/04 04:47:47, Steve Block wrote:
> As I mentioned, I think we need to check that changing these operator==()
> methods is OK for their uses by CSS.
> 
> Also, can you upload or at least explain your new code that will use them, so
I
> can understand what kind of 'equality' you need?

I was only using the equality during reverse functionality test code and in the
compositor conversion test code. (The tests uses the operator==). It seemed
strange that things which seemed equal where not.

It appears I ended up solving this problem three times;
 * reverse code specifically deals with subtypes
 * ::create understanding the presets
 * operator== change

Give me a moment and I'll refactor this a little bit and test if it is still
needed.

[mithro-old, 2013-11-04 12:16:35]

Hi Steve,

This CL now only adds operator== to ChainedTimingFunction. While this operator
is not needed for CSS, it is very useful for testing. 

It depends on:
 * https://codereview.chromium.org/52923003/ - Fixing reflectivity of operator==
for timing functions. 
 * https://codereview.chromium.org/39223002/ - TimingFunction test helper.

Tim

[mithro-old, 2013-11-06 01:10:32]

On 2013/11/04 12:16:35, mithro wrote:
> Hi Steve,
> 
> This CL now only adds operator== to ChainedTimingFunction. While this operator
> is not needed for CSS, it is very useful for testing. 
> 
> It depends on:
>  * https://codereview.chromium.org/52923003/ - Fixing reflectivity of
operator==
> for timing functions. 
>  * https://codereview.chromium.org/39223002/ - TimingFunction test helper.
> 
> Tim

As discussed the operator== has been removed from the production code. 

I'll merge this TimingFunction operator== stuff and move into the test helper.

Closing.

Tim