Web Animations: Correctly handle incomplete keyframes in CSS animations

Source/core/css/resolver/StyleResolver.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 2004-2005 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2006, 2007 Nicholas Shanks (webkit@nickshanks.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Alexey Proskuryakov <ap@webkit.org>
  * Copyright (C) 2007, 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
  * Copyright (C) Research In Motion Limited 2011. All rights reserved.
@@ skipping 65 matching lines @@
 #include "core/rendering/style/KeyframeList.h"
 #include "core/rendering/style/StyleCustomFilterProgramCache.h"
 #include "core/svg/SVGDocumentExtensions.h"
 #include "core/svg/SVGElement.h"
 #include "core/svg/SVGFontFaceElement.h"
 #include "wtf/StdLibExtras.h"
 #include "wtf/Vector.h"
 
 using namespace std;
 
+namespace {
+
+using namespace WebCore;
+
+void prepareEndKeyframes(const RenderStyle* elementStyle, const PropertySet& allProperties, KeyframeAnimationEffect::KeyframeVector& keyframes)
+{
+    // Add 0% and 100% keyframes if absent.
+    RefPtr<Keyframe> startKeyframe = keyframes[0];
+    if (startKeyframe->offset()) {
+        startKeyframe = Keyframe::create();
+        startKeyframe->setOffset(0);
+        keyframes.prepend(startKeyframe);
+    }
+    RefPtr<Keyframe> endKeyframe = keyframes[keyframes.size() - 1];
+    if (endKeyframe->offset() != 1) {
+        endKeyframe = Keyframe::create();
+        endKeyframe->setOffset(1);
+        keyframes.append(endKeyframe);
+    }
+    ASSERT(keyframes.size() >= 2);
+    ASSERT(!keyframes.first()->offset());
+    ASSERT(keyframes.last()->offset() == 1);
+
+    // Snapshot current property values for 0% and 100% if missing.
+    const PropertySet& startKeyframeProperties = startKeyframe->properties();
+    const PropertySet& endKeyframeProperties = endKeyframe->properties();
+    bool missingStartValues = startKeyframeProperties.size() < allProperties.size();
+    bool missingEndValues = endKeyframeProperties.size() < allProperties.size();
+    if (!missingStartValues && !missingEndValues)
+        return;
+    for (PropertySet::const_iterator iter = allProperties.begin(); iter != allProperties.end(); ++iter) {
+        const CSSPropertyID property = *iter;
+        bool startNeedsValue = missingStartValues && !startKeyframeProperties.contains(property);
+        bool endNeedsValue = missingEndValues && !endKeyframeProperties.contains(property);
+        if (!startNeedsValue && !endNeedsValue)
+            continue;
+        RefPtr<AnimatableValue> snapshotValue = CSSAnimatableValueFactory::create(property, elementStyle);
+        if (startNeedsValue)
+            startKeyframe->setPropertyValue(property, snapshotValue.get());
+        if (endNeedsValue)
+            endKeyframe->setPropertyValue(property, snapshotValue.get());
+    }
+    ASSERT(startKeyframe->properties().size() == allProperties.size());
+    ASSERT(endKeyframe->properties().size() == allProperties.size());
+}
+
+PassRefPtr<TimingFunction> generateTimingFunction(const KeyframeAnimationEffect::KeyframeVector keyframes, const HashMap<double, RefPtr<TimingFunction> > perKeyframeTimingFunctions)
+{
+    // Generate the chained timing function. Note that timing functions apply
+    // from the keyframe in which they're specified to the next keyframe.
+    bool isTimingFunctionLinearThroughout = true;
+    RefPtr<ChainedTimingFunction> chainedTimingFunction = ChainedTimingFunction::create();
+    for (size_t i = 0; i < keyframes.size() - 1; ++i) {
+        double lowerBound = keyframes[i]->offset();
+        ASSERT(lowerBound >=0 && lowerBound < 1);
+        double upperBound = keyframes[i + 1]->offset();
+        ASSERT(upperBound > 0 && upperBound <= 1);
+        TimingFunction* timingFunction = perKeyframeTimingFunctions.get(lowerBound);
+        isTimingFunctionLinearThroughout &= timingFunction->type() == TimingFunction::LinearFunction;
+        chainedTimingFunction->appendSegment(upperBound, timingFunction);
+    }
+    if (isTimingFunctionLinearThroughout)
+        return LinearTimingFunction::create();
+    return chainedTimingFunction;
+}
+
+} // namespace
+
 namespace WebCore {
 
 using namespace HTMLNames;
 
 RenderStyle* StyleResolver::s_styleNotYetAvailable;
 
 static StylePropertySet* leftToRightDeclaration()
 {
     DEFINE_STATIC_LOCAL(RefPtr<MutableStylePropertySet>, leftToRightDecl, (MutableStylePropertySet::create()));
     if (leftToRightDecl->isEmpty())
@@ skipping 730 matching lines @@
             hundredPercentKeyframe = StyleKeyframe::create().leakRef();
             hundredPercentKeyframe->setKeyText("100%");
         }
         KeyframeValue keyframeValue(1, 0);
         keyframeValue.setStyle(styleForKeyframe(e, elementStyle, hundredPercentKeyframe));
         keyframeValue.addProperties(hundredPercentKeyframe->properties());
         list.insert(keyframeValue);
     }
 }
 
-void StyleResolver::resolveKeyframes(const Element* element, const RenderStyle* style, const AtomicString& name, TimingFunction* defaultTimingFunction, KeyframeAnimationEffect::KeyframeVector& keyframes, RefPtr<TimingFunction>& timingFunction)
+void StyleResolver::resolveKeyframes(const Element* element, const RenderStyle* style, const AtomicString& name, TimingFunction* defaultTimingFunction, Vector<std::pair<KeyframeAnimationEffect::KeyframeVector, RefPtr<TimingFunction> > >& keyframesAndTimingFunctions)
 {
     ASSERT(RuntimeEnabledFeatures::webAnimationsCSSEnabled());
     const StyleRuleKeyframes* keyframesRule = matchScopedKeyframesRule(element, name.impl());
     if (!keyframesRule)
         return;
 
     // Construct and populate the style for each keyframe
-    HashMap<double, RefPtr<TimingFunction> > timingFunctions;
+    KeyframeAnimationEffect::KeyframeVector keyframes;
+    HashMap<double, RefPtr<TimingFunction> > perKeyframeTimingFunctions;
     const Vector<RefPtr<StyleKeyframe> >& styleKeyframes = keyframesRule->keyframes();
     for (size_t i = 0; i < styleKeyframes.size(); ++i) {
         const StyleKeyframe* styleKeyframe = styleKeyframes[i].get();
         RefPtr<RenderStyle> keyframeStyle = styleForKeyframe(0, style, styleKeyframe);
         RefPtr<Keyframe> keyframe = Keyframe::create();
         const Vector<double>& offsets = styleKeyframe->keys();
         ASSERT(!offsets.isEmpty());
         keyframe->setOffset(offsets[0]);
         TimingFunction* timingFunction = defaultTimingFunction;
         const StylePropertySet* properties = styleKeyframe->properties();
         for (unsigned j = 0; j < properties->propertyCount(); j++) {
             CSSPropertyID property = properties->propertyAt(j).id();
             if (property == CSSPropertyWebkitAnimationTimingFunction || property == CSSPropertyAnimationTimingFunction) {
                 // FIXME: This sometimes gets the wrong timing function. See crbug.com/288540.
-
                 timingFunction = KeyframeValue::timingFunction(keyframeStyle.get(), name);
             } else if (CSSAnimations::isAnimatableProperty(property)) {
                 keyframe->setPropertyValue(property, CSSAnimatableValueFactory::create(property, keyframeStyle.get()).get());
             }
         }
         keyframes.append(keyframe);
         // The last keyframe specified at a given offset is used.
-        timingFunctions.set(offsets[0], timingFunction);
+        perKeyframeTimingFunctions.set(offsets[0], timingFunction);
         for (size_t j = 1; j < offsets.size(); ++j) {
             keyframes.append(keyframe->cloneWithOffset(offsets[j]));
-            timingFunctions.set(offsets[j], timingFunction);
+            perKeyframeTimingFunctions.set(offsets[j], timingFunction);
         }
     }
 
     if (keyframes.isEmpty())
         return;
 
+    if (!perKeyframeTimingFunctions.contains(0))
+        perKeyframeTimingFunctions.set(0, defaultTimingFunction);
+
     // Remove duplicate keyframes. In CSS the last keyframe at a given offset takes priority.
     std::stable_sort(keyframes.begin(), keyframes.end(), Keyframe::compareOffsets);
     size_t targetIndex = 0;
     for (size_t i = 1; i < keyframes.size(); i++) {
         if (keyframes[i]->offset() != keyframes[targetIndex]->offset())
             targetIndex++;
         if (targetIndex != i)
             keyframes[targetIndex] = keyframes[i];
     }
     keyframes.shrink(targetIndex + 1);
 
-    // Add 0% and 100% keyframes if absent.
-    RefPtr<Keyframe> startKeyframe = keyframes[0];
-    if (startKeyframe->offset()) {
-        startKeyframe = Keyframe::create();
-        startKeyframe->setOffset(0);
-        keyframes.prepend(startKeyframe);
+    // Determine how many keyframes specify each property. Note that we
+    // consider keyframes at offsets of 0 and 1 to always specify all
+    // properties, as values for these properties will be filled in by

[dstockwell, 2013/09/30 07:30:57]

Can't we fill in the end keyframes before counting?

[Steve Block, 2013/09/30 12:17:54]

We could, but in the case that we have to split out any properties, we'd have to
find those newly added property values in the end keyframes and move them to the
new keyframe groups. That approach would eliminate a little code duplication,
but is less efficient and seems an odd way to do it.

Note that in the common case where all non-end keyframes specify all properties,
there's no significant difference between these two approaches.

[dstockwell, 2013/09/30 21:56:44]

It would eliminate lines 973 -> 987, right? I found that section quite
confusing.

I'm not sure being trivially more efficient is worth it for the non-common case,
but it's your call.

[Steve Block, 2013/10/01 04:39:53]

I've updated the patch to handle end keyframes first. We can revert to the
previous approach if we find this approach to be too slow or confusing.

+    // prepareEndKeyframes(). Note that to do this efficiently, we have to do
+    // so after duplicate keyframes have been removed above.
+    typedef HashCountedSet<CSSPropertyID> PropertyCountedSet;
+    PropertyCountedSet propertyCounts;
+    size_t numKeyframes = keyframes.size();
+    for (size_t i = 0; i < numKeyframes; ++i) {
+        const PropertySet& properties = keyframes[i]->properties();
+        for (PropertySet::const_iterator iter = properties.begin(); iter != properties.end(); ++iter)
+            propertyCounts.add(*iter);
     }
-    RefPtr<Keyframe> endKeyframe = keyframes[keyframes.size() - 1];
-    if (endKeyframe->offset() != 1) {
-        endKeyframe = Keyframe::create();
-        endKeyframe->setOffset(1);
-        keyframes.append(endKeyframe);
+
+    bool firstKeyframeIsAtOffsetZero = !keyframes.first()->offset();
+    bool lastKeyframeIsAtOffsetOne = keyframes.last()->offset() == 1;
+    if (firstKeyframeIsAtOffsetZero || lastKeyframeIsAtOffsetOne) {
+        const PropertySet& firstKeyframeProperties = keyframes.first()->properties();
+        const PropertySet& lastKeyframeProperties = keyframes.last()->properties();
+        if (firstKeyframeProperties.size() < propertyCounts.size() || lastKeyframeProperties.size() < propertyCounts.size()) {
+            for (HashCountedSet<CSSPropertyID>::const_iterator iter = propertyCounts.begin(); iter != propertyCounts.end(); ++iter) {
+                CSSPropertyID property = iter->key;
+                if (firstKeyframeIsAtOffsetZero && !keyframes.first()->properties().contains(property))
+                    propertyCounts.add(property);
+                if (lastKeyframeIsAtOffsetOne && !keyframes.last()->properties().contains(property))
+                    propertyCounts.add(property);
+            }
+        }
     }
-    ASSERT(keyframes.size() >= 2);
-    ASSERT(!keyframes.first()->offset());
-    ASSERT(keyframes.last()->offset() == 1);
 
-    // Generate the chained timing function. Note that timing functions apply
-    // from the keyframe in which they're specified to the next keyframe.
-    // FIXME: Handle keyframe sets where some keyframes don't specify all
-    // properties. In this case, timing functions apply between the keyframes
-    // which specify a particular property, so we'll need a separate chained
-    // timing function (and therefore animation) for each property. See
-    // LayoutTests/animations/missing-keyframe-properties-timing-function.html
-    if (!timingFunctions.contains(0))
-        timingFunctions.set(0, defaultTimingFunction);
-    bool isTimingFunctionLinearThroughout = true;
-    RefPtr<ChainedTimingFunction> chainedTimingFunction = ChainedTimingFunction::create();
-    for (size_t i = 0; i < keyframes.size() - 1; ++i) {
-        double lowerBound = keyframes[i]->offset();
-        ASSERT(lowerBound >=0 && lowerBound < 1);
-        double upperBound = keyframes[i + 1]->offset();
-        ASSERT(upperBound > 0 && upperBound <= 1);
-        TimingFunction* timingFunction = timingFunctions.get(lowerBound);
-        ASSERT(timingFunction);
-        isTimingFunctionLinearThroughout &= timingFunction->type() == TimingFunction::LinearFunction;
-        chainedTimingFunction->appendSegment(upperBound, timingFunction);
+    // Split keyframes into groups, where each group contains only keyframes
+    // which specify all properties used in that group. Each group is animated
+    // in a separate animation, to allow per-keyframe timing functions to be
+    // applied correctly.
+    PropertySet propertiesSpecifiedInAllKeyframes;
+    for (PropertyCountedSet::const_iterator iter = propertyCounts.begin(); iter != propertyCounts.end(); ++iter) {
+        const CSSPropertyID property = iter->key;
+        const size_t count = iter->value;
+        ASSERT(count <= numKeyframes);
+        if (count == numKeyframes) {
+            propertiesSpecifiedInAllKeyframes.add(property);
+            continue;
+        }
+        KeyframeAnimationEffect::KeyframeVector splitOutKeyframes;
+        for (size_t i = 0; i < numKeyframes; i++) {
+            Keyframe* keyframe = keyframes[i].get();
+            if (!keyframe->properties().contains(property))
+                continue;
+            RefPtr<Keyframe> clonedKeyframe = Keyframe::create();
+            clonedKeyframe->setOffset(keyframe->offset());
+            clonedKeyframe->setComposite(keyframe->composite());
+            clonedKeyframe->setPropertyValue(property, keyframe->propertyValue(property));
+            splitOutKeyframes.append(clonedKeyframe);
+            keyframe->clearPropertyValue(property);
+        }
+        PropertySet properties;
+        properties.add(property);
+        prepareEndKeyframes(style, properties, splitOutKeyframes);
+        keyframesAndTimingFunctions.append(std::make_pair(splitOutKeyframes, generateTimingFunction(splitOutKeyframes, perKeyframeTimingFunctions)));
     }
-    if (isTimingFunctionLinearThroughout)
-        timingFunction = LinearTimingFunction::create();
-    else
-        timingFunction = chainedTimingFunction;
-
-    // Snapshot current property values for 0% and 100% if missing.
-    PropertySet allProperties;
-    for (size_t i = 0; i < keyframes.size(); i++) {
-        const PropertySet& keyframeProperties = keyframes[i]->properties();
-        for (PropertySet::const_iterator iter = keyframeProperties.begin(); iter != keyframeProperties.end(); ++iter)
-            allProperties.add(*iter);
-    }
-    const PropertySet& startKeyframeProperties = startKeyframe->properties();
-    const PropertySet& endKeyframeProperties = endKeyframe->properties();
-    bool missingStartValues = startKeyframeProperties.size() < allProperties.size();
-    bool missingEndValues = endKeyframeProperties.size() < allProperties.size();
-    if (!missingStartValues && !missingEndValues)
-        return;
-    for (PropertySet::const_iterator iter = allProperties.begin(); iter != allProperties.end(); ++iter) {
-        const CSSPropertyID property = *iter;
-        bool startNeedsValue = missingStartValues && !startKeyframeProperties.contains(property);
-        bool endNeedsValue = missingEndValues && !endKeyframeProperties.contains(property);
-        if (!startNeedsValue && !endNeedsValue)
-            continue;
-        RefPtr<AnimatableValue> snapshotValue = CSSAnimatableValueFactory::create(property, style);
-        if (startNeedsValue)
-            startKeyframe->setPropertyValue(property, snapshotValue.get());
-        if (endNeedsValue)
-            endKeyframe->setPropertyValue(property, snapshotValue.get());
-    }
-    ASSERT(startKeyframe->properties().size() == allProperties.size());
-    ASSERT(endKeyframe->properties().size() == allProperties.size());
+#ifndef NDEBUG
+    size_t numPropertiesSpecifiedInAllKeyframes = propertiesSpecifiedInAllKeyframes.size();
+    for (size_t i = 0; i < keyframes.size(); ++i)
+        ASSERT(!keyframes[i]->offset() || keyframes[i]->offset() == 1 || keyframes[i]->properties().size() == numPropertiesSpecifiedInAllKeyframes);
+#endif
+    prepareEndKeyframes(style, propertiesSpecifiedInAllKeyframes, keyframes);
+    keyframesAndTimingFunctions.append(std::make_pair(keyframes, generateTimingFunction(keyframes, perKeyframeTimingFunctions)));

[dstockwell, 2013/09/30 07:30:57]

We probably don't want to do this if numPropertiesSpecifiedInAllKeyframes is 0
and keyframesAndTimingFunctions is not empty.

[Steve Block, 2013/09/30 12:17:54]

Good point. Done.

 }
 
 PassRefPtr<RenderStyle> StyleResolver::pseudoStyleForElement(Element* e, const PseudoStyleRequest& pseudoStyleRequest, RenderStyle* parentStyle)
 {
     ASSERT(document().frame());
     ASSERT(documentSettings());
     ASSERT(parentStyle);
     if (!e)
         return 0;
 
@@ skipping 637 matching lines @@
         m_matchedPropertiesSearches, m_matchedPropertiesHit, m_matchedPropertiesSharedInheritedHit, m_matchedPropertiesToCache, m_matchedPropertiesEnteredIntoCache);
 
     fprintf(stderr, "Total:\n");
     printStyleStats(m_totalSearches, m_totalElementsEligibleForSharing, m_totalStylesShared, m_totalSearchFoundSiblingForSharing, m_totalSearchesMissedSharing,
         m_totalMatchedPropertiesSearches, m_totalMatchedPropertiesHit, m_totalMatchedPropertiesSharedInheritedHit, m_totalMatchedPropertiesToCache, m_totalMatchedPropertiesEnteredIntoCache);
     fprintf(stderr, "--------------------------------------------------------------------------------\n");
 }
 #endif
 
 } // namespace WebCore