Title: [228702] trunk
Revision
228702
Author
grao...@webkit.org
Date
2018-02-19 11:06:25 -0800 (Mon, 19 Feb 2018)

Log Message

[Web Animations] Store all parsed keyframe input information in a single structure
https://bugs.webkit.org/show_bug.cgi?id=182903

Reviewed by Dean Jackson.

LayoutTests/imported/w3c:

Update test expectations with progressions resulting from returning the style values as provided
by the keyframe input when calling getKeyframes().

* web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt:
* web-platform-tests/web-animations/interfaces/KeyframeEffect/constructor-expected.txt:
* web-platform-tests/web-animations/interfaces/KeyframeEffect/setKeyframes-expected.txt:

Source/WebCore:

When parsing keyframe input provided through the JS API, we used to create several data structures.
During parsing we would create a Vector<ProcessedKeyframe> where we would store the validated values
for "offset", "easing" and "composite" as well as CSS properties and CSS values as strings.

Then we would create a KeyframeList, a class that pre-dates the work on Web Animations and is used
for hardware animations, with RenderStyle objects that are used for CSS property blending at runtime.
Once the KeyframeList was created, the Vector<ProcessedKeyframe> was discarded.

Since KeyframeList did not know about nullable offsets, timing functions and composite operations, and
because we do not with to modify a legacy class that we will eventually remove once all the Web Animations
work is complete, we also stored the parsed offsets as m_offsets, the timing functions as m_timingFunctions
and the composite operations as m_compositeOperations.

In this patch we rename the ProcessedKeyframe structure used temporarily during parsing to ParsedKeyframe and
store both the input and processed data related to a given keyframe in that single structure which we keep
around as m_parsedKeyframes when we finished processing the keyframes input. This update ParsedKeyframe structure
allows to keep around the original nullable offsets, the original CSS properties and CSS values as strings as
a HashMap<CSSPropertyID, String>, as well as the CSS properties and CSS values as CSSValue objects using a
MutableStyleProperties object.

This has the benefit of reducing the number of members, but also pave the way for supporting read-write targets
where we will be able to decouple parsing keyframes and creating a KeyframeList, which requires a valid target
to create RenderStyle objects used for blending, since the original parsing-time information is now stored.

Finally, this allowed getKeyframes() to be more compliant by returning the CSS values as originally provided in
the keyframe input with shorthand properties when provided, rather than the long-hands we used to read back
through RenderStyle objects.

The generated KeyframeList is now stored as m_blendingKeyframes and is only used for the purpose of interfacing
with hardware animations and CSS property blending.

While ProcessedKeyframe was copyable due to holding only simple types, ParsedKeyframe is not since it uses a Ref
to hold the MutableStyleProperties. This uncovered some cases where we copied ProcessedKeyframe objects, we now
ensure that the ParsedKeyframe objects are moved instead, which was the correct thing to do all along.

* animation/KeyframeEffectReadOnly.cpp:
(WebCore::computeMissingKeyframeOffsets): While we used to store std::optional<double> for the computed offset,
we now store a simple double, which makes more sense since the computed offset is eventually a fully resolved
value after calling computeMissingKeyframeOffsets(). So we now compute the final computed offset without resorting
to intermediate nullable computed offsets.
(WebCore::processIterableKeyframes):
(WebCore::processPropertyIndexedKeyframes):
(WebCore::KeyframeEffectReadOnly::KeyframeEffectReadOnly):
(WebCore::KeyframeEffectReadOnly::copyPropertiesFromSource):
(WebCore::KeyframeEffectReadOnly::getKeyframes):
(WebCore::KeyframeEffectReadOnly::processKeyframes):
(WebCore::KeyframeEffectReadOnly::computeStackingContextImpact):
(WebCore::KeyframeEffectReadOnly::shouldRunAccelerated):
(WebCore::KeyframeEffectReadOnly::getAnimatedStyle):
(WebCore::KeyframeEffectReadOnly::setAnimatedPropertiesInStyle):
(WebCore::KeyframeEffectReadOnly::startOrStopAccelerated):
* animation/KeyframeEffectReadOnly.h:
(WebCore::KeyframeEffectReadOnly::ParsedKeyframe::ParsedKeyframe):

Modified Paths

Diff

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (228701 => 228702)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2018-02-19 19:03:37 UTC (rev 228701)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2018-02-19 19:06:25 UTC (rev 228702)
@@ -1,5 +1,19 @@
 2018-02-17  Antoine Quint  <grao...@apple.com>
 
+        [Web Animations] Store all parsed keyframe input information in a single structure
+        https://bugs.webkit.org/show_bug.cgi?id=182903
+
+        Reviewed by Dean Jackson.
+
+        Update test expectations with progressions resulting from returning the style values as provided
+        by the keyframe input when calling getKeyframes(). 
+
+        * web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt:
+        * web-platform-tests/web-animations/interfaces/KeyframeEffect/constructor-expected.txt:
+        * web-platform-tests/web-animations/interfaces/KeyframeEffect/setKeyframes-expected.txt:
+
+2018-02-17  Antoine Quint  <grao...@apple.com>
+
         [Web Animations] Accept null composite modes in keyframes
         https://bugs.webkit.org/show_bug.cgi?id=182902
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt (228701 => 228702)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt	2018-02-19 19:03:37 UTC (rev 228701)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt	2018-02-19 19:06:25 UTC (rev 228702)
@@ -8,14 +8,14 @@
 PASS Element.animate() accepts empty keyframe lists (input: null) 
 PASS Element.animate() accepts empty keyframe lists (input: undefined) 
 PASS Element.animate() accepts a one property two value property-indexed keyframes specification 
-FAIL Element.animate() accepts a one shorthand property two value property-indexed keyframes specification assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
-FAIL Element.animate() accepts a two property (one shorthand and one of its longhand components) two value property-indexed keyframes specification assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,marginTop,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS Element.animate() accepts a one shorthand property two value property-indexed keyframes specification 
+PASS Element.animate() accepts a two property (one shorthand and one of its longhand components) two value property-indexed keyframes specification 
 PASS Element.animate() accepts a two property two value property-indexed keyframes specification 
 PASS Element.animate() accepts a two property property-indexed keyframes specification with different numbers of values 
 PASS Element.animate() accepts a property-indexed keyframes specification with an invalid value 
 PASS Element.animate() accepts a one property two value property-indexed keyframes specification that needs to stringify its values 
-FAIL Element.animate() accepts a property-indexed keyframes specification with a CSS variable reference assert_equals: value for 'left' on ComputedKeyframe #0 expected "var(--dist)" but got "auto"
-FAIL Element.animate() accepts a property-indexed keyframes specification with a CSS variable reference in a shorthand property assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS Element.animate() accepts a property-indexed keyframes specification with a CSS variable reference 
+PASS Element.animate() accepts a property-indexed keyframes specification with a CSS variable reference in a shorthand property 
 PASS Element.animate() accepts a one property one value property-indexed keyframes specification 
 PASS Element.animate() accepts a one property one non-array value property-indexed keyframes specification 
 PASS Element.animate() accepts a one property two value property-indexed keyframes specification where the first value is invalid 
@@ -47,13 +47,13 @@
 PASS Element.animate() accepts a one property one keyframe sequence 
 PASS Element.animate() accepts a one property two keyframe sequence 
 PASS Element.animate() accepts a two property two keyframe sequence 
-FAIL Element.animate() accepts a one shorthand property two keyframe sequence assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
-FAIL Element.animate() accepts a two property (a shorthand and one of its component longhands) two keyframe sequence assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,marginTop,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS Element.animate() accepts a one shorthand property two keyframe sequence 
+PASS Element.animate() accepts a two property (a shorthand and one of its component longhands) two keyframe sequence 
 PASS Element.animate() accepts a two property keyframe sequence where one property is missing from the first keyframe 
 PASS Element.animate() accepts a two property keyframe sequence where one property is missing from the last keyframe 
 PASS Element.animate() accepts a one property two keyframe sequence that needs to stringify its values 
-FAIL Element.animate() accepts a keyframe sequence with a CSS variable reference assert_equals: value for 'left' on ComputedKeyframe #0 expected "var(--dist)" but got "auto"
-FAIL Element.animate() accepts a keyframe sequence with a CSS variable reference in a shorthand property assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS Element.animate() accepts a keyframe sequence with a CSS variable reference 
+PASS Element.animate() accepts a keyframe sequence with a CSS variable reference in a shorthand property 
 FAIL Element.animate() accepts a keyframe sequence with duplicate values for a given interior offset Type error
 FAIL Element.animate() accepts a keyframe sequence with duplicate values for offsets 0 and 1 Type error
 FAIL Element.animate() accepts a two property four keyframe sequence Type error

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/constructor-expected.txt (228701 => 228702)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/constructor-expected.txt	2018-02-19 19:03:37 UTC (rev 228701)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/constructor-expected.txt	2018-02-19 19:06:25 UTC (rev 228702)
@@ -7,9 +7,9 @@
 PASS composite value is null if the composite operation specified on the keyframe effect is being used 
 PASS A KeyframeEffectReadOnly can be constructed with a one property two value property-indexed keyframes specification 
 PASS A KeyframeEffectReadOnly constructed with a one property two value property-indexed keyframes specification roundtrips 
-FAIL A KeyframeEffectReadOnly can be constructed with a one shorthand property two value property-indexed keyframes specification assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS A KeyframeEffectReadOnly can be constructed with a one shorthand property two value property-indexed keyframes specification 
 PASS A KeyframeEffectReadOnly constructed with a one shorthand property two value property-indexed keyframes specification roundtrips 
-FAIL A KeyframeEffectReadOnly can be constructed with a two property (one shorthand and one of its longhand components) two value property-indexed keyframes specification assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,marginTop,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS A KeyframeEffectReadOnly can be constructed with a two property (one shorthand and one of its longhand components) two value property-indexed keyframes specification 
 PASS A KeyframeEffectReadOnly constructed with a two property (one shorthand and one of its longhand components) two value property-indexed keyframes specification roundtrips 
 PASS A KeyframeEffectReadOnly can be constructed with a two property two value property-indexed keyframes specification 
 PASS A KeyframeEffectReadOnly constructed with a two property two value property-indexed keyframes specification roundtrips 
@@ -19,9 +19,9 @@
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframes specification with an invalid value roundtrips 
 PASS A KeyframeEffectReadOnly can be constructed with a one property two value property-indexed keyframes specification that needs to stringify its values 
 PASS A KeyframeEffectReadOnly constructed with a one property two value property-indexed keyframes specification that needs to stringify its values roundtrips 
-FAIL A KeyframeEffectReadOnly can be constructed with a property-indexed keyframes specification with a CSS variable reference assert_equals: value for 'left' on ComputedKeyframe #0 expected "var(--dist)" but got "auto"
+PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyframes specification with a CSS variable reference 
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframes specification with a CSS variable reference roundtrips 
-FAIL A KeyframeEffectReadOnly can be constructed with a property-indexed keyframes specification with a CSS variable reference in a shorthand property assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyframes specification with a CSS variable reference in a shorthand property 
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframes specification with a CSS variable reference in a shorthand property roundtrips 
 PASS A KeyframeEffectReadOnly can be constructed with a one property one value property-indexed keyframes specification 
 PASS A KeyframeEffectReadOnly constructed with a one property one value property-indexed keyframes specification roundtrips 
@@ -85,9 +85,9 @@
 PASS A KeyframeEffectReadOnly constructed with a one property two keyframe sequence roundtrips 
 PASS A KeyframeEffectReadOnly can be constructed with a two property two keyframe sequence 
 PASS A KeyframeEffectReadOnly constructed with a two property two keyframe sequence roundtrips 
-FAIL A KeyframeEffectReadOnly can be constructed with a one shorthand property two keyframe sequence assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS A KeyframeEffectReadOnly can be constructed with a one shorthand property two keyframe sequence 
 PASS A KeyframeEffectReadOnly constructed with a one shorthand property two keyframe sequence roundtrips 
-FAIL A KeyframeEffectReadOnly can be constructed with a two property (a shorthand and one of its component longhands) two keyframe sequence assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,marginTop,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS A KeyframeEffectReadOnly can be constructed with a two property (a shorthand and one of its component longhands) two keyframe sequence 
 PASS A KeyframeEffectReadOnly constructed with a two property (a shorthand and one of its component longhands) two keyframe sequence roundtrips 
 PASS A KeyframeEffectReadOnly can be constructed with a two property keyframe sequence where one property is missing from the first keyframe 
 PASS A KeyframeEffectReadOnly constructed with a two property keyframe sequence where one property is missing from the first keyframe roundtrips 
@@ -95,9 +95,9 @@
 PASS A KeyframeEffectReadOnly constructed with a two property keyframe sequence where one property is missing from the last keyframe roundtrips 
 PASS A KeyframeEffectReadOnly can be constructed with a one property two keyframe sequence that needs to stringify its values 
 PASS A KeyframeEffectReadOnly constructed with a one property two keyframe sequence that needs to stringify its values roundtrips 
-FAIL A KeyframeEffectReadOnly can be constructed with a keyframe sequence with a CSS variable reference assert_equals: value for 'left' on ComputedKeyframe #0 expected "var(--dist)" but got "auto"
+PASS A KeyframeEffectReadOnly can be constructed with a keyframe sequence with a CSS variable reference 
 PASS A KeyframeEffectReadOnly constructed with a keyframe sequence with a CSS variable reference roundtrips 
-FAIL A KeyframeEffectReadOnly can be constructed with a keyframe sequence with a CSS variable reference in a shorthand property assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS A KeyframeEffectReadOnly can be constructed with a keyframe sequence with a CSS variable reference in a shorthand property 
 PASS A KeyframeEffectReadOnly constructed with a keyframe sequence with a CSS variable reference in a shorthand property roundtrips 
 FAIL A KeyframeEffectReadOnly can be constructed with a keyframe sequence with duplicate values for a given interior offset Type error
 FAIL A KeyframeEffectReadOnly constructed with a keyframe sequence with duplicate values for a given interior offset roundtrips Type error

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/setKeyframes-expected.txt (228701 => 228702)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/setKeyframes-expected.txt	2018-02-19 19:03:37 UTC (rev 228701)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/setKeyframes-expected.txt	2018-02-19 19:06:25 UTC (rev 228702)
@@ -1,14 +1,14 @@
 
 PASS Keyframes can be replaced with an empty keyframe 
 PASS Keyframes can be replaced with a one property two value property-indexed keyframes specification 
-FAIL Keyframes can be replaced with a one shorthand property two value property-indexed keyframes specification assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
-FAIL Keyframes can be replaced with a two property (one shorthand and one of its longhand components) two value property-indexed keyframes specification assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,marginTop,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS Keyframes can be replaced with a one shorthand property two value property-indexed keyframes specification 
+PASS Keyframes can be replaced with a two property (one shorthand and one of its longhand components) two value property-indexed keyframes specification 
 PASS Keyframes can be replaced with a two property two value property-indexed keyframes specification 
 PASS Keyframes can be replaced with a two property property-indexed keyframes specification with different numbers of values 
 PASS Keyframes can be replaced with a property-indexed keyframes specification with an invalid value 
 PASS Keyframes can be replaced with a one property two value property-indexed keyframes specification that needs to stringify its values 
-FAIL Keyframes can be replaced with a property-indexed keyframes specification with a CSS variable reference assert_equals: value for 'left' on ComputedKeyframe #0 expected "var(--dist)" but got "auto"
-FAIL Keyframes can be replaced with a property-indexed keyframes specification with a CSS variable reference in a shorthand property assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS Keyframes can be replaced with a property-indexed keyframes specification with a CSS variable reference 
+PASS Keyframes can be replaced with a property-indexed keyframes specification with a CSS variable reference in a shorthand property 
 PASS Keyframes can be replaced with a one property one value property-indexed keyframes specification 
 PASS Keyframes can be replaced with a one property one non-array value property-indexed keyframes specification 
 PASS Keyframes can be replaced with a one property two value property-indexed keyframes specification where the first value is invalid 
@@ -40,13 +40,13 @@
 PASS Keyframes can be replaced with a one property one keyframe sequence 
 PASS Keyframes can be replaced with a one property two keyframe sequence 
 PASS Keyframes can be replaced with a two property two keyframe sequence 
-FAIL Keyframes can be replaced with a one shorthand property two keyframe sequence assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
-FAIL Keyframes can be replaced with a two property (a shorthand and one of its component longhands) two keyframe sequence assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,marginTop,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS Keyframes can be replaced with a one shorthand property two keyframe sequence 
+PASS Keyframes can be replaced with a two property (a shorthand and one of its component longhands) two keyframe sequence 
 PASS Keyframes can be replaced with a two property keyframe sequence where one property is missing from the first keyframe 
 PASS Keyframes can be replaced with a two property keyframe sequence where one property is missing from the last keyframe 
 PASS Keyframes can be replaced with a one property two keyframe sequence that needs to stringify its values 
-FAIL Keyframes can be replaced with a keyframe sequence with a CSS variable reference assert_equals: value for 'left' on ComputedKeyframe #0 expected "var(--dist)" but got "auto"
-FAIL Keyframes can be replaced with a keyframe sequence with a CSS variable reference in a shorthand property assert_equals: properties on ComputedKeyframe #0 should match expected "composite,computedOffset,easing,margin,offset" but got "composite,computedOffset,easing,marginBottom,marginLeft,marginRight,marginTop,offset"
+PASS Keyframes can be replaced with a keyframe sequence with a CSS variable reference 
+PASS Keyframes can be replaced with a keyframe sequence with a CSS variable reference in a shorthand property 
 FAIL Keyframes can be replaced with a keyframe sequence with duplicate values for a given interior offset Type error
 FAIL Keyframes can be replaced with a keyframe sequence with duplicate values for offsets 0 and 1 Type error
 FAIL Keyframes can be replaced with a two property four keyframe sequence Type error

Modified: trunk/Source/WebCore/ChangeLog (228701 => 228702)


--- trunk/Source/WebCore/ChangeLog	2018-02-19 19:03:37 UTC (rev 228701)
+++ trunk/Source/WebCore/ChangeLog	2018-02-19 19:06:25 UTC (rev 228702)
@@ -1,3 +1,64 @@
+2018-02-17  Antoine Quint  <grao...@apple.com>
+
+        [Web Animations] Store all parsed keyframe input information in a single structure
+        https://bugs.webkit.org/show_bug.cgi?id=182903
+
+        Reviewed by Dean Jackson.
+
+        When parsing keyframe input provided through the JS API, we used to create several data structures.
+        During parsing we would create a Vector<ProcessedKeyframe> where we would store the validated values
+        for "offset", "easing" and "composite" as well as CSS properties and CSS values as strings. 
+
+        Then we would create a KeyframeList, a class that pre-dates the work on Web Animations and is used
+        for hardware animations, with RenderStyle objects that are used for CSS property blending at runtime.
+        Once the KeyframeList was created, the Vector<ProcessedKeyframe> was discarded.
+
+        Since KeyframeList did not know about nullable offsets, timing functions and composite operations, and
+        because we do not with to modify a legacy class that we will eventually remove once all the Web Animations
+        work is complete, we also stored the parsed offsets as m_offsets, the timing functions as m_timingFunctions
+        and the composite operations as m_compositeOperations.
+
+        In this patch we rename the ProcessedKeyframe structure used temporarily during parsing to ParsedKeyframe and
+        store both the input and processed data related to a given keyframe in that single structure which we keep
+        around as m_parsedKeyframes when we finished processing the keyframes input. This update ParsedKeyframe structure
+        allows to keep around the original nullable offsets, the original CSS properties and CSS values as strings as
+        a HashMap<CSSPropertyID, String>, as well as the CSS properties and CSS values as CSSValue objects using a
+        MutableStyleProperties object. 
+
+        This has the benefit of reducing the number of members, but also pave the way for supporting read-write targets
+        where we will be able to decouple parsing keyframes and creating a KeyframeList, which requires a valid target
+        to create RenderStyle objects used for blending, since the original parsing-time information is now stored.
+
+        Finally, this allowed getKeyframes() to be more compliant by returning the CSS values as originally provided in
+        the keyframe input with shorthand properties when provided, rather than the long-hands we used to read back
+        through RenderStyle objects.
+
+        The generated KeyframeList is now stored as m_blendingKeyframes and is only used for the purpose of interfacing
+        with hardware animations and CSS property blending.
+
+        While ProcessedKeyframe was copyable due to holding only simple types, ParsedKeyframe is not since it uses a Ref
+        to hold the MutableStyleProperties. This uncovered some cases where we copied ProcessedKeyframe objects, we now
+        ensure that the ParsedKeyframe objects are moved instead, which was the correct thing to do all along.
+
+        * animation/KeyframeEffectReadOnly.cpp:
+        (WebCore::computeMissingKeyframeOffsets): While we used to store std::optional<double> for the computed offset,
+        we now store a simple double, which makes more sense since the computed offset is eventually a fully resolved
+        value after calling computeMissingKeyframeOffsets(). So we now compute the final computed offset without resorting
+        to intermediate nullable computed offsets.
+        (WebCore::processIterableKeyframes):
+        (WebCore::processPropertyIndexedKeyframes):
+        (WebCore::KeyframeEffectReadOnly::KeyframeEffectReadOnly):
+        (WebCore::KeyframeEffectReadOnly::copyPropertiesFromSource):
+        (WebCore::KeyframeEffectReadOnly::getKeyframes):
+        (WebCore::KeyframeEffectReadOnly::processKeyframes):
+        (WebCore::KeyframeEffectReadOnly::computeStackingContextImpact):
+        (WebCore::KeyframeEffectReadOnly::shouldRunAccelerated):
+        (WebCore::KeyframeEffectReadOnly::getAnimatedStyle):
+        (WebCore::KeyframeEffectReadOnly::setAnimatedPropertiesInStyle):
+        (WebCore::KeyframeEffectReadOnly::startOrStopAccelerated):
+        * animation/KeyframeEffectReadOnly.h:
+        (WebCore::KeyframeEffectReadOnly::ParsedKeyframe::ParsedKeyframe):
+
 2018-02-19  Zalan Bujtas  <za...@apple.com>
 
         [RenderTreeBuilder] Remove redundant RenderObject::removeFromParentAndDestroy

Modified: trunk/Source/WebCore/animation/KeyframeEffectReadOnly.cpp (228701 => 228702)


--- trunk/Source/WebCore/animation/KeyframeEffectReadOnly.cpp	2018-02-19 19:03:37 UTC (rev 228701)
+++ trunk/Source/WebCore/animation/KeyframeEffectReadOnly.cpp	2018-02-19 19:06:25 UTC (rev 228702)
@@ -36,7 +36,6 @@
 #include "JSCompositeOperation.h"
 #include "JSKeyframeEffectReadOnly.h"
 #include "RenderStyle.h"
-#include "StyleProperties.h"
 #include "StyleResolver.h"
 #include "TimingFunction.h"
 #include "WillChangeData.h"
@@ -71,7 +70,7 @@
     return getJSPropertyName(cssPropertyId);
 }
 
-static inline void computeMissingKeyframeOffsets(Vector<KeyframeEffectReadOnly::ProcessedKeyframe>& keyframes)
+static inline void computeMissingKeyframeOffsets(Vector<KeyframeEffectReadOnly::ParsedKeyframe>& keyframes)
 {
     // https://drafts.csswg.org/web-animations-1/#compute-missing-keyframe-offsets
 
@@ -79,16 +78,19 @@
         return;
 
     // 1. For each keyframe, in keyframes, let the computed keyframe offset of the keyframe be equal to its keyframe offset value.
+    // In our implementation, we only set non-null values to avoid making computedOffset std::optional<double>. Instead, we'll know
+    // that a keyframe hasn't had a computed offset by checking if it has a null offset and a 0 computedOffset, since the first
+    // keyframe will already have a 0 computedOffset.
     for (auto& keyframe : keyframes)
-        keyframe.computedOffset = keyframe.offset;
+        keyframe.computedOffset = keyframe.offset.value_or(0);
 
     // 2. If keyframes contains more than one keyframe and the computed keyframe offset of the first keyframe in keyframes is null,
     //    set the computed keyframe offset of the first keyframe to 0.
-    if (keyframes.size() > 1 && !keyframes[0].computedOffset)
+    if (keyframes.size() > 1 && !keyframes[0].offset)
         keyframes[0].computedOffset = 0;
 
     // 3. If the computed keyframe offset of the last keyframe in keyframes is null, set its computed keyframe offset to 1.
-    if (!keyframes.last().computedOffset)
+    if (!keyframes.last().offset)
         keyframes.last().computedOffset = 1;
 
     // 4. For each pair of keyframes A and B where:
@@ -108,8 +110,8 @@
         if (indexOfLastKeyframeWithNonNullOffset == i - 1)
             continue;
 
-        double lastNonNullOffset = keyframes[indexOfLastKeyframeWithNonNullOffset].computedOffset.value();
-        double offsetDelta = keyframe.computedOffset.value() - lastNonNullOffset;
+        double lastNonNullOffset = keyframes[indexOfLastKeyframeWithNonNullOffset].computedOffset;
+        double offsetDelta = keyframe.computedOffset - lastNonNullOffset;
         double offsetIncrement = offsetDelta / (i - indexOfLastKeyframeWithNonNullOffset);
         size_t indexOfFirstKeyframeWithNullOffset = indexOfLastKeyframeWithNonNullOffset + 1;
         for (size_t j = indexOfFirstKeyframeWithNullOffset; j < i; ++j)
@@ -119,13 +121,13 @@
     }
 }
 
-static inline ExceptionOr<void> processIterableKeyframes(ExecState& state, Strong<JSObject>&& keyframesInput, JSValue method, Vector<KeyframeEffectReadOnly::ProcessedKeyframe>& processedKeyframes)
+static inline ExceptionOr<void> processIterableKeyframes(ExecState& state, Strong<JSObject>&& keyframesInput, JSValue method, Vector<KeyframeEffectReadOnly::ParsedKeyframe>& parsedKeyframes)
 {
     VM& vm = state.vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
     // 1. Let iter be GetIterator(object, method).
-    forEachInIterable(state, keyframesInput.get(), method, [&processedKeyframes](VM& vm, ExecState& state, JSValue nextValue) -> ExceptionOr<void> {
+    forEachInIterable(state, keyframesInput.get(), method, [&parsedKeyframes](VM& vm, ExecState& state, JSValue nextValue) -> ExceptionOr<void> {
         if (!nextValue || !nextValue.isObject())
             return Exception { TypeError };
 
@@ -136,7 +138,7 @@
         JSObject::getOwnPropertyNames(keyframe, &state, ownPropertyNames, EnumerationMode());
         size_t numberOfProperties = ownPropertyNames.size();
 
-        KeyframeEffectReadOnly::ProcessedKeyframe keyframeOutput;
+        KeyframeEffectReadOnly::ParsedKeyframe keyframeOutput;
 
         String easing("linear");
         std::optional<double> offset;
@@ -153,8 +155,11 @@
                 composite = convert<IDLNullable<IDLEnumeration<CompositeOperation>>>(state, ownPropertyRawValue);
             else {
                 auto cssPropertyId = IDLAttributeNameToAnimationPropertyName(ownPropertyName.string());
-                if (CSSPropertyAnimation::isPropertyAnimatable(cssPropertyId))
-                    keyframeOutput.cssPropertiesAndValues.set(cssPropertyId, convert<IDLDOMString>(state, ownPropertyRawValue));
+                if (CSSPropertyAnimation::isPropertyAnimatable(cssPropertyId)) {
+                    auto stringValue = convert<IDLDOMString>(state, ownPropertyRawValue);
+                    if (keyframeOutput.style->setProperty(cssPropertyId, stringValue))
+                        keyframeOutput.unparsedStyle.set(cssPropertyId, stringValue);
+                }
             }
             RETURN_IF_EXCEPTION(scope, Exception { TypeError });
         }
@@ -163,7 +168,7 @@
         keyframeOutput.offset = offset;
         keyframeOutput.composite = composite;
 
-        processedKeyframes.append(WTFMove(keyframeOutput));
+        parsedKeyframes.append(WTFMove(keyframeOutput));
 
         return { };
     });
@@ -251,7 +256,7 @@
     return { WTFMove(keyframeOuput) };
 }
 
-static inline ExceptionOr<void> processPropertyIndexedKeyframes(ExecState& state, Strong<JSObject>&& keyframesInput, Vector<KeyframeEffectReadOnly::ProcessedKeyframe>& processedKeyframes, Vector<String>& unusedEasings)
+static inline ExceptionOr<void> processPropertyIndexedKeyframes(ExecState& state, Strong<JSObject>&& keyframesInput, Vector<KeyframeEffectReadOnly::ParsedKeyframe>& parsedKeyframes, Vector<String>& unusedEasings)
 {
     // 1. Let property-indexed keyframe be the result of running the procedure to process a keyframe-like object passing object as the keyframe input.
     auto processKeyframeLikeObjectResult = processKeyframeLikeObject(state, WTFMove(keyframesInput));
@@ -269,15 +274,16 @@
         // 3. Let property values be the value for m.
         auto propertyValues = m.values;
         // 4. Let property keyframes be an empty sequence of keyframes.
-        Vector<KeyframeEffectReadOnly::ProcessedKeyframe> propertyKeyframes;
+        Vector<KeyframeEffectReadOnly::ParsedKeyframe> propertyKeyframes;
         // 5. For each value, v, in property values perform the following steps:
         for (auto& v : propertyValues) {
             // 1. Let k be a new keyframe with a null keyframe offset.
-            KeyframeEffectReadOnly::ProcessedKeyframe k;
+            KeyframeEffectReadOnly::ParsedKeyframe k;
             // 2. Add the property-value pair, property name → v, to k.
-            k.cssPropertiesAndValues.set(propertyName, v);
+            if (k.style->setProperty(propertyName, v))
+                k.unparsedStyle.set(propertyName, v);
             // 3. Append k to property keyframes.
-            propertyKeyframes.append(k);
+            propertyKeyframes.append(WTFMove(k));
         }
         // 6. Apply the procedure to compute missing keyframe offsets to property keyframes.
         computeMissingKeyframeOffsets(propertyKeyframes);
@@ -284,34 +290,39 @@
 
         // 7. Add keyframes in property keyframes to processed keyframes.
         for (auto& keyframe : propertyKeyframes)
-            processedKeyframes.append(keyframe);
+            parsedKeyframes.append(WTFMove(keyframe));
     }
 
     // 3. Sort processed keyframes by the computed keyframe offset of each keyframe in increasing order.
-    std::sort(processedKeyframes.begin(), processedKeyframes.end(), [](auto& lhs, auto& rhs) {
-        return lhs.computedOffset.value() < rhs.computedOffset.value();
+    std::sort(parsedKeyframes.begin(), parsedKeyframes.end(), [](auto& lhs, auto& rhs) {
+        return lhs.computedOffset < rhs.computedOffset;
     });
 
     // 4. Merge adjacent keyframes in processed keyframes when they have equal computed keyframe offsets.
     size_t i = 1;
-    while (i < processedKeyframes.size()) {
-        auto& keyframe = processedKeyframes[i];
-        auto& previousKeyframe = processedKeyframes[i - 1];
+    while (i < parsedKeyframes.size()) {
+        auto& keyframe = parsedKeyframes[i];
+        auto& previousKeyframe = parsedKeyframes[i - 1];
         // If the offsets of this keyframe and the previous keyframe are different,
         // this means that the two keyframes should not be merged and we can move
         // on to the next keyframe.
-        if (keyframe.computedOffset.value() != previousKeyframe.computedOffset.value()) {
+        if (keyframe.computedOffset != previousKeyframe.computedOffset) {
             i++;
             continue;
         }
         // Otherwise, both this keyframe and the previous keyframe should be merged.
-        // Unprocessed keyframes in processedKeyframes at this stage have a single
+        // Unprocessed keyframes in parsedKeyframes at this stage have at most a single
         // property in cssPropertiesAndValues, so just set this on the previous keyframe.
-        auto singleValueInKeyframe = keyframe.cssPropertiesAndValues.begin();
-        previousKeyframe.cssPropertiesAndValues.set(singleValueInKeyframe->key, singleValueInKeyframe->value);
+        // In case an invalid or null value was originally provided, then the property
+        // was not set and the property count is 0, in which case there is nothing to merge.
+        if (keyframe.style->propertyCount()) {
+            auto property = keyframe.style->propertyAt(0);
+            previousKeyframe.style->setProperty(property.id(), property.value());
+            previousKeyframe.unparsedStyle.set(property.id(), keyframe.unparsedStyle.get(property.id()));
+        }
         // Since we've processed this keyframe, we can remove it and keep i the same
         // so that we process the next keyframe in the next loop iteration.
-        processedKeyframes.remove(i);
+        parsedKeyframes.remove(i);
     }
 
     // 5. Let offsets be a sequence of nullable double values assigned based on the type of the “offset” member of the property-indexed keyframe as follows:
@@ -326,8 +337,8 @@
         offsets.append(std::nullopt);
 
     // 6. Assign each value in offsets to the keyframe offset of the keyframe with corresponding position in property keyframes until the end of either sequence is reached.
-    for (size_t i = 0; i < offsets.size() && i < processedKeyframes.size(); ++i)
-        processedKeyframes[i].offset = offsets[i];
+    for (size_t i = 0; i < offsets.size() && i < parsedKeyframes.size(); ++i)
+        parsedKeyframes[i].offset = offsets[i];
 
     // 7. Let easings be a sequence of DOMString values assigned based on the type of the “easing” member of the property-indexed keyframe as follows:
     //    - sequence<DOMString>, the value of “easing” as-is.
@@ -344,20 +355,20 @@
 
     // 9. If easings has fewer items than property keyframes, repeat the elements in easings successively starting from the beginning of the list until easings has as many
     //    items as property keyframes.
-    if (easings.size() < processedKeyframes.size()) {
+    if (easings.size() < parsedKeyframes.size()) {
         size_t initialNumberOfEasings = easings.size();
-        for (i = initialNumberOfEasings + 1; i <= processedKeyframes.size(); ++i)
+        for (i = initialNumberOfEasings + 1; i <= parsedKeyframes.size(); ++i)
             easings.append(easings[i % initialNumberOfEasings]);
     }
 
     // 10. If easings has more items than property keyframes, store the excess items as unused easings.
-    while (easings.size() > processedKeyframes.size())
+    while (easings.size() > parsedKeyframes.size())
         unusedEasings.append(easings.takeLast());
 
     // 11. Assign each value in easings to a property named “easing” on the keyframe with the corresponding position in property keyframes until the end of property keyframes
     //     is reached.
-    for (size_t i = 0; i < processedKeyframes.size(); ++i)
-        processedKeyframes[i].easing = easings[i];
+    for (size_t i = 0; i < parsedKeyframes.size(); ++i)
+        parsedKeyframes[i].easing = easings[i];
 
     // 12. If the “composite” member of the property-indexed keyframe is not an empty sequence:
     Vector<std::optional<CompositeOperation>> compositeModes;
@@ -372,15 +383,15 @@
         //    operation, let composite modes be a sequence of length one, with the value of the “composite” as its single item.
         // 2. As with easings, if composite modes has fewer items than property keyframes, repeat the elements in composite modes successively starting from the beginning of
         //    the list until composite modes has as many items as property keyframes.
-        if (compositeModes.size() < processedKeyframes.size()) {
+        if (compositeModes.size() < parsedKeyframes.size()) {
             size_t initialNumberOfCompositeModes = compositeModes.size();
-            for (i = initialNumberOfCompositeModes + 1; i <= processedKeyframes.size(); ++i)
+            for (i = initialNumberOfCompositeModes + 1; i <= parsedKeyframes.size(); ++i)
                 compositeModes.append(compositeModes[i % initialNumberOfCompositeModes]);
         }
         // 3. Assign each value in composite modes to the keyframe-specific composite operation on the keyframe with the corresponding position in property keyframes until
         //    the end of property keyframes is reached.
-        for (size_t i = 0; i < compositeModes.size() && i < processedKeyframes.size(); ++i)
-            processedKeyframes[i].composite = compositeModes[i];
+        for (size_t i = 0; i < compositeModes.size() && i < parsedKeyframes.size(); ++i)
+            parsedKeyframes[i].composite = compositeModes[i];
     }
 
     return { };
@@ -411,7 +422,7 @@
 KeyframeEffectReadOnly::KeyframeEffectReadOnly(ClassType classType, Ref<AnimationEffectTimingReadOnly>&& timing, Element* target)
     : AnimationEffectReadOnly(classType, WTFMove(timing))
     , m_target(target)
-    , m_keyframes(emptyString())
+    , m_blendingKeyframes(emptyString())
 {
 }
 
@@ -418,22 +429,33 @@
 void KeyframeEffectReadOnly::copyPropertiesFromSource(Ref<KeyframeEffectReadOnly>&& source)
 {
     m_target = source->m_target;
-    m_offsets = source->m_offsets;
-    m_timingFunctions = source->m_timingFunctions;
     m_compositeOperation = source->m_compositeOperation;
-    m_compositeOperations = source->m_compositeOperations;
     m_iterationCompositeOperation = source->m_iterationCompositeOperation;
 
+    Vector<ParsedKeyframe> parsedKeyframes;
+    for (auto& sourceParsedKeyframe : source->m_parsedKeyframes) {
+        ParsedKeyframe parsedKeyframe;
+        parsedKeyframe.easing = sourceParsedKeyframe.easing;
+        parsedKeyframe.offset = sourceParsedKeyframe.offset;
+        parsedKeyframe.composite = sourceParsedKeyframe.composite;
+        parsedKeyframe.unparsedStyle = sourceParsedKeyframe.unparsedStyle;
+        parsedKeyframe.computedOffset = sourceParsedKeyframe.computedOffset;
+        parsedKeyframe.timingFunction = sourceParsedKeyframe.timingFunction;
+        parsedKeyframe.style = sourceParsedKeyframe.style->mutableCopy();
+        parsedKeyframes.append(WTFMove(parsedKeyframe));
+    }
+    m_parsedKeyframes = WTFMove(parsedKeyframes);
+
     timing()->copyPropertiesFromSource(source->timing());
 
     KeyframeList keyframeList("keyframe-effect-" + createCanonicalUUIDString());
-    for (auto& keyframe : source->m_keyframes.keyframes()) {
+    for (auto& keyframe : source->m_blendingKeyframes.keyframes()) {
         KeyframeValue keyframeValue(keyframe.key(), RenderStyle::clonePtr(*keyframe.style()));
         for (auto propertyId : keyframe.properties())
             keyframeValue.addProperty(propertyId);
         keyframeList.insert(WTFMove(keyframeValue));
     }
-    m_keyframes = WTFMove(keyframeList);
+    m_blendingKeyframes = WTFMove(keyframeList);
 }
 
 Vector<Strong<JSObject>> KeyframeEffectReadOnly::getKeyframes(ExecState& state)
@@ -451,9 +473,7 @@
     // 2. Let keyframes be the result of applying the procedure to compute missing keyframe offsets to the keyframes for this keyframe effect.
 
     // 3. For each keyframe in keyframes perform the following steps:
-    for (size_t i = 0; i < m_keyframes.size(); ++i) {
-        auto& keyframe = m_keyframes[i];
-
+    for (auto& parsedKeyframe : m_parsedKeyframes) {
         // 1. Initialize a dictionary object, output keyframe, using the following definition:
         //
         // dictionary BaseComputedKeyframe {
@@ -466,24 +486,20 @@
         // 2. Set offset, computedOffset, easing, composite members of output keyframe to the respective values keyframe offset, computed keyframe
         // offset, keyframe-specific timing function and keyframe-specific composite operation of keyframe.
         BaseComputedKeyframe computedKeyframe;
-        computedKeyframe.offset = m_offsets[i];
-        computedKeyframe.computedOffset = keyframe.key();
-        computedKeyframe.easing = m_timingFunctions[i]->cssText();
-        computedKeyframe.composite = m_compositeOperations[i];
+        computedKeyframe.offset = parsedKeyframe.offset;
+        computedKeyframe.computedOffset = parsedKeyframe.computedOffset;
+        computedKeyframe.easing = parsedKeyframe.timingFunction->cssText();
+        computedKeyframe.composite = parsedKeyframe.composite;
 
         auto outputKeyframe = convertDictionaryToJS(state, *jsCast<JSDOMGlobalObject*>(state.lexicalGlobalObject()), computedKeyframe);
 
-        auto& style = *keyframe.style();
-        auto computedStyleExtractor = ComputedStyleExtractor(m_target.get());
-
         // 3. For each animation property-value pair specified on keyframe, declaration, perform the following steps:
-        for (auto cssPropertyId : keyframe.properties()) {
+        for (auto it = parsedKeyframe.unparsedStyle.begin(), end = parsedKeyframe.unparsedStyle.end(); it != end; ++it) {
             // 1. Let property name be the result of applying the animation property name to IDL attribute name algorithm to the property name of declaration.
-            auto propertyName = CSSPropertyIDToIDLAttributeName(cssPropertyId);
+            auto propertyName = CSSPropertyIDToIDLAttributeName(it->key);
             // 2. Let IDL value be the result of serializing the property value of declaration by passing declaration to the algorithm to serialize a CSS value.
-            auto idlValue = computedStyleExtractor.valueForPropertyinStyle(style, cssPropertyId)->cssText();
             // 3. Let value be the result of converting IDL value to an ECMAScript String value.
-            auto value = toJS<IDLDOMString>(state, idlValue);
+            auto value = toJS<IDLDOMString>(state, it->value);
             // 4. Call the [[DefineOwnProperty]] internal method on output keyframe with property name property name,
             //    Property Descriptor { [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true, [[Value]]: value } and Boolean flag false.
             JSObject::defineOwnProperty(outputKeyframe, &state, AtomicString(propertyName).impl(), PropertyDescriptor(value, 0), false);
@@ -507,7 +523,7 @@
     auto scope = DECLARE_THROW_SCOPE(vm);
 
     // 2. Let processed keyframes be an empty sequence of keyframes.
-    Vector<ProcessedKeyframe> processedKeyframes;
+    Vector<ParsedKeyframe> parsedKeyframes;
 
     // 3. Let method be the result of GetMethod(object, @@iterator).
     auto method = keyframesInput.get()->get(&state, vm.propertyNames->iteratorSymbol);
@@ -518,15 +534,15 @@
     // 5. Perform the steps corresponding to the first matching condition from below,
     Vector<String> unusedEasings;
     if (!method.isUndefined())
-        processIterableKeyframes(state, WTFMove(keyframesInput), WTFMove(method), processedKeyframes);
+        processIterableKeyframes(state, WTFMove(keyframesInput), WTFMove(method), parsedKeyframes);
     else
-        processPropertyIndexedKeyframes(state, WTFMove(keyframesInput), processedKeyframes, unusedEasings);
+        processPropertyIndexedKeyframes(state, WTFMove(keyframesInput), parsedKeyframes, unusedEasings);
 
     // 6. If processed keyframes is not loosely sorted by offset, throw a TypeError and abort these steps.
     // 7. If there exist any keyframe in processed keyframes whose keyframe offset is non-null and less than
     //    zero or greater than one, throw a TypeError and abort these steps.
     double lastNonNullOffset = -1;
-    for (auto& keyframe : processedKeyframes) {
+    for (auto& keyframe : parsedKeyframes) {
         if (!keyframe.offset)
             continue;
         auto offset = keyframe.offset.value();
@@ -537,41 +553,22 @@
 
     // We take a slight detour from the spec text and compute the missing keyframe offsets right away
     // since they can be computed up-front.
-    computeMissingKeyframeOffsets(processedKeyframes);
+    computeMissingKeyframeOffsets(parsedKeyframes);
 
     KeyframeList keyframeList("keyframe-effect-" + createCanonicalUUIDString());
-    Vector<std::optional<double>> offsets;
-    Vector<RefPtr<TimingFunction>> timingFunctions;
-    Vector<std::optional<CompositeOperation>> compositeOperations;
-
     StyleResolver& styleResolver = m_target->styleResolver();
-    auto parserContext = CSSParserContext(HTMLStandardMode);
 
     // 8. For each frame in processed keyframes, perform the following steps:
-    for (auto& keyframe : processedKeyframes) {
-        offsets.append(keyframe.offset);
-        compositeOperations.append(keyframe.composite);
-
+    for (auto& keyframe : parsedKeyframes) {
         // 1. For each property-value pair in frame, parse the property value using the syntax specified for that property.
         //    If the property value is invalid according to the syntax for the property, discard the property-value pair.
         //    User agents that provide support for diagnosing errors in content SHOULD produce an appropriate warning
         //    highlighting the invalid property value.
 
-        StringBuilder cssText;
-        for (auto it = keyframe.cssPropertiesAndValues.begin(), end = keyframe.cssPropertiesAndValues.end(); it != end; ++it) {
-            cssText.append(getPropertyNameString(it->key));
-            cssText.appendLiteral(": ");
-            cssText.append(it->value);
-            cssText.appendLiteral("; ");
-        }
-
-        KeyframeValue keyframeValue(keyframe.computedOffset.value(), nullptr);
+        KeyframeValue keyframeValue(keyframe.computedOffset, nullptr);
         auto renderStyle = RenderStyle::createPtr();
-        auto styleProperties = MutableStyleProperties::create();
-        styleProperties->parseDeclaration(cssText.toString(), parserContext);
-        unsigned numberOfCSSProperties = styleProperties->propertyCount();
-
-        for (unsigned i = 0; i < numberOfCSSProperties; ++i) {
+        auto& styleProperties = keyframe.style;
+        for (unsigned i = 0; i < styleProperties->propertyCount(); ++i) {
             auto cssPropertyId = styleProperties->propertyAt(i).id();
             keyframeValue.addProperty(cssPropertyId);
             keyframeList.addProperty(cssPropertyId);
@@ -588,7 +585,7 @@
         auto timingFunctionResult = TimingFunction::createFromCSSText(keyframe.easing);
         if (timingFunctionResult.hasException())
             return timingFunctionResult.releaseException();
-        timingFunctions.append(timingFunctionResult.returnValue());
+        keyframe.timingFunction = timingFunctionResult.returnValue();
     }
 
     // 9. Parse each of the values in unused easings using the CSS syntax defined for easing property of the
@@ -600,10 +597,8 @@
             return timingFunctionResult.releaseException();
     }
 
-    m_offsets = WTFMove(offsets);
-    m_keyframes = WTFMove(keyframeList);
-    m_timingFunctions = WTFMove(timingFunctions);
-    m_compositeOperations = WTFMove(compositeOperations);
+    m_blendingKeyframes = WTFMove(keyframeList);
+    m_parsedKeyframes = WTFMove(parsedKeyframes);
 
     computeStackingContextImpact();
 
@@ -613,7 +608,7 @@
 void KeyframeEffectReadOnly::computeStackingContextImpact()
 {
     m_triggersStackingContext = false;
-    for (auto cssPropertyId : m_keyframes.properties()) {
+    for (auto cssPropertyId : m_blendingKeyframes.properties()) {
         if (WillChangeData::propertyCreatesStackingContext(cssPropertyId)) {
             m_triggersStackingContext = true;
             break;
@@ -666,7 +661,7 @@
 
 bool KeyframeEffectReadOnly::shouldRunAccelerated()
 {
-    for (auto cssPropertyId : m_keyframes.properties()) {
+    for (auto cssPropertyId : m_blendingKeyframes.properties()) {
         if (!CSSPropertyAnimation::animationOfPropertyIsAccelerated(cssPropertyId))
             return false;
     }
@@ -678,7 +673,7 @@
     if (!animation())
         return;
 
-    if (!m_keyframes.size())
+    if (!m_blendingKeyframes.size())
         return;
 
     auto progress = iterationProgress();
@@ -699,7 +694,7 @@
     // The effect value of a single property referenced by a keyframe effect as one of its target properties,
     // for a given iteration progress, current iteration and underlying value is calculated as follows.
 
-    for (auto cssPropertyId : m_keyframes.properties()) {
+    for (auto cssPropertyId : m_blendingKeyframes.properties()) {
         // 1. If iteration progress is unresolved abort this procedure.
         // 2. Let target property be the longhand property for which the effect value is to be calculated.
         // 3. If animation type of the target property is not animatable abort this procedure since the effect cannot be applied.
@@ -711,8 +706,8 @@
         unsigned numberOfKeyframesWithZeroOffset = 0;
         unsigned numberOfKeyframesWithOneOffset = 0;
         Vector<std::optional<size_t>> propertySpecificKeyframes;
-        for (size_t i = 0; i < m_keyframes.size(); ++i) {
-            auto& keyframe = m_keyframes[i];
+        for (size_t i = 0; i < m_blendingKeyframes.size(); ++i) {
+            auto& keyframe = m_blendingKeyframes[i];
             if (!keyframe.containsProperty(cssPropertyId))
                 continue;
             auto offset = keyframe.key();
@@ -768,7 +763,7 @@
                 auto offset = [&] () -> double {
                     if (!keyframeIndex)
                         return i ? 1 : 0;
-                    return m_keyframes[keyframeIndex.value()].key();
+                    return m_blendingKeyframes[keyframeIndex.value()].key();
                 }();
                 if (!offset)
                     indexOfLastKeyframeWithZeroOffset = i;
@@ -794,7 +789,7 @@
         // 13. If there is only one keyframe in interval endpoints return the property value of target property on that keyframe.
         if (intervalEndpoints.size() == 1) {
             auto keyframeIndex = intervalEndpoints[0];
-            auto keyframeStyle = !keyframeIndex ? &targetStyle : m_keyframes[keyframeIndex.value()].style();
+            auto keyframeStyle = !keyframeIndex ? &targetStyle : m_blendingKeyframes[keyframeIndex.value()].style();
             CSSPropertyAnimation::blendProperties(this, cssPropertyId, &targetStyle, keyframeStyle, keyframeStyle, 0);
             continue;
         }
@@ -801,11 +796,11 @@
 
         // 14. Let start offset be the computed keyframe offset of the first keyframe in interval endpoints.
         auto startKeyframeIndex = intervalEndpoints.first();
-        auto startOffset = !startKeyframeIndex ? 0 : m_keyframes[startKeyframeIndex.value()].key();
+        auto startOffset = !startKeyframeIndex ? 0 : m_blendingKeyframes[startKeyframeIndex.value()].key();
 
         // 15. Let end offset be the computed keyframe offset of last keyframe in interval endpoints.
         auto endKeyframeIndex = intervalEndpoints.last();
-        auto endOffset = !endKeyframeIndex ? 1 : m_keyframes[endKeyframeIndex.value()].key();
+        auto endOffset = !endKeyframeIndex ? 1 : m_blendingKeyframes[endKeyframeIndex.value()].key();
 
         // 16. Let interval distance be the result of evaluating (iteration progress - start offset) / (end offset - start offset).
         auto intervalDistance = (iterationProgress - startOffset) / (endOffset - startOffset);
@@ -816,7 +811,7 @@
         if (startKeyframeIndex) {
             if (auto iterationDuration = timing()->iterationDuration()) {
                 auto rangeDuration = (endOffset - startOffset) * iterationDuration.seconds();
-                transformedDistance = m_timingFunctions[startKeyframeIndex.value()]->transformTime(intervalDistance, rangeDuration);
+                transformedDistance = m_parsedKeyframes[startKeyframeIndex.value()].timingFunction->transformTime(intervalDistance, rangeDuration);
             }
         }
 
@@ -823,8 +818,8 @@
         // 18. Return the result of applying the interpolation procedure defined by the animation type of the target property, to the values of the target
         //     property specified on the two keyframes in interval endpoints taking the first such value as Vstart and the second as Vend and using transformed
         //     distance as the interpolation parameter p.
-        auto startStyle = !startKeyframeIndex ? &targetStyle : m_keyframes[startKeyframeIndex.value()].style();
-        auto endStyle = !endKeyframeIndex ? &targetStyle : m_keyframes[endKeyframeIndex.value()].style();
+        auto startStyle = !startKeyframeIndex ? &targetStyle : m_blendingKeyframes[startKeyframeIndex.value()].style();
+        auto endStyle = !endKeyframeIndex ? &targetStyle : m_blendingKeyframes[endKeyframeIndex.value()].style();
         CSSPropertyAnimation::blendProperties(this, cssPropertyId, &targetStyle, startStyle, endStyle, transformedDistance);
     }
 }
@@ -839,9 +834,9 @@
     if (m_startedAccelerated) {
         auto animation = Animation::create();
         animation->setDuration(timing()->iterationDuration().seconds());
-        compositedRenderer->startAnimation(0, animation.ptr(), m_keyframes);
+        compositedRenderer->startAnimation(0, animation.ptr(), m_blendingKeyframes);
     } else {
-        compositedRenderer->animationFinished(m_keyframes.animationName());
+        compositedRenderer->animationFinished(m_blendingKeyframes.animationName());
         if (!m_target->document().renderTreeBeingDestroyed())
             m_target->invalidateStyleAndLayerComposition();
     }

Modified: trunk/Source/WebCore/animation/KeyframeEffectReadOnly.h (228701 => 228702)


--- trunk/Source/WebCore/animation/KeyframeEffectReadOnly.h	2018-02-19 19:03:37 UTC (rev 228701)
+++ trunk/Source/WebCore/animation/KeyframeEffectReadOnly.h	2018-02-19 19:06:25 UTC (rev 228702)
@@ -34,6 +34,7 @@
 #include "KeyframeEffectOptions.h"
 #include "KeyframeList.h"
 #include "RenderStyle.h"
+#include "StyleProperties.h"
 #include <wtf/Ref.h>
 
 namespace WebCore {
@@ -63,12 +64,19 @@
         Vector<PropertyAndValues> propertiesAndValues;
     };
 
-    struct ProcessedKeyframe {
-        String easing;
+    struct ParsedKeyframe {
         std::optional<double> offset;
-        std::optional<double> computedOffset;
+        double computedOffset;
         std::optional<CompositeOperation> composite;
-        HashMap<CSSPropertyID, String> cssPropertiesAndValues;
+        String easing;
+        RefPtr<TimingFunction> timingFunction;
+        Ref<MutableStyleProperties> style;
+        HashMap<CSSPropertyID, String> unparsedStyle;
+
+        ParsedKeyframe()
+            : style(MutableStyleProperties::create())
+        {
+        }
     };
 
     struct BaseComputedKeyframe {
@@ -113,15 +121,13 @@
     void computeStackingContextImpact();
     bool shouldRunAccelerated();
 
-    Vector<std::optional<double>> m_offsets;
-    Vector<RefPtr<TimingFunction>> m_timingFunctions;
-    Vector<std::optional<CompositeOperation>> m_compositeOperations;
     bool m_triggersStackingContext { false };
     bool m_started { false };
     bool m_startedAccelerated { false };
 
     RefPtr<Element> m_target;
-    KeyframeList m_keyframes;
+    KeyframeList m_blendingKeyframes;
+    Vector<ParsedKeyframe> m_parsedKeyframes;
 };
 
 } // namespace WebCore
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to