[Web Animations] Make WPT test at interfaces/Animation/finish.html pass reliably
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Jul 2018 18:43:57 +0000 (18:43 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Jul 2018 18:43:57 +0000 (18:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186496
<rdar://problem/41000179>

Reviewed by Dean Jackson.

LayoutTests/imported/w3c:

Mark a WPT progression.

* web-platform-tests/web-animations/interfaces/Animation/finish-expected.txt:

Source/WebCore:

We used to only resolve animations that had a target element, but animations need not have a target and their
current time should still advance so that their finished promise may resolve. We now maintain a list of animations
without targets and we iterate through them as well as animations with targets in DocumentTimeline::updateAnimations().

* animation/AnimationTimeline.cpp:
(WebCore::AnimationTimeline::addAnimation):
(WebCore::AnimationTimeline::removeAnimation):
(WebCore::AnimationTimeline::animationWasAddedToElement):
(WebCore::AnimationTimeline::animationWasRemovedFromElement):
* animation/AnimationTimeline.h:
(WebCore::AnimationTimeline:: const):
* animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::updateAnimations):
* animation/WebAnimation.cpp:
(WebCore::WebAnimation::resolve):
* animation/WebAnimation.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233585 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animation/finish-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/animation/AnimationTimeline.cpp
Source/WebCore/animation/AnimationTimeline.h
Source/WebCore/animation/DocumentTimeline.cpp
Source/WebCore/animation/WebAnimation.cpp
Source/WebCore/animation/WebAnimation.h

index b70f1a0..568b2f9 100644 (file)
@@ -1,5 +1,17 @@
 2018-07-05  Antoine Quint  <graouts@apple.com>
 
+        [Web Animations] Make WPT test at interfaces/Animation/finish.html pass reliably
+        https://bugs.webkit.org/show_bug.cgi?id=186496
+        <rdar://problem/41000179>
+
+        Reviewed by Dean Jackson.
+
+        Mark a WPT progression.
+
+        * web-platform-tests/web-animations/interfaces/Animation/finish-expected.txt:
+
+2018-07-05  Antoine Quint  <graouts@apple.com>
+
         [Web Animations] Make WPT test at interfaces/Animation/finished.html pass reliably
         https://bugs.webkit.org/show_bug.cgi?id=186497
         <rdar://problem/41000193>
index 1fa3e2e..bf7f8f5 100644 (file)
@@ -13,5 +13,5 @@ PASS Test finish() during aborted pause
 PASS Test resetting of computed style 
 PASS Test finish() resolves finished promise synchronously 
 PASS Test finish() resolves finished promise synchronously with an animation without a target 
-FAIL Test normally finished animation resolves finished promise synchronously with an animation without a target assert_true: Animation.finished should be resolved soon after Animation finishes normally expected true got false
+PASS Test normally finished animation resolves finished promise synchronously with an animation without a target 
 
index b3cb38a..eac3a58 100644 (file)
@@ -1,5 +1,30 @@
 2018-07-05  Antoine Quint  <graouts@apple.com>
 
+        [Web Animations] Make WPT test at interfaces/Animation/finish.html pass reliably
+        https://bugs.webkit.org/show_bug.cgi?id=186496
+        <rdar://problem/41000179>
+
+        Reviewed by Dean Jackson.
+
+        We used to only resolve animations that had a target element, but animations need not have a target and their
+        current time should still advance so that their finished promise may resolve. We now maintain a list of animations
+        without targets and we iterate through them as well as animations with targets in DocumentTimeline::updateAnimations().
+
+        * animation/AnimationTimeline.cpp:
+        (WebCore::AnimationTimeline::addAnimation):
+        (WebCore::AnimationTimeline::removeAnimation):
+        (WebCore::AnimationTimeline::animationWasAddedToElement):
+        (WebCore::AnimationTimeline::animationWasRemovedFromElement):
+        * animation/AnimationTimeline.h:
+        (WebCore::AnimationTimeline:: const):
+        * animation/DocumentTimeline.cpp:
+        (WebCore::DocumentTimeline::updateAnimations):
+        * animation/WebAnimation.cpp:
+        (WebCore::WebAnimation::resolve):
+        * animation/WebAnimation.h:
+
+2018-07-05  Antoine Quint  <graouts@apple.com>
+
         [Web Animations] Make WPT test at interfaces/Animation/finished.html pass reliably
         https://bugs.webkit.org/show_bug.cgi?id=186497
         <rdar://problem/41000193>
index bdf44ad..01860b9 100644 (file)
@@ -56,12 +56,14 @@ AnimationTimeline::~AnimationTimeline()
 
 void AnimationTimeline::addAnimation(Ref<WebAnimation>&& animation)
 {
+    m_animationsWithoutTarget.add(animation.ptr());
     m_animations.add(WTFMove(animation));
     timingModelDidChange();
 }
 
 void AnimationTimeline::removeAnimation(Ref<WebAnimation>&& animation)
 {
+    m_animationsWithoutTarget.remove(animation.ptr());
     m_animations.remove(WTFMove(animation));
     timingModelDidChange();
 }
@@ -91,6 +93,8 @@ HashMap<Element*, ListHashSet<RefPtr<WebAnimation>>>& AnimationTimeline::relevan
 
 void AnimationTimeline::animationWasAddedToElement(WebAnimation& animation, Element& element)
 {
+    m_animationsWithoutTarget.remove(&animation);
+
     relevantMapForAnimation(animation).ensure(&element, [] {
         return ListHashSet<RefPtr<WebAnimation>> { };
     }).iterator->value.add(&animation);
@@ -111,6 +115,9 @@ static inline bool removeCSSTransitionFromMap(CSSTransition& transition, Element
 
 void AnimationTimeline::animationWasRemovedFromElement(WebAnimation& animation, Element& element)
 {
+    // This animation doesn't have a target for now.
+    m_animationsWithoutTarget.add(&animation);
+
     // First, we clear this animation from one of the m_elementToCSSAnimationsMap, m_elementToCSSTransitionsMap,
     // m_elementToAnimationsMap or m_elementToCompletedCSSTransitionByCSSPropertyID map, whichever is relevant to
     // this type of animation.
index fb2a0c3..f6c17a6 100644 (file)
@@ -81,6 +81,7 @@ protected:
 
     bool hasElementAnimations() const { return !m_elementToAnimationsMap.isEmpty() || !m_elementToCSSAnimationsMap.isEmpty() || !m_elementToCSSTransitionsMap.isEmpty(); }
 
+    const ListHashSet<WebAnimation*>& animationsWithoutTarget() const { return m_animationsWithoutTarget; }
     const HashMap<Element*, ListHashSet<RefPtr<WebAnimation>>>& elementToAnimationsMap() { return m_elementToAnimationsMap; }
     const HashMap<Element*, ListHashSet<RefPtr<WebAnimation>>>& elementToCSSAnimationsMap() { return m_elementToCSSAnimationsMap; }
     const HashMap<Element*, ListHashSet<RefPtr<WebAnimation>>>& elementToCSSTransitionsMap() { return m_elementToCSSTransitionsMap; }
@@ -97,6 +98,7 @@ private:
     HashMap<Element*, ListHashSet<RefPtr<WebAnimation>>> m_elementToCSSTransitionsMap;
     ListHashSet<RefPtr<WebAnimation>> m_animations;
 
+    ListHashSet<WebAnimation*> m_animationsWithoutTarget;
     HashMap<Element*, HashMap<String, RefPtr<CSSAnimation>>> m_elementToCSSAnimationByName;
     HashMap<Element*, HashMap<CSSPropertyID, RefPtr<CSSTransition>>> m_elementToRunningCSSTransitionByCSSPropertyID;
     HashMap<Element*, HashMap<CSSPropertyID, RefPtr<CSSTransition>>> m_elementToCompletedCSSTransitionByCSSPropertyID;
index 2105be5..51976af 100644 (file)
@@ -280,6 +280,12 @@ void DocumentTimeline::updateAnimations()
     // running pending tasks can fire right away.
     MicrotaskQueue::mainThreadQueue().performMicrotaskCheckpoint();
 
+    // Let's first resolve any animation that does not have a target.
+    for (auto* animation : animationsWithoutTarget())
+        animation->resolve();
+
+    // For the rest of the animations, we will resolve them via TreeResolver::createAnimatedElementUpdate()
+    // by invalidating their target element's style.
     if (m_document && hasElementAnimations()) {
         for (const auto& elementToAnimationsMapItem : elementToAnimationsMap())
             elementToAnimationsMapItem.key->invalidateStyleAndLayerComposition();
index 55538b9..9eacd45 100644 (file)
@@ -1055,10 +1055,14 @@ void WebAnimation::runPendingTasks()
         runPendingPlayTask();
 }
 
-void WebAnimation::resolve(RenderStyle& targetStyle)
+void WebAnimation::resolve()
 {
     updateFinishedState(DidSeek::No, SynchronouslyNotify::Yes);
+}
 
+void WebAnimation::resolve(RenderStyle& targetStyle)
+{
+    resolve();
     if (m_effect)
         m_effect->apply(targetStyle);
 }
index 3e34e87..d0578e8 100644 (file)
@@ -105,6 +105,7 @@ public:
     virtual ExceptionOr<void> bindingsPause() { return pause(); }
 
     Seconds timeToNextRequiredTick() const;
+    void resolve();
     virtual void resolve(RenderStyle&);
     void runPendingTasks();
     void effectTargetDidChange(Element* previousTarget, Element* newTarget);