Diff
Modified: trunk/LayoutTests/ChangeLog (229889 => 229890)
--- trunk/LayoutTests/ChangeLog 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/LayoutTests/ChangeLog 2018-03-23 11:47:26 UTC (rev 229890)
@@ -1,3 +1,14 @@
+2018-03-22 Antoine Quint <grao...@apple.com>
+
+ [Web Animations] Correctly cancel animations when a parent gets a "display: none" style or when an element is removed
+ https://bugs.webkit.org/show_bug.cgi?id=183919
+
+ Reviewed by Dean Jackson.
+
+ Three of the imported Mozilla tests now pass reliably, removing them from the list of flaky failure and timeout tests.
+
+ * TestExpectations:
+
2018-03-23 Antoine Quint <grao...@apple.com>
[Web Animations] Animated transform styles are ignored when calling getComputedStyle()
Modified: trunk/LayoutTests/TestExpectations (229889 => 229890)
--- trunk/LayoutTests/TestExpectations 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/LayoutTests/TestExpectations 2018-03-23 11:47:26 UTC (rev 229890)
@@ -1705,7 +1705,6 @@
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/183815 imported/mozilla/css-animations/test_animation-cancel.html [ Pass Failure Timeout ]
webkit.org/b/183816 imported/mozilla/css-transitions/test_keyframeeffect-getkeyframes.html [ Pass Failure Timeout ]
webkit.org/b/183817 imported/mozilla/css-animations/test_animation-computed-timing.html [ Pass Failure Timeout ]
webkit.org/b/183818 imported/mozilla/css-animations/test_pseudoElement-get-animations.html [ Pass Failure Timeout ]
@@ -1729,10 +1728,8 @@
webkit.org/b/183836 imported/mozilla/css-animations/test_animations-dynamic-changes.html [ Pass Failure Timeout ]
webkit.org/b/183837 imported/mozilla/css-transitions/test_document-get-animations.html [ Pass Failure Timeout ]
webkit.org/b/183838 imported/mozilla/css-animations/test_cssanimation-animationname.html [ Pass Failure Timeout ]
-webkit.org/b/183839 imported/mozilla/css-transitions/test_effect-target.html [ Pass Failure Timeout ]
webkit.org/b/183840 imported/mozilla/css-animations/test_document-get-animations.html [ Pass Failure Timeout ]
webkit.org/b/183841 imported/mozilla/css-transitions/test_element-get-animations.html [ Pass Failure Timeout ]
-webkit.org/b/183842 imported/mozilla/css-animations/test_effect-target.html [ Pass Failure Timeout ]
webkit.org/b/183844 imported/mozilla/css-animations/test_element-get-animations.html [ Pass Failure Timeout ]
webkit.org/b/183846 imported/mozilla/css-transitions/test_pseudoElement-get-animations.html [ Pass Failure Timeout ]
webkit.org/b/183847 imported/mozilla/css-animations/test_event-order.html [ Pass Failure Timeout ]
Modified: trunk/LayoutTests/imported/mozilla/ChangeLog (229889 => 229890)
--- trunk/LayoutTests/imported/mozilla/ChangeLog 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/LayoutTests/imported/mozilla/ChangeLog 2018-03-23 11:47:26 UTC (rev 229890)
@@ -1,3 +1,19 @@
+2018-03-22 Antoine Quint <grao...@apple.com>
+
+ [Web Animations] Correctly cancel animations when a parent gets a "display: none" style or when an element is removed
+ https://bugs.webkit.org/show_bug.cgi?id=183919
+
+ Reviewed by Dean Jackson.
+
+ A number of tests now pass thanks to correctly canceling animations when a parent element in the hierarchy gets a
+ "display: none" style or if an element with animations is removed.
+
+ * css-animations/test_animation-cancel-expected.txt:
+ * css-animations/test_document-get-animations-expected.txt:
+ * css-animations/test_effect-target-expected.txt:
+ * css-transitions/test_animation-cancel-expected.txt:
+ * css-transitions/test_effect-target-expected.txt:
+
2018-03-23 Antoine Quint <grao...@apple.com>
[Web Animations] Animated transform styles are ignored when calling getComputedStyle()
Modified: trunk/LayoutTests/imported/mozilla/css-animations/test_animation-cancel-expected.txt (229889 => 229890)
--- trunk/LayoutTests/imported/mozilla/css-animations/test_animation-cancel-expected.txt 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/LayoutTests/imported/mozilla/css-animations/test_animation-cancel-expected.txt 2018-03-23 11:47:26 UTC (rev 229890)
@@ -7,5 +7,5 @@
PASS After cancelling an animation, updating animation-play-state doesn't make it live again
PASS Setting animation-name to 'none' cancels the animation
PASS Setting display:none on an element cancel its animations
-FAIL Setting display:none on an ancestor element cancels animations on descendants assert_equals: expected "idle" but got "running"
+PASS Setting display:none on an ancestor element cancels animations on descendants
Modified: trunk/LayoutTests/imported/mozilla/css-animations/test_document-get-animations-expected.txt (229889 => 229890)
--- trunk/LayoutTests/imported/mozilla/css-animations/test_document-get-animations-expected.txt 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/LayoutTests/imported/mozilla/css-animations/test_document-get-animations-expected.txt 2018-03-23 11:47:26 UTC (rev 229890)
@@ -3,7 +3,7 @@
PASS getAnimations for CSS Animations
PASS Order of CSS Animations - within an element
FAIL Order of CSS Animations - across elements assert_equals: Order of first animation returned after tree surgery expected Element node <div style="animation: animLeft 100s"><div style="animati... but got Element node <div style="animation: animLeft 100s"></div>
-FAIL Order of CSS Animations - across and within elements assert_equals: getAnimations returns all running CSS Animations after making changes expected 4 but got 6
+PASS Order of CSS Animations - across and within elements
FAIL Order of CSS Animations - markup-bound vs free animations assert_equals: getAnimations returns markup-bound and free animations expected 2 but got 1
FAIL Order of CSS Animations - free animations assert_equals: getAnimations returns free animations expected 2 but got 0
FAIL Order of CSS Animations and CSS Transitions assert_equals: Both CSS animations and transitions are returned expected 2 but got 1
Modified: trunk/LayoutTests/imported/mozilla/css-animations/test_effect-target-expected.txt (229889 => 229890)
--- trunk/LayoutTests/imported/mozilla/css-animations/test_effect-target-expected.txt 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/LayoutTests/imported/mozilla/css-animations/test_effect-target-expected.txt 2018-03-23 11:47:26 UTC (rev 229890)
@@ -1,5 +1,5 @@
PASS Returned CSS animations have the correct effect target
PASS effect.target should return the same CSSPseudoElement object each time
-FAIL effect.target from the script-generated animation should return the same CSSPseudoElement object as that from the CSS generated animation assert_equals: Got animations running on ::after pseudo element expected 2 but got 4
+PASS effect.target from the script-generated animation should return the same CSSPseudoElement object as that from the CSS generated animation
Modified: trunk/LayoutTests/imported/mozilla/css-transitions/test_animation-cancel-expected.txt (229889 => 229890)
--- trunk/LayoutTests/imported/mozilla/css-transitions/test_animation-cancel-expected.txt 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/LayoutTests/imported/mozilla/css-transitions/test_animation-cancel-expected.txt 2018-03-23 11:47:26 UTC (rev 229890)
@@ -4,7 +4,7 @@
PASS After cancelling a finished transition, it can still be re-used
PASS After cancelling a transition, updating transition properties doesn't make it live again
PASS Setting display:none on an element cancels its transitions
-FAIL Setting display:none cancels transitions on a child element assert_equals: expected "idle" but got "running"
+PASS Setting display:none cancels transitions on a child element
PASS Removing a property from transition-property cancels transitions on that property
PASS Setting zero combined duration
FAIL Changing style to another interpolable value cancels the original transition assert_equals: expected "idle" but got "finished"
Modified: trunk/LayoutTests/imported/mozilla/css-transitions/test_effect-target-expected.txt (229889 => 229890)
--- trunk/LayoutTests/imported/mozilla/css-transitions/test_effect-target-expected.txt 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/LayoutTests/imported/mozilla/css-transitions/test_effect-target-expected.txt 2018-03-23 11:47:26 UTC (rev 229890)
@@ -1,5 +1,5 @@
PASS Returned CSS transitions have the correct Animation.target
PASS effect.target should return the same CSSPseudoElement object each time
-FAIL effect.target from the script-generated animation should return the same CSSPseudoElement object as that from the CSS generated transition assert_equals: Got animations running on ::after pseudo element expected 2 but got 4
+PASS effect.target from the script-generated animation should return the same CSSPseudoElement object as that from the CSS generated transition
Modified: trunk/Source/WebCore/ChangeLog (229889 => 229890)
--- trunk/Source/WebCore/ChangeLog 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/Source/WebCore/ChangeLog 2018-03-23 11:47:26 UTC (rev 229890)
@@ -1,3 +1,31 @@
+2018-03-22 Antoine Quint <grao...@apple.com>
+
+ [Web Animations] Correctly cancel animations when a parent gets a "display: none" style or when an element is removed
+ https://bugs.webkit.org/show_bug.cgi?id=183919
+
+ Reviewed by Dean Jackson.
+
+ The old CSSAnimationController provided a cancelAnimations(Element&) method that allowed for animations for a given element
+ to be canceled when a parent element in the hierarchy gets a "display: none" style or if an element with animations is removed.
+ We add a similar cancelAnimationsForElement(Element&) method on AnimationTimeline and update CSSAnimationController::cancelAnimations()
+ call sites to use AnimationTimeline::cancelAnimationsForElement() when the flag to use Web Animations is on.
+
+ * animation/AnimationTimeline.cpp:
+ (WebCore::AnimationTimeline::cancelAnimationsForElement): Iterate over all animations for the provided element and call cancel() on them.
+ * animation/AnimationTimeline.h:
+ * animation/DocumentTimeline.cpp:
+ (WebCore::DocumentTimeline::animatedStyleForRenderer): Drive-by fix while I was reviewed call sites to animationsForElement() to make
+ sure we don't create extra RefPtr<> objects.
+ * dom/Element.cpp:
+ (WebCore::Element::removedFromAncestor): Call AnimationTimeline::cancelAnimationsForElement() if the Web Animations flag is on when an
+ element is removed.
+ * dom/PseudoElement.cpp:
+ (WebCore::PseudoElement::clearHostElement): Call AnimationTimeline::cancelAnimationsForElement() if the Web Animations flag is on when
+ a pseudo-element is removed.
+ * rendering/updating/RenderTreeUpdater.cpp:
+ (WebCore::RenderTreeUpdater::tearDownRenderers): Call AnimationTimeline::cancelAnimationsForElement() if the Web Animations flag is on
+ for all children elements when an element gets a "display: none" style.
+
2018-03-23 Antoine Quint <grao...@apple.com>
[Web Animations] Animated transform styles are ignored when calling getComputedStyle()
Modified: trunk/Source/WebCore/animation/AnimationTimeline.cpp (229889 => 229890)
--- trunk/Source/WebCore/animation/AnimationTimeline.cpp 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/Source/WebCore/animation/AnimationTimeline.cpp 2018-03-23 11:47:26 UTC (rev 229890)
@@ -127,6 +127,12 @@
return animations;
}
+void AnimationTimeline::cancelAnimationsForElement(Element& element)
+{
+ for (const auto& animation : animationsForElement(element))
+ animation->cancel();
+}
+
void AnimationTimeline::updateCSSAnimationsForElement(Element& element, const RenderStyle& newStyle, const RenderStyle* oldStyle)
{
if (element.document().pageCacheState() != Document::NotInPageCache)
Modified: trunk/Source/WebCore/animation/AnimationTimeline.h (229889 => 229890)
--- trunk/Source/WebCore/animation/AnimationTimeline.h 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/Source/WebCore/animation/AnimationTimeline.h 2018-03-23 11:47:26 UTC (rev 229890)
@@ -59,6 +59,7 @@
const ListHashSet<RefPtr<WebAnimation>>& animations() const { return m_animations; }
Vector<RefPtr<WebAnimation>> animationsForElement(Element&);
+ void cancelAnimationsForElement(Element&);
void animationWasAddedToElement(WebAnimation&, Element&);
void animationWasRemovedFromElement(WebAnimation&, Element&);
Modified: trunk/Source/WebCore/animation/DocumentTimeline.cpp (229889 => 229890)
--- trunk/Source/WebCore/animation/DocumentTimeline.cpp 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/Source/WebCore/animation/DocumentTimeline.cpp 2018-03-23 11:47:26 UTC (rev 229890)
@@ -192,7 +192,7 @@
std::unique_ptr<RenderStyle> result;
if (auto* element = renderer.element()) {
- for (auto animation : animationsForElement(*element)) {
+ for (const auto& animation : animationsForElement(*element)) {
if (is<KeyframeEffectReadOnly>(animation->effect()))
downcast<KeyframeEffectReadOnly>(animation->effect())->getAnimatedStyle(result);
}
Modified: trunk/Source/WebCore/dom/Element.cpp (229889 => 229890)
--- trunk/Source/WebCore/dom/Element.cpp 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/Source/WebCore/dom/Element.cpp 2018-03-23 11:47:26 UTC (rev 229890)
@@ -1795,7 +1795,10 @@
document().accessSVGExtensions().removeElementFromPendingResources(this);
RefPtr<Frame> frame = document().frame();
- if (frame)
+ if (RuntimeEnabledFeatures::sharedFeatures().cssAnimationsAndCSSTransitionsBackedByWebAnimationsEnabled()) {
+ if (auto* timeline = document().existingTimeline())
+ timeline->cancelAnimationsForElement(*this);
+ } else if (frame)
frame->animation().cancelAnimations(*this);
#if PLATFORM(MAC)
Modified: trunk/Source/WebCore/dom/PseudoElement.cpp (229889 => 229890)
--- trunk/Source/WebCore/dom/PseudoElement.cpp 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/Source/WebCore/dom/PseudoElement.cpp 2018-03-23 11:47:26 UTC (rev 229890)
@@ -30,10 +30,12 @@
#include "CSSAnimationController.h"
#include "ContentData.h"
+#include "DocumentTimeline.h"
#include "InspectorInstrumentation.h"
#include "RenderElement.h"
#include "RenderImage.h"
#include "RenderQuote.h"
+#include "RuntimeEnabledFeatures.h"
#include "StyleResolver.h"
#include <wtf/IsoMallocInlines.h>
@@ -88,7 +90,10 @@
{
InspectorInstrumentation::pseudoElementDestroyed(document().page(), *this);
- if (auto* frame = document().frame())
+ if (RuntimeEnabledFeatures::sharedFeatures().cssAnimationsAndCSSTransitionsBackedByWebAnimationsEnabled()) {
+ if (auto* timeline = document().existingTimeline())
+ timeline->cancelAnimationsForElement(*this);
+ } else if (auto* frame = document().frame())
frame->animation().cancelAnimations(*this);
m_hostElement = nullptr;
Modified: trunk/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp (229889 => 229890)
--- trunk/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp 2018-03-23 11:45:40 UTC (rev 229889)
+++ trunk/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp 2018-03-23 11:47:26 UTC (rev 229890)
@@ -31,6 +31,7 @@
#include "ComposedTreeAncestorIterator.h"
#include "ComposedTreeIterator.h"
#include "Document.h"
+#include "DocumentTimeline.h"
#include "Element.h"
#include "HTMLParserIdioms.h"
#include "HTMLSlotElement.h"
@@ -41,6 +42,7 @@
#include "RenderFullScreen.h"
#include "RenderInline.h"
#include "RenderTreeUpdaterGeneratedContent.h"
+#include "RuntimeEnabledFeatures.h"
#include "StyleResolver.h"
#include "StyleTreeResolver.h"
#include <wtf/SystemTracing.h>
@@ -542,14 +544,21 @@
teardownStack.append(&element);
};
- auto& animationController = root.document().frame()->animation();
+ auto& document = root.document();
+ auto* timeline = document.existingTimeline();
+ auto& animationController = document.frame()->animation();
auto pop = [&] (unsigned depth) {
while (teardownStack.size() > depth) {
auto& element = *teardownStack.takeLast();
- if (teardownType == TeardownType::Full || teardownType == TeardownType::RendererUpdateCancelingAnimations)
- animationController.cancelAnimations(element);
+ if (teardownType == TeardownType::Full || teardownType == TeardownType::RendererUpdateCancelingAnimations) {
+ if (RuntimeEnabledFeatures::sharedFeatures().cssAnimationsAndCSSTransitionsBackedByWebAnimationsEnabled()) {
+ if (timeline)
+ timeline->cancelAnimationsForElement(element);
+ } else
+ animationController.cancelAnimations(element);
+ }
if (teardownType == TeardownType::Full)
element.clearHoverAndActiveStatusBeforeDetachingRenderer();
