- Revision
- 264912
- Author
- carlo...@webkit.org
- Date
- 2020-07-27 03:48:05 -0700 (Mon, 27 Jul 2020)
Log Message
Merge r261217 - 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: releases/WebKitGTK/webkit-2.28/LayoutTests/imported/w3c/ChangeLog (264911 => 264912)
--- releases/WebKitGTK/webkit-2.28/LayoutTests/imported/w3c/ChangeLog 2020-07-27 10:47:58 UTC (rev 264911)
+++ releases/WebKitGTK/webkit-2.28/LayoutTests/imported/w3c/ChangeLog 2020-07-27 10:48:05 UTC (rev 264912)
@@ -1,3 +1,15 @@
+2020-05-05 Antoine Quint <grao...@apple.com>
+
+ 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-02-21 Carlos Alberto Lopez Perez <clo...@igalia.com>
Value sanitization for input[type=text] should not truncate a value at a control character
Modified: releases/WebKitGTK/webkit-2.28/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/Element-getAnimations.tentative-expected.txt (264911 => 264912)
--- releases/WebKitGTK/webkit-2.28/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/Element-getAnimations.tentative-expected.txt 2020-07-27 10:47:58 UTC (rev 264911)
+++ releases/WebKitGTK/webkit-2.28/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/Element-getAnimations.tentative-expected.txt 2020-07-27 10:48:05 UTC (rev 264912)
@@ -19,6 +19,5 @@
PASS { subtree: false } on a leaf element returns the element's animations and ignore pseudo-elements
FAIL { subtree: true } on a leaf element returns the element's animations and its pseudo-elements' animations assert_equals: The animation targeting the parent element should be returned first expected (object) null but got (undefined) undefined
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 parent element should be returned first expected (object) null but got (undefined) undefined
-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: releases/WebKitGTK/webkit-2.28/Source/WebCore/ChangeLog (264911 => 264912)
--- releases/WebKitGTK/webkit-2.28/Source/WebCore/ChangeLog 2020-07-27 10:47:58 UTC (rev 264911)
+++ releases/WebKitGTK/webkit-2.28/Source/WebCore/ChangeLog 2020-07-27 10:48:05 UTC (rev 264912)
@@ -1,3 +1,28 @@
+2020-05-05 Antoine Quint <grao...@apple.com>
+
+ 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-07-09 Adrian Perez de Castro <ape...@igalia.com>
Unreviewed non-unified build fixes.
Modified: releases/WebKitGTK/webkit-2.28/Source/WebCore/animation/DocumentTimeline.cpp (264911 => 264912)
--- releases/WebKitGTK/webkit-2.28/Source/WebCore/animation/DocumentTimeline.cpp 2020-07-27 10:47:58 UTC (rev 264911)
+++ releases/WebKitGTK/webkit-2.28/Source/WebCore/animation/DocumentTimeline.cpp 2020-07-27 10:48:05 UTC (rev 264912)
@@ -142,7 +142,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;
@@ -151,15 +151,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());
@@ -181,7 +181,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();
@@ -194,7 +194,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: releases/WebKitGTK/webkit-2.28/Source/WebCore/animation/KeyframeEffectStack.cpp (264911 => 264912)
--- releases/WebKitGTK/webkit-2.28/Source/WebCore/animation/KeyframeEffectStack.cpp 2020-07-27 10:47:58 UTC (rev 264911)
+++ releases/WebKitGTK/webkit-2.28/Source/WebCore/animation/KeyframeEffectStack.cpp 2020-07-27 10:48:05 UTC (rev 264912)
@@ -80,7 +80,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) {
auto* lhsAnimation = lhs->animation();
auto* rhsAnimation = rhs->animation();
Modified: releases/WebKitGTK/webkit-2.28/Source/WebCore/animation/WebAnimationUtilities.cpp (264911 => 264912)
--- releases/WebKitGTK/webkit-2.28/Source/WebCore/animation/WebAnimationUtilities.cpp 2020-07-27 10:47:58 UTC (rev 264911)
+++ releases/WebKitGTK/webkit-2.28/Source/WebCore/animation/WebAnimationUtilities.cpp 2020-07-27 10:48:05 UTC (rev 264912)
@@ -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();
