Changeset 233585 in webkit

Timestamp:

Jul 6, 2018, 11:43:57 AM (8 years ago)

Author:

graouts@webkit.org

Message:

[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.

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:

Location:

trunk

Files:

• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animation/finish-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/animation/AnimationTimeline.cpp (modified) (4 diffs)
• Source/WebCore/animation/AnimationTimeline.h (modified) (2 diffs)
• Source/WebCore/animation/DocumentTimeline.cpp (modified) (1 diff)
• Source/WebCore/animation/WebAnimation.cpp (modified) (1 diff)
• Source/WebCore/animation/WebAnimation.h (modified) (1 diff)

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animation/finish-expected.txt

@@ -14 +14 @@
 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 
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/animation/AnimationTimeline.cpp

@@ -57 +57 @@
 void AnimationTimeline::addAnimation(Ref<WebAnimation>&& animation)
 {
+    m_animationsWithoutTarget.add(animation.ptr());
     m_animations.add(WTFMove(animation));
     timingModelDidChange();
@@ -63 +64 @@
 void AnimationTimeline::removeAnimation(Ref<WebAnimation>&& animation)
 {
+    m_animationsWithoutTarget.remove(animation.ptr());
     m_animations.remove(WTFMove(animation));
     timingModelDidChange();
@@ -92 +94 @@
 void AnimationTimeline::animationWasAddedToElement(WebAnimation& animation, Element& element)
 {
+    m_animationsWithoutTarget.remove(&animation);
+
     relevantMapForAnimation(animation).ensure(&element, [] {
         return ListHashSet<RefPtr<WebAnimation>> { };
@@ -112 +116 @@
 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

trunk/Source/WebCore/animation/AnimationTimeline.h

@@ -82 +82 @@
     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; }
@@ -98 +99 @@
     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;

trunk/Source/WebCore/animation/DocumentTimeline.cpp

@@ -281 +281 @@
     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())

trunk/Source/WebCore/animation/WebAnimation.cpp

@@ -1056 +1056 @@
 }
 
+void WebAnimation::resolve()
+{
+    updateFinishedState(DidSeek::No, SynchronouslyNotify::Yes);
+}
+
 void WebAnimation::resolve(RenderStyle& targetStyle)
 {
-    updateFinishedState(DidSeek::No, SynchronouslyNotify::Yes);
-
+    resolve();
     if (m_effect)
         m_effect->apply(targetStyle);

trunk/Source/WebCore/animation/WebAnimation.h

@@ -106 +106 @@
 
     Seconds timeToNextRequiredTick() const;
+    void resolve();
     virtual void resolve(RenderStyle&);
     void runPendingTasks();