WebKit Bugzilla
Attachment 343062 Details for
Bug 183818
: [Web Animations] Make imported/mozilla/css-animations/test_pseudoElement-get-animations.html pass reliably
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[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:
Patch
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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
dino
:
review+
ews-watchlist
:
commit-queue-
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 183818
:
343056
| 343062 |
343081