Attachment 343062 Details for Bug 183818 – Patch

[patch] Patch

bug-183818-20180619183127.patch (text/plain), 18.04 KB, created by Antoine Quint on 2018-06-19 09:31:28 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antoine Quint

Created: 2018-06-19 09:31:28 PDT

Size: 18.04 KB

patch

obsolete

>Subversion Revision: 232961
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 407eb59039e274f54c7cf8f9f4621dfdcd2173b7..4f4e1ed378e7ef54da4fc93f21e0ab755d095d3c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,28 @@
>+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 NOBODY (OOPS!).
>+
>+        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  Antoine Quint  <graouts@apple.com>
> 
>         [Web Animations] Make imported/mozilla/css-transitions/test_animation-cancel.html pass reliably
>diff --git a/Source/WebCore/animation/AnimationTimeline.cpp b/Source/WebCore/animation/AnimationTimeline.cpp
>index 52fc1a00892bfa141762e6eab506d03aebdd0ac6..7c3fe0d1f5a8aed4cd5d564ee2ace7b9bf08da1c 100644
>--- a/Source/WebCore/animation/AnimationTimeline.cpp
>+++ b/Source/WebCore/animation/AnimationTimeline.cpp
>@@ -142,12 +142,26 @@ void AnimationTimeline::animationWasRemovedFromElement(WebAnimation& animation,
>     }
> }
> 
>-Vector<RefPtr<WebAnimation>> AnimationTimeline::animationsForElement(Element& element) const
>+Vector<RefPtr<WebAnimation>> AnimationTimeline::animationsForElement(Element& element, bool sorted) const
> {
>     Vector<RefPtr<WebAnimation>> animations;
>     if (m_elementToCSSTransitionsMap.contains(&element)) {
>         const auto& cssTransitions = m_elementToCSSTransitionsMap.get(&element);
>-        animations.appendRange(cssTransitions.begin(), cssTransitions.end());
>+        if (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->transitionProperty().utf8() < rhsTransition->transitionProperty().utf8();
>+                return lhsTransition->generationTime() < rhsTransition->generationTime();
>+            });
>+            animations.appendVector(sortedCSSTransitions);
>+        } else
>+            animations.appendRange(cssTransitions.begin(), cssTransitions.end());
>     }
>     if (m_elementToCSSAnimationsMap.contains(&element)) {
>         const auto& cssAnimations = m_elementToCSSAnimationsMap.get(&element);
>@@ -300,6 +314,8 @@ void AnimationTimeline::updateCSSTransitionsForElement(Element& element, const R
>         return HashMap<CSSPropertyID, RefPtr<CSSTransition>> { };
>     }).iterator->value;
> 
>+    auto generationTime = MonotonicTime::now();
>+
>     auto numberOfProperties = CSSPropertyAnimation::getNumProperties();
>     for (int propertyIndex = 0; propertyIndex < numberOfProperties; ++propertyIndex) {
>         bool isShorthand;
>@@ -348,7 +364,7 @@ void AnimationTimeline::updateCSSTransitionsForElement(Element& element, const R
>             auto duration = Seconds(matchingBackingAnimation->duration());
>             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
>             //    the after-change style for the property, then implementations must remove the completed transition from the set of completed transitions.
>@@ -401,7 +417,7 @@ void AnimationTimeline::updateCSSTransitionsForElement(Element& element, const R
>                 auto reversingShorteningFactor = std::max(std::min(((transformedProgress * previouslyRunningTransition->reversingShorteningFactor()) + (1 - previouslyRunningTransition->reversingShorteningFactor())), 1.0), 0.0);
>                 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
>                 previouslyRunningTransition->cancel();
>@@ -417,7 +433,7 @@ void AnimationTimeline::updateCSSTransitionsForElement(Element& element, const R
>                 auto duration = Seconds(matchingBackingAnimation->duration());
>                 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));
>             }
>         }
>     }
>diff --git a/Source/WebCore/animation/AnimationTimeline.h b/Source/WebCore/animation/AnimationTimeline.h
>index 139272ca941bcc0eb83e37599a404de5281f212b..dfaba8eb6a3066358624570fa3a2f7c5919d6f76 100644
>--- a/Source/WebCore/animation/AnimationTimeline.h
>+++ b/Source/WebCore/animation/AnimationTimeline.h
>@@ -58,7 +58,7 @@ public:
>     virtual void timingModelDidChange() { };
> 
>     const ListHashSet<RefPtr<WebAnimation>>& animations() const { return m_animations; }
>-    Vector<RefPtr<WebAnimation>> animationsForElement(Element&) const;
>+    Vector<RefPtr<WebAnimation>> animationsForElement(Element&, bool sorted = false) const;
>     void removeAnimationsForElement(Element&);
>     void cancelDeclarativeAnimationsForElement(Element&);
>     void animationWasAddedToElement(WebAnimation&, Element&);
>diff --git a/Source/WebCore/animation/CSSTransition.cpp b/Source/WebCore/animation/CSSTransition.cpp
>index 30f2262c32a7a8f26fd975cd07a2467a4a410db3..509c578d102a85b47613e20cca0116250f6e739e 100644
>--- a/Source/WebCore/animation/CSSTransition.cpp
>+++ b/Source/WebCore/animation/CSSTransition.cpp
>@@ -32,17 +32,18 @@
> 
> 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);
>     return result;
> }
> 
>-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))
>     , m_reversingShorteningFactor(reversingShorteningFactor)
>diff --git a/Source/WebCore/animation/CSSTransition.h b/Source/WebCore/animation/CSSTransition.h
>index a933313f18379d05e0f96f9f14130bcef8c88c5e..3b9729b24c8e06b6ecb86a1eaa4d5616b063fc32 100644
>--- a/Source/WebCore/animation/CSSTransition.h
>+++ b/Source/WebCore/animation/CSSTransition.h
>@@ -37,12 +37,13 @@ class RenderStyle;
> 
> 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;
> 
>     bool isCSSTransition() const override { return true; }
>     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; }
>     const RenderStyle& reversingAdjustedStartStyle() const { return *m_reversingAdjustedStartStyle; }
>@@ -52,10 +53,11 @@ public:
>     void resolve(RenderStyle&) final;
> 
> 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;
>     std::unique_ptr<RenderStyle> m_reversingAdjustedStartStyle;
>diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
>index a6b1f05be35a2d75107ebb2408c0761ef9566bc7..4035986df33e1cba49c24f28c73aec5a58173b48 100644
>--- a/Source/WebCore/dom/Element.cpp
>+++ b/Source/WebCore/dom/Element.cpp
>@@ -3833,7 +3833,7 @@ Vector<RefPtr<WebAnimation>> Element::getAnimations()
> 
>     Vector<RefPtr<WebAnimation>> animations;
>     if (auto timeline = document().existingTimeline()) {
>-        for (auto& animation : timeline->animationsForElement(*this)) {
>+        for (auto& animation : timeline->animationsForElement(*this, true)) {
>             if (animation->canBeListed())
>                 animations.append(animation);
>         }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index c643ed625760f4f91b0cb7a7b431d90be3629f3c..dd9e4f1ea5c28152f75bb6274789f1338e9aa469 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+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 NOBODY (OOPS!).
>+
>+        This test now passes reliably.
>+
>+        * TestExpectations:
>+
> 2018-06-19  Antoine Quint  <graouts@apple.com>
> 
>         Layout Test imported/mozilla/css-animations/test_animation-cancel.html is a flaky failure
>diff --git a/LayoutTests/imported/mozilla/ChangeLog b/LayoutTests/imported/mozilla/ChangeLog
>index bd26ef0ecd6554f2577d74a508bbdfbb8a2b4a29..740da7af16151bc351be69f817971564b111d2a7 100644
>--- a/LayoutTests/imported/mozilla/ChangeLog
>+++ b/LayoutTests/imported/mozilla/ChangeLog
>@@ -1,3 +1,16 @@
>+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 NOBODY (OOPS!).
>+
>+        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>
> 
>         [Web Animations] Make imported/mozilla/css-transitions/test_animation-cancel.html pass reliably
>diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
>index 9c2c893bf40e09e2b9d46e0d3e1c6f199c4bbb93..ff800edea0d8cf3cd76e46315aad6f3295395035 100644
>--- a/LayoutTests/TestExpectations
>+++ b/LayoutTests/TestExpectations
>@@ -1926,7 +1926,6 @@ webkit.org/b/181122 imported/w3c/web-platform-tests/web-animations/interfaces/An
> webkit.org/b/181123 imported/w3c/web-platform-tests/web-animations/interfaces/Animation/finish.html [ Pass Failure ]
> 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/183828 imported/mozilla/css-animations/test_animation-playstate.html [ Pass Failure Timeout ]
> webkit.org/b/183830 imported/mozilla/css-animations/test_animation-ready.html [ Pass Failure Timeout ]
>diff --git a/LayoutTests/imported/mozilla/css-animations/test_pseudoElement-get-animations-expected.txt b/LayoutTests/imported/mozilla/css-animations/test_pseudoElement-get-animations-expected.txt
>index 3b101639bc522bd3297474d96c5223dbf003e981..18122b4be2d472123328846cb8c7396f61f246d5 100644
>--- a/LayoutTests/imported/mozilla/css-animations/test_pseudoElement-get-animations-expected.txt
>+++ b/LayoutTests/imported/mozilla/css-animations/test_pseudoElement-get-animations-expected.txt
>@@ -1,4 +1,4 @@
> 
> 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 
> 
>diff --git a/LayoutTests/imported/mozilla/css-transitions/test_element-get-animations-expected.txt b/LayoutTests/imported/mozilla/css-transitions/test_element-get-animations-expected.txt
>index 7cbf626b13a69b3557d783a749ecab610c8c07bf..9c796e00d029db0b5980b92f0e70d74ea778e551 100644
>--- a/LayoutTests/imported/mozilla/css-transitions/test_element-get-animations-expected.txt
>+++ b/LayoutTests/imported/mozilla/css-transitions/test_element-get-animations-expected.txt
>@@ -4,6 +4,6 @@ PASS getAnimations returns CSSTransition objects for CSS Transitions
> PASS getAnimations for CSS Transitions that have finished 
> 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 
>

Flags:

dino: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 183818: 343056 | 343062 | 343081