Avoid reparsing keyframe selectors

At the moment, keyframe selectors are parsed by CSSParser and then at runtime by StyleKeyframe class.

To avoid reparsing, StyleKeyframe could store array of parsed keyframe selector values.

BUG=233107
TEST= covered by LayoutTests/animations/

[apavlov, 2013-04-22 13:30:23]

Doesn't https://codereview.chromium.org/13871007/ cover this entirely?

One other thing that needs some treatment is this: "To determine the set of
keyframes, all of the values in the selectors are _sorted in increasing order_
by time." (http://www.w3.org/TR/css3-animations/#keyframes) Instead, currently
we use the order of keys that is found in the source code.

https://codereview.chromium.org/13943004/diff/1/Source/core/css/WebKitCSSKeyf...
File Source/core/css/WebKitCSSKeyframeRule.cpp (right):

https://codereview.chromium.org/13943004/diff/1/Source/core/css/WebKitCSSKeyf...
Source/core/css/WebKitCSSKeyframeRule.cpp:65:
keyString.append(String::number(key*100));
Whitespace around '*'

[shalamov, 2013-04-23 10:46:39]

> Doesn't https://codereview.chromium.org/13871007/ cover this entirely?
> 

It covers my fix partially. I could keep changes in
Source/core/css/WebKitCSSKeyframeRule.* files and in style resolver.

I will upload new patch and change subject of the bug / review request.

[apavlov, 2013-04-29 09:55:45]

On 2013/04/23 10:46:39, shalamov wrote:
> > Doesn't https://codereview.chromium.org/13871007/ cover this entirely?
> > 
> 
> It covers my fix partially. I could keep changes in
> Source/core/css/WebKitCSSKeyframeRule.* files and in style resolver.
> 
> I will upload new patch and change subject of the bug / review request.

Good. Please update the description to explain what additional value this patch
adds (since keyframe rules with bad selectors are already ignored).

[apavlov, 2013-06-13 12:13:55]

@shalamov: ping? Is this change still of current interest?

[apavlov, 2013-06-18 09:59:50]

LGTM with nits

https://codereview.chromium.org/13943004/diff/10001/Source/core/css/CSSKeyfra...
File Source/core/css/CSSKeyframeRule.cpp (right):

https://codereview.chromium.org/13943004/diff/10001/Source/core/css/CSSKeyfra...
Source/core/css/CSSKeyframeRule.cpp:65:
keyString.append(String::number(key*100));
whitespace around '*'

https://codereview.chromium.org/13943004/diff/10001/Source/core/css/CSSKeyfra...
Source/core/css/CSSKeyframeRule.cpp:89: Vector<float>
StyleKeyframe::parseKeyString(const String& s)
Vector's copy ctor performs TypeOperations::uninitializedCopy(), so I would tend
to use an out parameter here, as it used to.

https://codereview.chromium.org/13943004/diff/10001/Source/core/css/CSSParser...
File Source/core/css/CSSParser.cpp (right):

https://codereview.chromium.org/13943004/diff/10001/Source/core/css/CSSParser...
Source/core/css/CSSParser.cpp:11355: for (unsigned i = 0; i < keyCount; ++i) {
IIRC, Vector::size() returns a size_t, so while we are here, it's a good idea to
retain the exact type.

[apavlov, 2013-06-26 10:45:39]

https://codereview.chromium.org/13943004/diff/18001/Source/core/css/CSSKeyfra...
File Source/core/css/CSSKeyframeRule.h (right):

https://codereview.chromium.org/13943004/diff/18001/Source/core/css/CSSKeyfra...
Source/core/css/CSSKeyframeRule.h:52: Vector<float> getKeys() const { return
m_keys; }
This should also be
const Vector<float>& getKeys() const (returning a reference)
to avoid Vector copies.

https://codereview.chromium.org/13943004/diff/18001/Source/core/css/CSSParser...
File Source/core/css/CSSParser.cpp (right):

https://codereview.chromium.org/13943004/diff/18001/Source/core/css/CSSParser...
Source/core/css/CSSParser.cpp:11359: if (keys->valueAt(i)->unit ==
CSSPrimitiveValue::CSS_UNKNOWN) {
Due to the recent fixes to CSSGrammar.y.in, this kind of errors is now handled
on the parser level, so the check should be removed.

https://codereview.chromium.org/13943004/diff/18001/Source/core/css/resolver/...
File Source/core/css/resolver/StyleResolver.cpp (right):

https://codereview.chromium.org/13943004/diff/18001/Source/core/css/resolver/...
Source/core/css/resolver/StyleResolver.cpp:1250: Vector<float> keys =
keyframe->getKeys();
Vector<float>& keys = keyframe->getKeys();
to follow the getKeys() signature fix.

[apavlov, 2013-07-04 22:02:51]

https://codereview.chromium.org/13943004/diff/28001/Source/core/css/CSSKeyfra...
File Source/core/css/CSSKeyframeRule.cpp (left):

https://codereview.chromium.org/13943004/diff/28001/Source/core/css/CSSKeyfra...
Source/core/css/CSSKeyframeRule.cpp:59: keys.clear();
Hmm, just noticed. Why did this get removed? When parsing a new key string, key
values will get appended to the existing Vector of keys, right?

https://codereview.chromium.org/13943004/diff/28001/Source/core/css/CSSKeyfra...
File Source/core/css/CSSKeyframeRule.h (right):

https://codereview.chromium.org/13943004/diff/28001/Source/core/css/CSSKeyfra...
Source/core/css/CSSKeyframeRule.h:52: const Vector<float>& getKeys() const {
return m_keys; }
In WebKit/Blink, getFoo(...) usually fills an output parameter. Normally,
getters do not have the "get" prefix. You can fix this in a subsequent patch.