Changeset 201818 in webkit for trunk/Source/WebCore/css/StyleResolver.cpp

Timestamp:

Jun 8, 2016, 11:46:43 AM (9 years ago)

Author:

dino@apple.com

Message:

Multiple selectors break keyframes animation
​https://bugs.webkit.org/show_bug.cgi?id=158199
<rdar://problem/26652591>

Reviewed by Simon Fraser.

Source/WebCore:

If we came across a duplicate key entry in a keyframe, we
were replacing the existing entry, instead of merging.

Test: animations/duplicate-keys.html

• css/CSSKeyframeRule.h:

(WebCore::StyleKeyframe::setKey): Add a way to set the key of a rule
as a number, rather than going through a string and the CSS parser.

• css/StyleResolver.cpp:

(WebCore::StyleResolver::keyframeStylesForAnimation): Check if the rule
has duplicates, and if it does, merge all the common entries.

• rendering/style/KeyframeList.cpp:

(WebCore::KeyframeList::insert): Now that we've removed duplicates at
the processing time, we should never come across a duplicate while
building this list.

LayoutTests:

• animations/duplicate-keys-expected.html: Added.
• animations/duplicate-keys.html: Added.

File:

• trunk/Source/WebCore/css/StyleResolver.cpp (modified) (5 diffs)

trunk/Source/WebCore/css/StyleResolver.cpp

@@ -514 +514 @@
     list.clear();
 
-    // Get the keyframesRule for this name
+    // Get the keyframesRule for this name.
     if (list.animationName().isEmpty())
         return;
@@ -526 +526 @@
     const StyleRuleKeyframes* keyframesRule = it->value.get();
 
-    // Construct and populate the style for each keyframe
-    for (auto& keyframe : keyframesRule->keyframes()) {
-        // Apply the declaration to the style. This is a simplified version of the logic in styleForElement
+    auto* keyframes = &keyframesRule->keyframes();
+    Vector<Ref<StyleKeyframe>> newKeyframesIfNecessary;
+
+    bool hasDuplicateKeys;
+    HashSet<double> keyframeKeys;
+    for (auto& keyframe : *keyframes) {
+        for (auto key : keyframe->keys()) {
+            if (!keyframeKeys.add(key)) {
+                hasDuplicateKeys = true;
+                break;
+            }
+        }
+        if (hasDuplicateKeys)
+            break;
+    }
+
+    // FIXME: If HashMaps could have Ref<> as value types, we wouldn't need
+    // to copy the HashMap into a Vector.
+    if (hasDuplicateKeys) {
+        // Merge duplicate key times.
+        HashMap<double, RefPtr<StyleKeyframe>> keyframesMap;
+
+        for (auto& originalKeyframe : keyframesRule->keyframes()) {
+            for (auto key : originalKeyframe->keys()) {
+                if (auto keyframe = keyframesMap.get(key))
+                    keyframe->mutableProperties().mergeAndOverrideOnConflict(originalKeyframe->properties());
+                else {
+                    auto styleKeyframe = StyleKeyframe::create(MutableStyleProperties::create());
+                    styleKeyframe.ptr()->setKey(key);
+                    styleKeyframe.ptr()->mutableProperties().mergeAndOverrideOnConflict(originalKeyframe->properties());
+                    keyframesMap.set(key, styleKeyframe.ptr());
+                }
+            }
+        }
+
+        for (auto& keyframe : keyframesMap.values())
+            newKeyframesIfNecessary.append(*keyframe.get());
+
+        keyframes = &newKeyframesIfNecessary;
+    }
+
+    // Construct and populate the style for each keyframe.
+    for (auto& keyframe : *keyframes) {
+        // Apply the declaration to the style. This is a simplified version of the logic in styleForElement.
         m_state = State(element, nullptr);
 
         // Add this keyframe style to all the indicated key times
-        for (auto& key : keyframe->keys()) {
+        for (auto key : keyframe->keys()) {
             KeyframeValue keyframeValue(0, nullptr);
             keyframeValue.setStyle(styleForKeyframe(elementStyle, keyframe.ptr(), keyframeValue));
@@ -540 +581 @@
     }
 
-    // If the 0% keyframe is missing, create it (but only if there is at least one other keyframe)
+    // If the 0% keyframe is missing, create it (but only if there is at least one other keyframe).
     int initialListSize = list.size();
     if (initialListSize > 0 && list[0].key()) {
@@ -546 +587 @@
         if (!zeroPercentKeyframe) {
             zeroPercentKeyframe = &StyleKeyframe::create(MutableStyleProperties::create()).leakRef();
-            zeroPercentKeyframe->setKeyText("0%");
+            zeroPercentKeyframe->setKey(0);
         }
         KeyframeValue keyframeValue(0, nullptr);
@@ -553 +594 @@
     }
 
-    // If the 100% keyframe is missing, create it (but only if there is at least one other keyframe)
+    // If the 100% keyframe is missing, create it (but only if there is at least one other keyframe).
     if (initialListSize > 0 && (list[list.size() - 1].key() != 1)) {
         static StyleKeyframe* hundredPercentKeyframe;
         if (!hundredPercentKeyframe) {
             hundredPercentKeyframe = &StyleKeyframe::create(MutableStyleProperties::create()).leakRef();
-            hundredPercentKeyframe->setKeyText("100%");
+            hundredPercentKeyframe->setKey(1);
         }
         KeyframeValue keyframeValue(1, nullptr);