Changeset 233004 in webkit

Timestamp:

Jun 20, 2018 12:07:34 AM (6 years ago)

Author:

graouts@webkit.org

Message:

[Web Animations] Make imported/mozilla/css-animations/test_pseudoElement-get-animations.html pass reliably
​https://bugs.webkit.org/show_bug.cgi?id=183818
<rdar://problem/40997015>

Reviewed by Dean Jackson.

LayoutTests/imported/mozilla:

Mark progressions in the Mozilla CSS Transitions and CSS Animations tests.

• css-animations/test_pseudoElement-get-animations-expected.txt:
• css-transitions/test_element-get-animations-expected.txt:

Source/WebCore:

We add a new opt-in flag to return sorted animations when calling AnimationTimeline::animationsForElement() so that
Element::getAnimations() can opt into returning animations sorted by the rules defined by the CSS Transitions spec.
The rule is that CSS Transitions must be sorted prior to CSS Animations and regular Web Animations, and then sorted
by the time they were generated and, if generated at the same time, sorted alphabetically based on the transitioned
property. To be able to accomodate this, we add a new generationTime() method to CSSTransition.

• animation/AnimationTimeline.cpp:

(WebCore::AnimationTimeline::animationsForElement const):
(WebCore::AnimationTimeline::updateCSSTransitionsForElement):

• animation/AnimationTimeline.h:
• animation/CSSTransition.cpp:

(WebCore::CSSTransition::create):
(WebCore::CSSTransition::CSSTransition):

• animation/CSSTransition.h:
• dom/Element.cpp:

(WebCore::Element::getAnimations):

LayoutTests:

This test now passes reliably.

• TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/imported/mozilla/ChangeLog (modified) (1 diff)
• LayoutTests/imported/mozilla/css-animations/test_pseudoElement-get-animations-expected.txt (modified) (1 diff)
• LayoutTests/imported/mozilla/css-transitions/test_element-get-animations-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/animation/AnimationTimeline.cpp (modified) (5 diffs)
• Source/WebCore/animation/AnimationTimeline.h (modified) (1 diff)
• Source/WebCore/animation/CSSTransition.cpp (modified) (2 diffs)
• Source/WebCore/animation/CSSTransition.h (modified) (3 diffs)
• Source/WebCore/dom/Element.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-06-19  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Make imported/mozilla/css-animations/test_pseudoElement-get-animations.html pass reliably
+        https://bugs.webkit.org/show_bug.cgi?id=183818
+        <rdar://problem/40997015>
+
+        Reviewed by Dean Jackson.
+
+        This test now passes reliably.
+
+        * TestExpectations:
+
 2018-06-19  Per Arne Vollan  <pvollan@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -1927 +1927 @@
 webkit.org/b/181888 imported/w3c/web-platform-tests/web-animations/timing-model/animation-effects/current-iteration.html [ Pass Failure ]
 
-webkit.org/b/183818 imported/mozilla/css-animations/test_pseudoElement-get-animations.html [ Pass Failure Timeout ]
 webkit.org/b/183826 imported/mozilla/css-animations/test_animation-pausing.html [ Pass Failure Timeout ]
 webkit.org/b/183830 imported/mozilla/css-animations/test_animation-ready.html [ Pass Failure Timeout ]

trunk/LayoutTests/imported/mozilla/ChangeLog

@@ +1 @@
+2018-06-19  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Make imported/mozilla/css-animations/test_pseudoElement-get-animations.html pass reliably
+        https://bugs.webkit.org/show_bug.cgi?id=183818
+        <rdar://problem/40997015>
+
+        Reviewed by Dean Jackson.
+
+        Mark progressions in the Mozilla CSS Transitions and CSS Animations tests.
+
+        * css-animations/test_pseudoElement-get-animations-expected.txt:
+        * css-transitions/test_element-get-animations-expected.txt:
+
 2018-06-19  Antoine Quint  <graouts@apple.com>
 

trunk/LayoutTests/imported/mozilla/css-animations/test_pseudoElement-get-animations-expected.txt

@@ -1 +1 @@
 
 PASS getAnimations returns CSSAnimation objects 
-FAIL getAnimations returns css transitions/animations, and script-generated animations in the expected order assert_equals: Got expected number of animations/trnasitions running on ::after pseudo element expected 5 but got 2
+PASS getAnimations returns css transitions/animations, and script-generated animations in the expected order 
 

trunk/LayoutTests/imported/mozilla/css-transitions/test_element-get-animations-expected.txt

@@ -5 +5 @@
 FAIL getAnimations for transition on non-animatable property assert_equals: getAnimations returns an empty sequence for a transition of a non-animatable property expected 0 but got 1
 PASS getAnimations for transition on unsupported property 
-FAIL getAnimations sorts simultaneous transitions by name assert_equals: expected "border-bottom-width" but got "border-left-width"
+PASS getAnimations sorts simultaneous transitions by name 
 PASS getAnimations sorts transitions by when they were generated 
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-06-19  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Make imported/mozilla/css-animations/test_pseudoElement-get-animations.html pass reliably
+        https://bugs.webkit.org/show_bug.cgi?id=183818
+        <rdar://problem/40997015>
+
+        Reviewed by Dean Jackson.
+
+        We add a new opt-in flag to return sorted animations when calling AnimationTimeline::animationsForElement() so that
+        Element::getAnimations() can opt into returning animations sorted by the rules defined by the CSS Transitions spec.
+        The rule is that CSS Transitions must be sorted prior to CSS Animations and regular Web Animations, and then sorted
+        by the time they were generated and, if generated at the same time, sorted alphabetically based on the transitioned
+        property. To be able to accomodate this, we add a new generationTime() method to CSSTransition.
+
+        * animation/AnimationTimeline.cpp:
+        (WebCore::AnimationTimeline::animationsForElement const):
+        (WebCore::AnimationTimeline::updateCSSTransitionsForElement):
+        * animation/AnimationTimeline.h:
+        * animation/CSSTransition.cpp:
+        (WebCore::CSSTransition::create):
+        (WebCore::CSSTransition::CSSTransition):
+        * animation/CSSTransition.h:
+        * dom/Element.cpp:
+        (WebCore::Element::getAnimations):
+
 2018-06-19  Basuke Suzuki  <Basuke.Suzuki@sony.com>
 

trunk/Source/WebCore/animation/AnimationTimeline.cpp

@@ -143 +143 @@
 }
 
-Vector<RefPtr<WebAnimation>> AnimationTimeline::animationsForElement(Element& element) const
+Vector<RefPtr<WebAnimation>> AnimationTimeline::animationsForElement(Element& element, Ordering ordering) const
 {
     Vector<RefPtr<WebAnimation>> animations;
     if (m_elementToCSSTransitionsMap.contains(&element)) {
         const auto& cssTransitions = m_elementToCSSTransitionsMap.get(&element);
-        animations.appendRange(cssTransitions.begin(), cssTransitions.end());
+        if (ordering == Ordering::Sorted) {
+            Vector<RefPtr<WebAnimation>> sortedCSSTransitions;
+            sortedCSSTransitions.appendRange(cssTransitions.begin(), cssTransitions.end());
+            std::sort(sortedCSSTransitions.begin(), sortedCSSTransitions.end(), [](auto& lhs, auto& rhs) {
+                // Sort transitions first by their generation time, and then by transition-property.
+                // https://drafts.csswg.org/css-transitions-2/#animation-composite-order
+                auto* lhsTransition = downcast<CSSTransition>(lhs.get());
+                auto* rhsTransition = downcast<CSSTransition>(rhs.get());
+                if (lhsTransition->generationTime() != rhsTransition->generationTime())
+                    return lhsTransition->generationTime() < rhsTransition->generationTime();
+                return lhsTransition->transitionProperty().utf8() < rhsTransition->transitionProperty().utf8();
+            });
+            animations.appendVector(sortedCSSTransitions);
+        } else
+            animations.appendRange(cssTransitions.begin(), cssTransitions.end());
     }
     if (m_elementToCSSAnimationsMap.contains(&element)) {
@@ -301 +315 @@
     }).iterator->value;
 
+    auto generationTime = MonotonicTime::now();
+
     auto numberOfProperties = CSSPropertyAnimation::getNumProperties();
     for (int propertyIndex = 0; propertyIndex < numberOfProperties; ++propertyIndex) {
@@ -349 +365 @@
             auto& reversingAdjustedStartStyle = beforeChangeStyle;
             auto reversingShorteningFactor = 1;
-            runningTransitionsByProperty.set(property, CSSTransition::create(element, property, *matchingBackingAnimation, &beforeChangeStyle, afterChangeStyle, delay, duration, reversingAdjustedStartStyle, reversingShorteningFactor));
+            runningTransitionsByProperty.set(property, CSSTransition::create(element, property, generationTime, *matchingBackingAnimation, &beforeChangeStyle, afterChangeStyle, delay, duration, reversingAdjustedStartStyle, reversingShorteningFactor));
         } else if (completedTransitionsByProperty.contains(property) && !propertyInStyleMatchesValueForTransitionInMap(property, afterChangeStyle, completedTransitionsByProperty)) {
             // 2. Otherwise, if the element has a completed transition for the property and the end value of the completed transition is different from
@@ -402 +418 @@
                 auto delay = matchingBackingAnimation->delay() < 0 ? Seconds(matchingBackingAnimation->delay()) * reversingShorteningFactor : Seconds(matchingBackingAnimation->delay());
                 auto duration = Seconds(matchingBackingAnimation->duration()) * reversingShorteningFactor;
-                runningTransitionsByProperty.set(property, CSSTransition::create(element, property, *matchingBackingAnimation, &previouslyRunningTransitionCurrentStyle, afterChangeStyle, delay, duration, reversingAdjustedStartStyle, reversingShorteningFactor));
+                runningTransitionsByProperty.set(property, CSSTransition::create(element, property, generationTime, *matchingBackingAnimation, &previouslyRunningTransitionCurrentStyle, afterChangeStyle, delay, duration, reversingAdjustedStartStyle, reversingShorteningFactor));
             } else {
                 // 4. Otherwise, implementations must cancel the running transition
@@ -418 +434 @@
                 auto& reversingAdjustedStartStyle = currentStyle;
                 auto reversingShorteningFactor = 1;
-                runningTransitionsByProperty.set(property, CSSTransition::create(element, property, *matchingBackingAnimation, &previouslyRunningTransitionCurrentStyle, afterChangeStyle, delay, duration, reversingAdjustedStartStyle, reversingShorteningFactor));
+                runningTransitionsByProperty.set(property, CSSTransition::create(element, property, generationTime, *matchingBackingAnimation, &previouslyRunningTransitionCurrentStyle, afterChangeStyle, delay, duration, reversingAdjustedStartStyle, reversingShorteningFactor));
             }
         }

trunk/Source/WebCore/animation/AnimationTimeline.h

@@ -58 +58 @@
     virtual void timingModelDidChange() { };
 
+    enum class Ordering { Sorted, Unsorted };
     const ListHashSet<RefPtr<WebAnimation>>& animations() const { return m_animations; }
-    Vector<RefPtr<WebAnimation>> animationsForElement(Element&) const;
+    Vector<RefPtr<WebAnimation>> animationsForElement(Element&, Ordering ordering = Ordering::Unsorted) const;
     void removeAnimationsForElement(Element&);
     void cancelDeclarativeAnimationsForElement(Element&);

trunk/Source/WebCore/animation/CSSTransition.cpp

@@ -33 +33 @@
 namespace WebCore {
 
-Ref<CSSTransition> CSSTransition::create(Element& target, CSSPropertyID property, const Animation& backingAnimation, const RenderStyle* oldStyle, const RenderStyle& newStyle, Seconds delay, Seconds duration, const RenderStyle& reversingAdjustedStartStyle, double reversingShorteningFactor)
+Ref<CSSTransition> CSSTransition::create(Element& target, CSSPropertyID property, MonotonicTime generationTime, const Animation& backingAnimation, const RenderStyle* oldStyle, const RenderStyle& newStyle, Seconds delay, Seconds duration, const RenderStyle& reversingAdjustedStartStyle, double reversingShorteningFactor)
 {
-    auto result = adoptRef(*new CSSTransition(target, property, backingAnimation, newStyle, reversingAdjustedStartStyle, reversingShorteningFactor));
+    auto result = adoptRef(*new CSSTransition(target, property, generationTime, backingAnimation, newStyle, reversingAdjustedStartStyle, reversingShorteningFactor));
     result->initialize(target, oldStyle, newStyle);
     result->setTimingProperties(delay, duration);
@@ -41 +41 @@
 }
 
-CSSTransition::CSSTransition(Element& element, CSSPropertyID property, const Animation& backingAnimation, const RenderStyle& targetStyle, const RenderStyle& reversingAdjustedStartStyle, double reversingShorteningFactor)
+CSSTransition::CSSTransition(Element& element, CSSPropertyID property, MonotonicTime generationTime, const Animation& backingAnimation, const RenderStyle& targetStyle, const RenderStyle& reversingAdjustedStartStyle, double reversingShorteningFactor)
     : DeclarativeAnimation(element, backingAnimation)
     , m_property(property)
+    , m_generationTime(generationTime)
     , m_targetStyle(RenderStyle::clonePtr(targetStyle))
     , m_reversingAdjustedStartStyle(RenderStyle::clonePtr(reversingAdjustedStartStyle))

trunk/Source/WebCore/animation/CSSTransition.h

@@ -38 +38 @@
 class CSSTransition final : public DeclarativeAnimation {
 public:
-    static Ref<CSSTransition> create(Element&, CSSPropertyID, const Animation&, const RenderStyle* oldStyle, const RenderStyle& newStyle, Seconds delay, Seconds duration, const RenderStyle& reversingAdjustedStartStyle, double);
+    static Ref<CSSTransition> create(Element&, CSSPropertyID, MonotonicTime generationTime, const Animation&, const RenderStyle* oldStyle, const RenderStyle& newStyle, Seconds delay, Seconds duration, const RenderStyle& reversingAdjustedStartStyle, double);
     ~CSSTransition() = default;
 
@@ -44 +44 @@
     String transitionProperty() const { return getPropertyNameString(m_property); }
     CSSPropertyID property() const { return m_property; }
+    MonotonicTime generationTime() const { return m_generationTime; }
     const RenderStyle& targetStyle() const { return *m_targetStyle; }
     const RenderStyle& currentStyle() const { return *m_currentStyle; }
@@ -53 +54 @@
 
 private:
-    CSSTransition(Element&, CSSPropertyID, const Animation&, const RenderStyle& targetStyle, const RenderStyle& reversingAdjustedStartStyle, double);
+    CSSTransition(Element&, CSSPropertyID, MonotonicTime generationTime, const Animation&, const RenderStyle& targetStyle, const RenderStyle& reversingAdjustedStartStyle, double);
     void setTimingProperties(Seconds delay, Seconds duration);
 
     CSSPropertyID m_property;
+    MonotonicTime m_generationTime;
     std::unique_ptr<RenderStyle> m_targetStyle;
     std::unique_ptr<RenderStyle> m_currentStyle;

trunk/Source/WebCore/dom/Element.cpp

@@ -3834 +3834 @@
     Vector<RefPtr<WebAnimation>> animations;
     if (auto timeline = document().existingTimeline()) {
-        for (auto& animation : timeline->animationsForElement(*this)) {
+        for (auto& animation : timeline->animationsForElement(*this, AnimationTimeline::Ordering::Sorted)) {
             if (animation->canBeListed())
                 animations.append(animation);