- Revision
- 261217
- Author
- [email protected]
- Date
- 2020-05-06 01:06:03 -0700 (Wed, 06 May 2020)
Log Message
Fix animation ordering to make imported/w3c/web-platform-tests/css/css-animations/Element-getAnimations.tentative.html pass
https://bugs.webkit.org/show_bug.cgi?id=211468
<rdar://problem/62732578>
Reviewed by David Kilzer.
LayoutTests/imported/w3c:
Mark the final two failures in imported/w3c/web-platform-tests/css/css-animations/Element-getAnimations.tentative.html as PASS.
* web-platform-tests/css/css-animations/Element-getAnimations.tentative-expected.txt:
Source/WebCore:
The "Animation composite order" section of the CSS Animations Level 2 specification (https://drafts.csswg.org/css-animations-2/#animation-composite-order)
defines the relative composite order of animations. We bake this into compareAnimationsByCompositeOrder(), but this function would not yield consistent
results if it is called in a non-stable sort, because if both CSSAnimation objects passed to this function have the same backing Animation object, they
would not return the same value if passed in a different order. The Web Animations spec always ensures that procedures that sort using the composite
order are called as part of a stable sort. So we change all call sites to use std::stable_sort and add an assertion in case we have two CSSAnimation
objects with the same backing Animation objects to catch cases like this in the future.
Finally, since we already know only relevant animations can find their way into the output of Document::getAnimations(), we also ensure we iterate over
m_animations (which holds only relevant animations) rather than m_allAnimations (which may not).
* animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::getAnimations const):
* animation/KeyframeEffectStack.cpp:
(WebCore::KeyframeEffectStack::ensureEffectsAreSorted):
* animation/WebAnimationUtilities.cpp:
(WebCore::compareAnimationsByCompositeOrder):
Modified Paths
Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (261216 => 261217)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2020-05-06 06:51:50 UTC (rev 261216)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2020-05-06 08:06:03 UTC (rev 261217)
@@ -1,3 +1,15 @@
+2020-05-05 Antoine Quint <[email protected]>
+
+ Fix animation ordering to make imported/w3c/web-platform-tests/css/css-animations/Element-getAnimations.tentative.html pass
+ https://bugs.webkit.org/show_bug.cgi?id=211468
+ <rdar://problem/62732578>
+
+ Reviewed by David Kilzer.
+
+ Mark the final two failures in imported/w3c/web-platform-tests/css/css-animations/Element-getAnimations.tentative.html as PASS.
+
+ * web-platform-tests/css/css-animations/Element-getAnimations.tentative-expected.txt:
+
2020-05-05 Rob Buis <[email protected]>
Fix setting host on URL when no port is specified
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/Element-getAnimations.tentative-expected.txt (261216 => 261217)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/Element-getAnimations.tentative-expected.txt 2020-05-06 06:51:50 UTC (rev 261216)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/Element-getAnimations.tentative-expected.txt 2020-05-06 08:06:03 UTC (rev 261217)
@@ -19,6 +19,6 @@
PASS { subtree: false } on a leaf element returns the element's animations and ignore pseudo-elements
PASS { subtree: true } on a leaf element returns the element's animations and its pseudo-elements' animations
PASS { subtree: false } on an element with a child returns only the element's animations
-FAIL { subtree: true } on an element with a child returns animations from the element, its pseudo-elements, its child and its child pseudo-elements assert_equals: The animation targeting the ::after pesudo-element should be returned third expected (string) "::after" but got (object) null
-FAIL { subtree: true } on an element with many descendants returns animations from all the descendants assert_equals: The animation targeting the child1 element should be returned second expected Element node <div id="child1" style="animation: anim1 100s;"><div id="... but got Element node <div id="grandchild1" style="animation: anim1 100s;"></div>
+PASS { subtree: true } on an element with a child returns animations from the element, its pseudo-elements, its child and its child pseudo-elements
+PASS { subtree: true } on an element with many descendants returns animations from all the descendants
Modified: trunk/Source/WebCore/ChangeLog (261216 => 261217)
--- trunk/Source/WebCore/ChangeLog 2020-05-06 06:51:50 UTC (rev 261216)
+++ trunk/Source/WebCore/ChangeLog 2020-05-06 08:06:03 UTC (rev 261217)
@@ -1,3 +1,28 @@
+2020-05-05 Antoine Quint <[email protected]>
+
+ Fix animation ordering to make imported/w3c/web-platform-tests/css/css-animations/Element-getAnimations.tentative.html pass
+ https://bugs.webkit.org/show_bug.cgi?id=211468
+ <rdar://problem/62732578>
+
+ Reviewed by David Kilzer.
+
+ The "Animation composite order" section of the CSS Animations Level 2 specification (https://drafts.csswg.org/css-animations-2/#animation-composite-order)
+ defines the relative composite order of animations. We bake this into compareAnimationsByCompositeOrder(), but this function would not yield consistent
+ results if it is called in a non-stable sort, because if both CSSAnimation objects passed to this function have the same backing Animation object, they
+ would not return the same value if passed in a different order. The Web Animations spec always ensures that procedures that sort using the composite
+ order are called as part of a stable sort. So we change all call sites to use std::stable_sort and add an assertion in case we have two CSSAnimation
+ objects with the same backing Animation objects to catch cases like this in the future.
+
+ Finally, since we already know only relevant animations can find their way into the output of Document::getAnimations(), we also ensure we iterate over
+ m_animations (which holds only relevant animations) rather than m_allAnimations (which may not).
+
+ * animation/DocumentTimeline.cpp:
+ (WebCore::DocumentTimeline::getAnimations const):
+ * animation/KeyframeEffectStack.cpp:
+ (WebCore::KeyframeEffectStack::ensureEffectsAreSorted):
+ * animation/WebAnimationUtilities.cpp:
+ (WebCore::compareAnimationsByCompositeOrder):
+
2020-05-05 Simon Fraser <[email protected]>
EventHandler::dispatchMouseEvent() cleanup
Modified: trunk/Source/WebCore/animation/DocumentTimeline.cpp (261216 => 261217)
--- trunk/Source/WebCore/animation/DocumentTimeline.cpp 2020-05-06 06:51:50 UTC (rev 261216)
+++ trunk/Source/WebCore/animation/DocumentTimeline.cpp 2020-05-06 08:06:03 UTC (rev 261217)
@@ -147,7 +147,7 @@
Vector<RefPtr<WebAnimation>> webAnimations;
// First, let's get all qualifying animations in their right group.
- for (const auto& animation : m_allAnimations) {
+ for (const auto& animation : m_animations) {
if (!animation || !animation->isRelevant() || animation->timeline() != this || !is<KeyframeEffect>(animation->effect()))
continue;
@@ -156,15 +156,15 @@
continue;
if (is<CSSTransition>(animation.get()) && downcast<CSSTransition>(animation.get())->owningElement())
- cssTransitions.append(animation.get());
+ cssTransitions.append(animation);
else if (is<CSSAnimation>(animation.get()) && downcast<CSSAnimation>(animation.get())->owningElement())
- cssAnimations.append(animation.get());
+ cssAnimations.append(animation);
else
- webAnimations.append(animation.get());
+ webAnimations.append(animation);
}
// Now sort CSS Transitions by their composite order.
- std::sort(cssTransitions.begin(), cssTransitions.end(), [](auto& lhs, auto& rhs) {
+ std::stable_sort(cssTransitions.begin(), cssTransitions.end(), [](auto& lhs, auto& rhs) {
// https://drafts.csswg.org/css-transitions-2/#animation-composite-order
auto* lhsTransition = downcast<CSSTransition>(lhs.get());
auto* rhsTransition = downcast<CSSTransition>(rhs.get());
@@ -186,7 +186,7 @@
});
// Now sort CSS Animations by their composite order.
- std::sort(cssAnimations.begin(), cssAnimations.end(), [](auto& lhs, auto& rhs) {
+ std::stable_sort(cssAnimations.begin(), cssAnimations.end(), [](auto& lhs, auto& rhs) {
// https://drafts.csswg.org/css-animations-2/#animation-composite-order
auto* lhsOwningElement = downcast<CSSAnimation>(lhs.get())->owningElement();
auto* rhsOwningElement = downcast<CSSAnimation>(rhs.get())->owningElement();
@@ -199,7 +199,7 @@
return compareAnimationsByCompositeOrder(*lhs, *rhs, lhsOwningElement->ensureKeyframeEffectStack().cssAnimationList());
});
- std::sort(webAnimations.begin(), webAnimations.end(), [](auto& lhs, auto& rhs) {
+ std::stable_sort(webAnimations.begin(), webAnimations.end(), [](auto& lhs, auto& rhs) {
return lhs->globalPosition() < rhs->globalPosition();
});
Modified: trunk/Source/WebCore/animation/KeyframeEffectStack.cpp (261216 => 261217)
--- trunk/Source/WebCore/animation/KeyframeEffectStack.cpp 2020-05-06 06:51:50 UTC (rev 261216)
+++ trunk/Source/WebCore/animation/KeyframeEffectStack.cpp 2020-05-06 08:06:03 UTC (rev 261217)
@@ -88,7 +88,7 @@
if (m_isSorted || m_effects.size() < 2)
return;
- std::sort(m_effects.begin(), m_effects.end(), [&](auto& lhs, auto& rhs) {
+ std::stable_sort(m_effects.begin(), m_effects.end(), [&](auto& lhs, auto& rhs) {
RELEASE_ASSERT(lhs.get());
RELEASE_ASSERT(rhs.get());
Modified: trunk/Source/WebCore/animation/WebAnimationUtilities.cpp (261216 => 261217)
--- trunk/Source/WebCore/animation/WebAnimationUtilities.cpp 2020-05-06 06:51:50 UTC (rev 261216)
+++ trunk/Source/WebCore/animation/WebAnimationUtilities.cpp 2020-05-06 08:06:03 UTC (rev 261217)
@@ -37,6 +37,11 @@
bool compareAnimationsByCompositeOrder(WebAnimation& lhsAnimation, WebAnimation& rhsAnimation, const AnimationList* cssAnimationList)
{
+ // We should not ever be calling this function with two WebAnimation objects that are the same. If that were the case,
+ // then comparing objects of this kind would yield inconsistent results when comparing A == B and B == A. As such,
+ // this function should be called with std::stable_sort().
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(&lhsAnimation != &rhsAnimation);
+
bool lhsHasOwningElement = is<DeclarativeAnimation>(lhsAnimation) && downcast<DeclarativeAnimation>(lhsAnimation).owningElement();
bool rhsHasOwningElement = is<DeclarativeAnimation>(rhsAnimation) && downcast<DeclarativeAnimation>(rhsAnimation).owningElement();
