Web Animations: Add CSSAnimatableValueFactory API (partial implementation)

Web Animations: Add CSSAnimatableValueFactory API (partial implementation)

This introduces the API for constructing AnimatableValues out of
RenderStyles and CSS Values. Basic support for Lengths and
AnimatableNumbers implemented.

Further coverage of all CSS properties and animatable CSS value types
to be accomplished by generated code in future.

This patch is part of replacing CSSPropertyAnimation with
AnimatableValues for Web Animations.
http://dev.w3.org/fxtf/web-animations/

BUG=258896
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=155408

[dstockwell, 2013-07-18 07:43:12]

Update description to mention any plans around generating the factory.

Reference the code that this intends to replace (CSSPropertyAnimation).

https://codereview.chromium.org/19744002/diff/1/Source/core/css/resolver/CSSA...
File Source/core/css/resolver/CSSAnimatableValueFactory.cpp (right):

https://codereview.chromium.org/19744002/diff/1/Source/core/css/resolver/CSSA...
Source/core/css/resolver/CSSAnimatableValueFactory.cpp:40: namespace WebCore {
Fixme, generate this file?

https://codereview.chromium.org/19744002/diff/1/Source/core/css/resolver/CSSA...
File Source/core/css/resolver/CSSAnimatableValueFactory.h (left):

https://codereview.chromium.org/19744002/diff/1/Source/core/css/resolver/CSSA...
Source/core/css/resolver/CSSAnimatableValueFactory.h:34: #include
"bindings/v8/ScriptWrappable.h"
Upload with --no-find-copies

[alancutter (OOO until 2018), 2013-07-24 07:09:23]

On 2013/07/18 07:43:12, dstockwell wrote:
> Update description to mention any plans around generating the factory.
> 
> Reference the code that this intends to replace (CSSPropertyAnimation).
> 

Done.

>
https://codereview.chromium.org/19744002/diff/1/Source/core/css/resolver/CSSA...
> File Source/core/css/resolver/CSSAnimatableValueFactory.cpp (right):
> 
>
https://codereview.chromium.org/19744002/diff/1/Source/core/css/resolver/CSSA...
> Source/core/css/resolver/CSSAnimatableValueFactory.cpp:40: namespace WebCore {
> Fixme, generate this file?
> 

Added fixme.

>
https://codereview.chromium.org/19744002/diff/1/Source/core/css/resolver/CSSA...
> File Source/core/css/resolver/CSSAnimatableValueFactory.h (left):
> 
>
https://codereview.chromium.org/19744002/diff/1/Source/core/css/resolver/CSSA...
> Source/core/css/resolver/CSSAnimatableValueFactory.h:34: #include
> "bindings/v8/ScriptWrappable.h"
> Upload with --no-find-copies

Done.

[alancutter (OOO until 2018), 2013-07-24 23:44:24]

ping

[dstockwell, 2013-07-30 03:35:04]

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
File Source/core/css/resolver/CSSAnimatableValueFactory.cpp (right):

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.cpp:42: // FIXME: Handle
remaining animatable properties:
Where does this list come from? It should be at least everything in
CSSPropertyAnimation

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.cpp:98: if
(AnimatableNumber::canCreateFrom(value))
otherwise, shouldn't we create an AnimatableUnknown?

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.cpp:159:
ASSERT_NOT_REACHED();
doesn't default: with return mean this can't ever be reached?

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.cpp:182:
ASSERT_WITH_MESSAGE(false, "Unable to convert platform CalculationValue to
AnimatableValue, not yet implemented!");
I was surprised that we didn't already have this problem. How hard is it for
AnimatableNumber to store the platform calc, and perform this conversion in
toCSSValue?

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
File Source/core/css/resolver/CSSAnimatableValueFactory.h (right):

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.h:46: static
PassRefPtr<AnimatableValue> createFromCSSValue(CSSValue*, CSSPropertyID);
As discussed we probably won't need this variant, we can probably drop it and
add it later if really necessary.

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.h:49: static
PassRefPtr<AnimatableValue> createFromLength(const Length&, RenderStyle*);
Does this need to be public?

[alancutter (OOO until 2018), 2013-07-30 08:31:01]

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
File Source/core/css/resolver/CSSAnimatableValueFactory.cpp (right):

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.cpp:42: // FIXME: Handle
remaining animatable properties:
On 2013/07/30 03:35:04, dstockwell wrote:
> Where does this list come from? It should be at least everything in
> CSSPropertyAnimation

Updated list using CSSPropertyAnimation.

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.cpp:98: if
(AnimatableNumber::canCreateFrom(value))
On 2013/07/30 03:35:04, dstockwell wrote:
> otherwise, shouldn't we create an AnimatableUnknown?

Function removed from patch.

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.cpp:159:
ASSERT_NOT_REACHED();
On 2013/07/30 03:35:04, dstockwell wrote:
> doesn't default: with return mean this can't ever be reached?

Done.

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.cpp:182:
ASSERT_WITH_MESSAGE(false, "Unable to convert platform CalculationValue to
AnimatableValue, not yet implemented!");
On 2013/07/30 03:35:04, dstockwell wrote:
> I was surprised that we didn't already have this problem. How hard is it for
> AnimatableNumber to store the platform calc, and perform this conversion in
> toCSSValue?

It's not particularly difficult though I would prefer to keep it as a separate
patch.

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
File Source/core/css/resolver/CSSAnimatableValueFactory.h (right):

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.h:46: static
PassRefPtr<AnimatableValue> createFromCSSValue(CSSValue*, CSSPropertyID);
On 2013/07/30 03:35:04, dstockwell wrote:
> As discussed we probably won't need this variant, we can probably drop it and
> add it later if really necessary.

Done.

https://codereview.chromium.org/19744002/diff/10001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.h:49: static
PassRefPtr<AnimatableValue> createFromLength(const Length&, RenderStyle*);
On 2013/07/30 03:35:04, dstockwell wrote:
> Does this need to be public?

We don't really need it at this stage, it can be exposed later if needed. For
now I removed it from the header file.

[dstockwell, 2013-07-31 02:17:09]

lgtm

https://codereview.chromium.org/19744002/diff/20001/Source/core/css/resolver/...
File Source/core/css/resolver/CSSAnimatableValueFactory.h (right):

https://codereview.chromium.org/19744002/diff/20001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.h:46: static
PassRefPtr<AnimatableValue> create(RenderStyle*, CSSPropertyID);
const RenderStyle?

[alancutter (OOO until 2018), 2013-08-02 03:24:10]

https://codereview.chromium.org/19744002/diff/20001/Source/core/css/resolver/...
File Source/core/css/resolver/CSSAnimatableValueFactory.h (right):

https://codereview.chromium.org/19744002/diff/20001/Source/core/css/resolver/...
Source/core/css/resolver/CSSAnimatableValueFactory.h:46: static
PassRefPtr<AnimatableValue> create(RenderStyle*, CSSPropertyID);
On 2013/07/31 02:17:09, dstockwell wrote:
> const RenderStyle?

Done.

[pdr., 2013-08-02 03:36:23]

On 2013/08/02 03:24:10, alancutter wrote:
>
https://codereview.chromium.org/19744002/diff/20001/Source/core/css/resolver/...
> File Source/core/css/resolver/CSSAnimatableValueFactory.h (right):
> 
>
https://codereview.chromium.org/19744002/diff/20001/Source/core/css/resolver/...
> Source/core/css/resolver/CSSAnimatableValueFactory.h:46: static
> PassRefPtr<AnimatableValue> create(RenderStyle*, CSSPropertyID);
> On 2013/07/31 02:17:09, dstockwell wrote:
> > const RenderStyle?
> 
> Done.

LGTM

[commit-bot: I haz the power, 2013-08-02 03:37:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alancutter@chromium.org/19744002/25001

[commit-bot: I haz the power, 2013-08-02 06:12:35]

Change committed as 155408