Title: [237726] trunk
Revision
237726
Author
grao...@webkit.org
Date
2018-11-02 00:54:19 -0700 (Fri, 02 Nov 2018)

Log Message

[Web Animations] Make document.getAnimations() return declarative animations in the correct order
https://bugs.webkit.org/show_bug.cgi?id=191153

Reviewed by Dean Jackson.

LayoutTests/imported/mozilla:

Mark progressions for the document.getAnimations() tests covering CSS Animations and CSS Transitions.
These tests are now in WPT and will be updated as part of a general WPT update for all of Web Animations
shortly. We will upstream the minor changes made at that point. Those changes were necessary because we
don't support the PseudoElement interface and instead we check which animation names and CSS properties
are applied to pseudo-elements rather than the pseudo-element's type.

* css-animations/test_document-get-animations-expected.txt:
* css-animations/test_document-get-animations.html:
* css-animations/test_event-dispatch.html: Remove a comment that was added by mistake in a previous patch.
* css-transitions/test_document-get-animations-expected.txt:
* css-transitions/test_document-get-animations.html:

Source/WebCore:

The Web Animations spec has the notion of "composite order" which determines the order in which animations should
be returned when document.getAnimations() is called. The CSS Transitions and CSS Animations specifications also
determine the composite order of their respective animation classes, as well as the notion of "owning element",
the element that was the animation target when specified through style, prior to any manipulation via the Web
Animations API. We implement these two notions so that we can pass the document.getAnimations() tests for
declarative animations.

It is important that we have the notion of an "owning element" since when a CSS Transition or CSS Animation is
modified via the Web Animations API in a way that an animation created through CSS we must consider no longer
as a declarative animation but as a regular Web Animation. In this patch, we remove the DeclarativeAnimation's
target(), which returned a reference, to owningElement(), which returns a pointer and return nullptr once the
declarative animation has been modified.

In order to also keep a correct count of declarative animations applied to elements, we correctly add transitions
that have completed to a list of completed transitions, as specified by the CSS Transitions spec. We have had this
list declared in AnimationTimeline.h for a while but never actually did the work to add items to it. Thanks to that,
AnimationTimeline::updateCSSTransitionsForElement() now correctly accounts for completed transitions so that they
may be canceled if overridden with a new animation, correctly removing their "owning element".

Finally, now that we have saner lists of animations per classes, we can apply the right sorting routines to match
the composite order for CSS Transitions, CSS Animations and Web Animations, keeping a list of all animations created
in order as the final criterion for sorting.

* animation/AnimationTimeline.cpp:
(WebCore::AnimationTimeline::animationTimingDidChange): Make sure this animation is tracked on the list of all animations
created for this timeline in the order in which they were created, which is needed to sort animations correctly when
document.getAnimations() is called.
(WebCore::AnimationTimeline::animationWasAddedToElement): We've removed the relevantMapForAnimation() method which we used
to determine which map we should add an animation to based on its class and instead have code that accounts for not just
the animation's class, but also whether it has an owning element since a valid owning element is required to qualify as
a CSS Transition or CSS Animation, regardless of the animation's original class.
(WebCore::removeAnimationFromMapForElement): Adding this helper to remove an animation from the provided animation map so
that animationWasRemovedFromElement() can call this with all of the various animation maps.
(WebCore::AnimationTimeline::animationWasRemovedFromElement): Check all of the various animation maps to see which may
contain the animation we're trying to remove as the owning element might have been cleared by the time this function is
called and looking at the animation's class would not be sufficient to determine which animation map the animation was in.
(WebCore::AnimationTimeline::removeDeclarativeAnimationFromListsForOwningElement): New function in which the logic from
animationWasRemovedFromElement() that dealt with removing animations from the list of running CSS Animations/Transitions as
well as completed CSS Transitions was factored out. This allowed us to also call this function from
DeclarativeAnimation::disassociateFromOwningElement().
(WebCore::AnimationTimeline::elementWasRemoved): We no longer need to remove an animation as canceling it will remove it
correctly when DocumentTimeline::updateAnimationsAndSendEvents() is called.
(WebCore::AnimationTimeline::updateCSSAnimationsForElement): Call cancelDeclarativeAnimation() instead of the removed
cancelOrRemoveDeclarativeAnimation() when a CSS Animation should be canceled.
(WebCore::AnimationTimeline::ensureRunningTransitionsByProperty): Now that we correctly remove transitions from the list
of running transitions once they've completed or have been canceled, we need a helper to always get a valid map of running
transitions for a given element as that map can be cleared while updateCSSTransitionsForElement() is running.
(WebCore::AnimationTimeline::updateCSSTransitionsForElement): Call cancelDeclarativeAnimation() instead of the removed
cancelOrRemoveDeclarativeAnimation() when a CSS Transition should be canceled. Additionally we always get the list of running
transitions for a given element as it can be cleared by a prior cancelation.
(WebCore::AnimationTimeline::cancelDeclarativeAnimation): New function replacing cancelOrRemoveDeclarativeAnimation() in which
we call the new DeclarativeAnimation::cancelFromStyle() function on the provided animation and then remove the animation from
all known lists, including the new list of all animations. We do this final part so that the animation gets re-added as if it
were a new animation since canceling a declarative animation via a style change removes its declarative-ness. This guarantees
that a declarative animation that is canceled through style but then resumed through the Web Animations API sorts after any
declarative animation with an owning element.
(WebCore::AnimationTimeline::relevantMapForAnimation): Deleted.
(WebCore::AnimationTimeline::cancelOrRemoveDeclarativeAnimation): Deleted.
* animation/AnimationTimeline.h:
(WebCore::AnimationTimeline::timingModelDidChange): Deleted. This was left over in the patch where we implemented the "update
animations and send events" procedure.
(WebCore::AnimationTimeline::animations const): Deleted.
* animation/CSSAnimation.cpp:
(WebCore::CSSAnimation::create): Some refactoring to make the handling of a declarative animation's owning element part of the
DeclarativeAnimation constructor.
* animation/CSSTransition.cpp:
(WebCore::CSSTransition::create): Some refactoring to make the handling of a declarative animation's owning element part of the
DeclarativeAnimation constructor.
* animation/DeclarativeAnimation.cpp:
(WebCore::DeclarativeAnimation::DeclarativeAnimation):
(WebCore::DeclarativeAnimation::tick): Make sure we disassociate from the animation's owning element once we transition from a
relevant state to an idle state. This will catch any changes made via the Web Animations API to a declarative animation when the
"update animations and send events" procedure is run.
(WebCore::DeclarativeAnimation::disassociateFromOwningElement): Remove this animation from the list of declarative animations on
the associated timeline and make owningElement() nullptr so that document.getAnimations() will know to sort this animation with other
Web Animations created via the Web Animations API.
(WebCore::DeclarativeAnimation::initialize):
(WebCore::DeclarativeAnimation::cancelFromStyle): New method called from AnimationTimeline::cancelDeclarativeAnimation() which
cancels the animation but also disassociates it from its owning element.
(WebCore::DeclarativeAnimation::invalidateDOMEvents):
(WebCore::DeclarativeAnimation::enqueueDOMEvent):
* animation/DeclarativeAnimation.h:
(WebCore::DeclarativeAnimation::owningElement const):
(WebCore::DeclarativeAnimation::target const): Deleted.
* animation/DocumentTimeline.cpp:
(WebCore::compareDeclarativeAnimationOwningElementPositionsInDocumentTreeOrder): Adding a new utility used when sorting both
CSS Transitions and CSS Animations by tree order when their owning element differ, with special logic for pseudo-elements.
(WebCore::DocumentTimeline::getAnimations const): Filter and sort animations according to their composite order.
(WebCore::DocumentTimeline::updateAnimationsAndSendEvents): Compile a list of transitions that move to a finished state to
the list of completed transitions so that they may be canceled correctly in AnimationTimeline::updateCSSTransitionsForElement().
(WebCore::DocumentTimeline::transitionDidComplete):
* animation/DocumentTimeline.h:

LayoutTests:

Mark that the two document.getAnimations() tests for declarative animations are no longer flaky.

* TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (237725 => 237726)


--- trunk/LayoutTests/ChangeLog	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/LayoutTests/ChangeLog	2018-11-02 07:54:19 UTC (rev 237726)
@@ -1,3 +1,14 @@
+2018-11-01  Antoine Quint  <grao...@apple.com>
+
+        [Web Animations] Make document.getAnimations() return declarative animations in the correct order
+        https://bugs.webkit.org/show_bug.cgi?id=191153
+
+        Reviewed by Dean Jackson.
+
+        Mark that the two document.getAnimations() tests for declarative animations are no longer flaky.
+
+        * TestExpectations:
+
 2018-11-02  Justin Fan  <justin_...@apple.com>
 
         [WebGPU] Experimental prototype for MSL shaders

Modified: trunk/LayoutTests/TestExpectations (237725 => 237726)


--- trunk/LayoutTests/TestExpectations	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/LayoutTests/TestExpectations	2018-11-02 07:54:19 UTC (rev 237726)
@@ -2047,8 +2047,6 @@
 webkit.org/b/181888 imported/w3c/web-platform-tests/web-animations/timing-model/animation-effects/current-iteration.html [ Pass Failure ]
 
 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/183840 imported/mozilla/css-animations/test_document-get-animations.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 (237725 => 237726)


--- trunk/LayoutTests/imported/mozilla/ChangeLog	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/LayoutTests/imported/mozilla/ChangeLog	2018-11-02 07:54:19 UTC (rev 237726)
@@ -1,3 +1,22 @@
+2018-11-01  Antoine Quint  <grao...@apple.com>
+
+        [Web Animations] Make document.getAnimations() return declarative animations in the correct order
+        https://bugs.webkit.org/show_bug.cgi?id=191153
+
+        Reviewed by Dean Jackson.
+
+        Mark progressions for the document.getAnimations() tests covering CSS Animations and CSS Transitions.
+        These tests are now in WPT and will be updated as part of a general WPT update for all of Web Animations
+        shortly. We will upstream the minor changes made at that point. Those changes were necessary because we
+        don't support the PseudoElement interface and instead we check which animation names and CSS properties
+        are applied to pseudo-elements rather than the pseudo-element's type.
+
+        * css-animations/test_document-get-animations-expected.txt:
+        * css-animations/test_document-get-animations.html:
+        * css-animations/test_event-dispatch.html: Remove a comment that was added by mistake in a previous patch.
+        * css-transitions/test_document-get-animations-expected.txt:
+        * css-transitions/test_document-get-animations.html:
+
 2018-10-28  Antoine Quint  <grao...@apple.com>
 
         [Web Animations] Implement the update animations and send events procedure

Modified: trunk/LayoutTests/imported/mozilla/css-animations/test_document-get-animations-expected.txt (237725 => 237726)


--- trunk/LayoutTests/imported/mozilla/css-animations/test_document-get-animations-expected.txt	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/LayoutTests/imported/mozilla/css-animations/test_document-get-animations-expected.txt	2018-11-02 07:54:19 UTC (rev 237726)
@@ -2,15 +2,15 @@
 PASS getAnimations for non-animated content 
 PASS getAnimations for CSS Animations 
 PASS Order of CSS Animations - within an element 
-FAIL Order of CSS Animations - across elements assert_equals: Order of second animation returned after tree surgery expected Element node <div style="animation: animLeft 100s"></div> but got Element node <div style="animation: animLeft 100s"></div>
+PASS Order of CSS Animations - across elements 
 PASS Order of CSS Animations - across and within elements 
 PASS Order of CSS Animations - markup-bound vs free animations 
 PASS Order of CSS Animations - free animations 
-FAIL Order of CSS Animations and CSS Transitions assert_equals: Transition comes first expected "[object CSSTransition]" but got "[object CSSAnimation]"
+PASS Order of CSS Animations and CSS Transitions 
 PASS Finished but filling CSS Animations are returned 
 PASS Finished but not filling CSS Animations are not returned 
 PASS Yet-to-start CSS Animations are returned 
 PASS CSS Animations cancelled via the API are not returned 
 PASS CSS Animations cancelled and restarted via the API are returned 
-FAIL CSS Animations targetting (pseudo-)elements should have correct order after sorting assert_equals: The animation targeting the ::before element comes second expected (string) "::before" but got (undefined) undefined
+PASS CSS Animations targetting (pseudo-)elements should have correct order after sorting 
 

Modified: trunk/LayoutTests/imported/mozilla/css-animations/test_document-get-animations.html (237725 => 237726)


--- trunk/LayoutTests/imported/mozilla/css-animations/test_document-get-animations.html	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/LayoutTests/imported/mozilla/css-animations/test_document-get-animations.html	2018-11-02 07:54:19 UTC (rev 237726)
@@ -259,7 +259,7 @@
   //        |
   //       child
   var parent = addDiv(t, { 'id': 'parent' });
-  var child = addDiv(t);
+  var child = addDiv(t, { 'id': 'child' });
   parent.appendChild(child);
   [parent, child].forEach((div) => {
     div.setAttribute('style', 'animation: animBottom 10s');
@@ -271,9 +271,9 @@
                 'are returned');
   assert_equals(anims[0].effect.target, parent,
                 'The animation targeting the parent element comes first');
-  assert_equals(anims[1].effect.target.type, '::before',
+  assert_equals(anims[1].animationName, 'animRight',
                 'The animation targeting the ::before element comes second');
-  assert_equals(anims[2].effect.target.type, '::after',
+  assert_equals(anims[2].animationName, 'animLeft',
                 'The animation targeting the ::after element comes third');
   assert_equals(anims[3].effect.target, child,
                 'The animation targeting the child element comes last');

Modified: trunk/LayoutTests/imported/mozilla/css-animations/test_event-dispatch.html (237725 => 237726)


--- trunk/LayoutTests/imported/mozilla/css-animations/test_event-dispatch.html	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/LayoutTests/imported/mozilla/css-animations/test_event-dispatch.html	2018-11-02 07:54:19 UTC (rev 237726)
@@ -307,7 +307,6 @@
   });
 }, 'Call Animation.cancel after cancelling animation.');
 
-// FIXME: timeout
 promise_test(t => {
   const { animation, watcher, div } = setupAnimation(t, 'anim 1s');
 

Modified: trunk/LayoutTests/imported/mozilla/css-transitions/test_document-get-animations-expected.txt (237725 => 237726)


--- trunk/LayoutTests/imported/mozilla/css-transitions/test_document-get-animations-expected.txt	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/LayoutTests/imported/mozilla/css-transitions/test_document-get-animations-expected.txt	2018-11-02 07:54:19 UTC (rev 237726)
@@ -1,6 +1,6 @@
 
 PASS getAnimations for non-animated content 
 PASS getAnimations for CSS Transitions 
-FAIL CSS Transitions targetting (pseudo-)elements should have correct order after sorting assert_equals: The animation targeting the ::before element comes second expected (string) "::before" but got (undefined) undefined
+PASS CSS Transitions targetting (pseudo-)elements should have correct order after sorting 
 PASS Transitions are not returned after they have finished 
 

Modified: trunk/LayoutTests/imported/mozilla/css-transitions/test_document-get-animations.html (237725 => 237726)


--- trunk/LayoutTests/imported/mozilla/css-transitions/test_document-get-animations.html	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/LayoutTests/imported/mozilla/css-transitions/test_document-get-animations.html	2018-11-02 07:54:19 UTC (rev 237726)
@@ -37,10 +37,10 @@
 test(function(t) {
   addStyle(t, { '.init::after': 'content: ""; width: 0px; ' +
                                 'transition: all 100s;',
-                '.init::before': 'content: ""; width: 0px; ' +
+                '.init::before': 'content: ""; height: 0px; ' +
                                  'transition: all 10s;',
                 '.change::after': 'width: 100px;',
-                '.change::before': 'width: 100px;' });
+                '.change::before': 'height: 100px;' });
   // create two divs with these arrangement:
   //       parent
   //     ::before,
@@ -68,9 +68,9 @@
                 'are returned');
   assert_equals(anims[0].effect.target, parent,
                 'The animation targeting the parent element comes first');
-  assert_equals(anims[1].effect.target.type, '::before',
+  assert_equals(anims[1].transitionProperty, 'height',
                 'The animation targeting the ::before element comes second');
-  assert_equals(anims[2].effect.target.type, '::after',
+  assert_equals(anims[2].transitionProperty, 'width',
                 'The animation targeting the ::after element comes third');
   assert_equals(anims[3].effect.target, child,
                 'The animation targeting the child element comes last');

Modified: trunk/Source/WebCore/ChangeLog (237725 => 237726)


--- trunk/Source/WebCore/ChangeLog	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/Source/WebCore/ChangeLog	2018-11-02 07:54:19 UTC (rev 237726)
@@ -1,3 +1,103 @@
+2018-11-01  Antoine Quint  <grao...@apple.com>
+
+        [Web Animations] Make document.getAnimations() return declarative animations in the correct order
+        https://bugs.webkit.org/show_bug.cgi?id=191153
+
+        Reviewed by Dean Jackson.
+
+        The Web Animations spec has the notion of "composite order" which determines the order in which animations should
+        be returned when document.getAnimations() is called. The CSS Transitions and CSS Animations specifications also
+        determine the composite order of their respective animation classes, as well as the notion of "owning element",
+        the element that was the animation target when specified through style, prior to any manipulation via the Web
+        Animations API. We implement these two notions so that we can pass the document.getAnimations() tests for
+        declarative animations.
+
+        It is important that we have the notion of an "owning element" since when a CSS Transition or CSS Animation is
+        modified via the Web Animations API in a way that an animation created through CSS we must consider no longer
+        as a declarative animation but as a regular Web Animation. In this patch, we remove the DeclarativeAnimation's
+        target(), which returned a reference, to owningElement(), which returns a pointer and return nullptr once the
+        declarative animation has been modified.
+
+        In order to also keep a correct count of declarative animations applied to elements, we correctly add transitions
+        that have completed to a list of completed transitions, as specified by the CSS Transitions spec. We have had this
+        list declared in AnimationTimeline.h for a while but never actually did the work to add items to it. Thanks to that,
+        AnimationTimeline::updateCSSTransitionsForElement() now correctly accounts for completed transitions so that they
+        may be canceled if overridden with a new animation, correctly removing their "owning element".
+
+        Finally, now that we have saner lists of animations per classes, we can apply the right sorting routines to match
+        the composite order for CSS Transitions, CSS Animations and Web Animations, keeping a list of all animations created
+        in order as the final criterion for sorting.
+
+        * animation/AnimationTimeline.cpp:
+        (WebCore::AnimationTimeline::animationTimingDidChange): Make sure this animation is tracked on the list of all animations
+        created for this timeline in the order in which they were created, which is needed to sort animations correctly when
+        document.getAnimations() is called.
+        (WebCore::AnimationTimeline::animationWasAddedToElement): We've removed the relevantMapForAnimation() method which we used
+        to determine which map we should add an animation to based on its class and instead have code that accounts for not just
+        the animation's class, but also whether it has an owning element since a valid owning element is required to qualify as
+        a CSS Transition or CSS Animation, regardless of the animation's original class.
+        (WebCore::removeAnimationFromMapForElement): Adding this helper to remove an animation from the provided animation map so
+        that animationWasRemovedFromElement() can call this with all of the various animation maps.
+        (WebCore::AnimationTimeline::animationWasRemovedFromElement): Check all of the various animation maps to see which may
+        contain the animation we're trying to remove as the owning element might have been cleared by the time this function is
+        called and looking at the animation's class would not be sufficient to determine which animation map the animation was in.
+        (WebCore::AnimationTimeline::removeDeclarativeAnimationFromListsForOwningElement): New function in which the logic from
+        animationWasRemovedFromElement() that dealt with removing animations from the list of running CSS Animations/Transitions as
+        well as completed CSS Transitions was factored out. This allowed us to also call this function from
+        DeclarativeAnimation::disassociateFromOwningElement().
+        (WebCore::AnimationTimeline::elementWasRemoved): We no longer need to remove an animation as canceling it will remove it
+        correctly when DocumentTimeline::updateAnimationsAndSendEvents() is called.
+        (WebCore::AnimationTimeline::updateCSSAnimationsForElement): Call cancelDeclarativeAnimation() instead of the removed
+        cancelOrRemoveDeclarativeAnimation() when a CSS Animation should be canceled.
+        (WebCore::AnimationTimeline::ensureRunningTransitionsByProperty): Now that we correctly remove transitions from the list
+        of running transitions once they've completed or have been canceled, we need a helper to always get a valid map of running
+        transitions for a given element as that map can be cleared while updateCSSTransitionsForElement() is running. 
+        (WebCore::AnimationTimeline::updateCSSTransitionsForElement): Call cancelDeclarativeAnimation() instead of the removed
+        cancelOrRemoveDeclarativeAnimation() when a CSS Transition should be canceled. Additionally we always get the list of running
+        transitions for a given element as it can be cleared by a prior cancelation.
+        (WebCore::AnimationTimeline::cancelDeclarativeAnimation): New function replacing cancelOrRemoveDeclarativeAnimation() in which
+        we call the new DeclarativeAnimation::cancelFromStyle() function on the provided animation and then remove the animation from
+        all known lists, including the new list of all animations. We do this final part so that the animation gets re-added as if it
+        were a new animation since canceling a declarative animation via a style change removes its declarative-ness. This guarantees
+        that a declarative animation that is canceled through style but then resumed through the Web Animations API sorts after any
+        declarative animation with an owning element.
+        (WebCore::AnimationTimeline::relevantMapForAnimation): Deleted.
+        (WebCore::AnimationTimeline::cancelOrRemoveDeclarativeAnimation): Deleted.
+        * animation/AnimationTimeline.h:
+        (WebCore::AnimationTimeline::timingModelDidChange): Deleted. This was left over in the patch where we implemented the "update
+        animations and send events" procedure.
+        (WebCore::AnimationTimeline::animations const): Deleted.
+        * animation/CSSAnimation.cpp:
+        (WebCore::CSSAnimation::create): Some refactoring to make the handling of a declarative animation's owning element part of the
+        DeclarativeAnimation constructor.
+        * animation/CSSTransition.cpp:
+        (WebCore::CSSTransition::create): Some refactoring to make the handling of a declarative animation's owning element part of the
+        DeclarativeAnimation constructor.
+        * animation/DeclarativeAnimation.cpp:
+        (WebCore::DeclarativeAnimation::DeclarativeAnimation):
+        (WebCore::DeclarativeAnimation::tick): Make sure we disassociate from the animation's owning element once we transition from a
+        relevant state to an idle state. This will catch any changes made via the Web Animations API to a declarative animation when the
+        "update animations and send events" procedure is run.
+        (WebCore::DeclarativeAnimation::disassociateFromOwningElement): Remove this animation from the list of declarative animations on
+        the associated timeline and make owningElement() nullptr so that document.getAnimations() will know to sort this animation with other
+        Web Animations created via the Web Animations API.
+        (WebCore::DeclarativeAnimation::initialize):
+        (WebCore::DeclarativeAnimation::cancelFromStyle): New method called from AnimationTimeline::cancelDeclarativeAnimation() which
+        cancels the animation but also disassociates it from its owning element.
+        (WebCore::DeclarativeAnimation::invalidateDOMEvents):
+        (WebCore::DeclarativeAnimation::enqueueDOMEvent):
+        * animation/DeclarativeAnimation.h:
+        (WebCore::DeclarativeAnimation::owningElement const):
+        (WebCore::DeclarativeAnimation::target const): Deleted.
+        * animation/DocumentTimeline.cpp:
+        (WebCore::compareDeclarativeAnimationOwningElementPositionsInDocumentTreeOrder): Adding a new utility used when sorting both
+        CSS Transitions and CSS Animations by tree order when their owning element differ, with special logic for pseudo-elements.
+        (WebCore::DocumentTimeline::getAnimations const): Filter and sort animations according to their composite order.
+        (WebCore::DocumentTimeline::updateAnimationsAndSendEvents): Compile a list of transitions that move to a finished state to
+        the list of completed transitions so that they may be canceled correctly in AnimationTimeline::updateCSSTransitionsForElement().
+        (WebCore::DocumentTimeline::transitionDidComplete):
+        * animation/DocumentTimeline.h:
+
 2018-11-02  Zan Dobersek  <zdober...@igalia.com>
 
         PNGImageDecoder: report no repetition for non-animated images

Modified: trunk/Source/WebCore/animation/AnimationTimeline.cpp (237725 => 237726)


--- trunk/Source/WebCore/animation/AnimationTimeline.cpp	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/Source/WebCore/animation/AnimationTimeline.cpp	2018-11-02 07:54:19 UTC (rev 237726)
@@ -58,6 +58,7 @@
 void AnimationTimeline::animationTimingDidChange(WebAnimation& animation)
 {
     if (m_animations.add(&animation)) {
+        m_allAnimations.add(&animation);
         auto* timeline = animation.timeline();
         if (timeline && timeline != this)
             timeline->removeAnimation(animation);
@@ -82,23 +83,20 @@
     return secondsToWebAnimationsAPITime(*time);
 }
 
-HashMap<Element*, ListHashSet<RefPtr<WebAnimation>>>& AnimationTimeline::relevantMapForAnimation(WebAnimation& animation)
-{
-    if (animation.isCSSAnimation())
-        return m_elementToCSSAnimationsMap;
-    if (animation.isCSSTransition())
-        return m_elementToCSSTransitionsMap;
-    return m_elementToAnimationsMap;
-}
-
 void AnimationTimeline::animationWasAddedToElement(WebAnimation& animation, Element& element)
 {
-    relevantMapForAnimation(animation).ensure(&element, [] {
+    [&] () -> ElementToAnimationsMap& {
+        if (is<CSSTransition>(animation) && downcast<CSSTransition>(animation).owningElement())
+            return m_elementToCSSTransitionsMap;
+        if (is<CSSAnimation>(animation) && downcast<CSSAnimation>(animation).owningElement())
+            return m_elementToCSSAnimationsMap;
+        return m_elementToAnimationsMap;
+    }().ensure(&element, [] {
         return ListHashSet<RefPtr<WebAnimation>> { };
     }).iterator->value.add(&animation);
 }
 
-static inline bool removeCSSTransitionFromMap(CSSTransition& transition, Element& element, HashMap<Element*, HashMap<CSSPropertyID, RefPtr<CSSTransition>>>& map)
+static inline bool removeCSSTransitionFromMap(CSSTransition& transition, Element& element, HashMap<Element*, AnimationTimeline::PropertyToTransitionMap>& map)
 {
     auto iterator = map.find(&element);
     if (iterator == map.end())
@@ -117,12 +115,8 @@
     return true;
 }
 
-void AnimationTimeline::animationWasRemovedFromElement(WebAnimation& animation, Element& element)
+static inline void removeAnimationFromMapForElement(WebAnimation& animation, AnimationTimeline::ElementToAnimationsMap& map, Element& element)
 {
-    // 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.
-    auto& map = relevantMapForAnimation(animation);
     auto iterator = map.find(&element);
     if (iterator == map.end())
         return;
@@ -131,9 +125,24 @@
     animations.remove(&animation);
     if (!animations.size())
         map.remove(iterator);
+}
 
+void AnimationTimeline::animationWasRemovedFromElement(WebAnimation& animation, Element& element)
+{
+    removeAnimationFromMapForElement(animation, m_elementToCSSTransitionsMap, element);
+    removeAnimationFromMapForElement(animation, m_elementToCSSAnimationsMap, element);
+    removeAnimationFromMapForElement(animation, m_elementToAnimationsMap, element);
+
     // Now, if we're dealing with a declarative animation, we remove it from either the m_elementToCSSAnimationByName
     // or the m_elementToRunningCSSTransitionByCSSPropertyID map, whichever is relevant to this type of animation.
+    if (is<DeclarativeAnimation>(animation))
+        removeDeclarativeAnimationFromListsForOwningElement(animation, element);
+}
+
+void AnimationTimeline::removeDeclarativeAnimationFromListsForOwningElement(WebAnimation& animation, Element& element)
+{
+    ASSERT(is<DeclarativeAnimation>(animation));
+
     if (is<CSSAnimation>(animation)) {
         auto iterator = m_elementToCSSAnimationByName.find(&element);
         if (iterator != m_elementToCSSAnimationByName.end()) {
@@ -184,11 +193,8 @@
 
 void AnimationTimeline::elementWasRemoved(Element& element)
 {
-    for (auto& animation : animationsForElement(element)) {
-        animationWasRemovedFromElement(*animation, element);
-        removeAnimation(*animation);
+    for (auto& animation : animationsForElement(element))
         animation->cancel(WebAnimation::Silently::Yes);
-    }
 }
 
 void AnimationTimeline::removeAnimationsForElement(Element& element)
@@ -228,7 +234,7 @@
     if (currentStyle && currentStyle->hasAnimations() && currentStyle->display() != DisplayType::None && afterChangeStyle.display() == DisplayType::None) {
         if (m_elementToCSSAnimationByName.contains(&element)) {
             for (const auto& cssAnimationsByNameMapItem : m_elementToCSSAnimationByName.take(&element))
-                cancelOrRemoveDeclarativeAnimation(cssAnimationsByNameMapItem.value);
+                cancelDeclarativeAnimation(*cssAnimationsByNameMapItem.value);
         }
         return;
     }
@@ -275,7 +281,7 @@
     // remove the CSSAnimation object created for them.
     for (const auto& nameOfAnimationToRemove : namesOfPreviousAnimations) {
         if (auto animation = cssAnimationsByName.take(nameOfAnimationToRemove))
-            cancelOrRemoveDeclarativeAnimation(animation);
+            cancelDeclarativeAnimation(*animation);
     }
 }
 
@@ -290,7 +296,7 @@
     return matchingAnimation;
 }
 
-static bool propertyInStyleMatchesValueForTransitionInMap(CSSPropertyID property, const RenderStyle& style, HashMap<CSSPropertyID, RefPtr<CSSTransition>>& transitions)
+static bool propertyInStyleMatchesValueForTransitionInMap(CSSPropertyID property, const RenderStyle& style, AnimationTimeline::PropertyToTransitionMap& transitions)
 {
     if (auto* transition = transitions.get(property)) {
         if (CSSPropertyAnimation::propertiesEqual(property, &style, &transition->targetStyle()))
@@ -323,6 +329,13 @@
     return true;
 }
 
+AnimationTimeline::PropertyToTransitionMap& AnimationTimeline::ensureRunningTransitionsByProperty(Element& element)
+{
+    return m_elementToRunningCSSTransitionByCSSPropertyID.ensure(&element, [] {
+        return PropertyToTransitionMap { };
+    }).iterator->value;
+}
+
 void AnimationTimeline::updateCSSTransitionsForElement(Element& element, const RenderStyle& currentStyle, const RenderStyle& afterChangeStyle)
 {
     // In case this element is newly getting a "display: none" we need to cancel all of its transitions and disregard new ones.
@@ -329,7 +342,7 @@
     if (currentStyle.hasTransitions() && currentStyle.display() != DisplayType::None && afterChangeStyle.display() == DisplayType::None) {
         if (m_elementToRunningCSSTransitionByCSSPropertyID.contains(&element)) {
             for (const auto& cssTransitionsByCSSPropertyIDMapItem : m_elementToRunningCSSTransitionByCSSPropertyID.take(&element))
-                cancelOrRemoveDeclarativeAnimation(cssTransitionsByCSSPropertyIDMapItem.value);
+                cancelDeclarativeAnimation(*cssTransitionsByCSSPropertyIDMapItem.value);
         }
         return;
     }
@@ -337,12 +350,10 @@
     // Section 3 "Starting of transitions" from the CSS Transitions Level 1 specification.
     // https://drafts.csswg.org/css-transitions-1/#starting
 
-    auto& runningTransitionsByProperty = m_elementToRunningCSSTransitionByCSSPropertyID.ensure(&element, [] {
-        return HashMap<CSSPropertyID, RefPtr<CSSTransition>> { };
-    }).iterator->value;
+    auto& runningTransitionsByProperty = ensureRunningTransitionsByProperty(element);
 
     auto& completedTransitionsByProperty = m_elementToCompletedCSSTransitionByCSSPropertyID.ensure(&element, [] {
-        return HashMap<CSSPropertyID, RefPtr<CSSTransition>> { };
+        return PropertyToTransitionMap { };
     }).iterator->value;
 
     auto generationTime = MonotonicTime::now();
@@ -420,15 +431,15 @@
             if (CSSPropertyAnimation::propertiesEqual(property, &previouslyRunningTransitionCurrentStyle, &afterChangeStyle) || !CSSPropertyAnimation::canPropertyBeInterpolated(property, &currentStyle, &afterChangeStyle)) {
                 // 1. If the current value of the property in the running transition is equal to the value of the property in the after-change style,
                 //    or if these two values cannot be interpolated, then implementations must cancel the running transition.
-                previouslyRunningTransition->cancel();
+                cancelDeclarativeAnimation(*previouslyRunningTransition);
             } else if (transitionCombinedDuration(matchingBackingAnimation) <= 0.0 || !CSSPropertyAnimation::canPropertyBeInterpolated(property, &previouslyRunningTransitionCurrentStyle, &afterChangeStyle)) {
                 // 2. Otherwise, if the combined duration is less than or equal to 0s, or if the current value of the property in the running transition
                 //    cannot be interpolated with the value of the property in the after-change style, then implementations must cancel the running transition.
-                previouslyRunningTransition->cancel();
+                cancelDeclarativeAnimation(*previouslyRunningTransition);
             } else if (CSSPropertyAnimation::propertiesEqual(property, &previouslyRunningTransition->reversingAdjustedStartStyle(), &afterChangeStyle)) {
                 // 3. Otherwise, if the reversing-adjusted start value of the running transition is the same as the value of the property in the after-change
                 //    style (see the section on reversing of transitions for why these case exists), implementations must cancel the running transition
-                previouslyRunningTransition->cancel();
+                cancelDeclarativeAnimation(*previouslyRunningTransition);
 
                 // and start a new transition whose:
                 //   - reversing-adjusted start value is the end value of the running transition,
@@ -448,10 +459,11 @@
                 auto reversingShorteningFactor = std::max(std::min(((transformedProgress * previouslyRunningTransition->reversingShorteningFactor()) + (1 - previouslyRunningTransition->reversingShorteningFactor())), 1.0), 0.0);
                 auto delay = matchingBackingAnimation->delay() < 0 ? Seconds(matchingBackingAnimation->delay()) * reversingShorteningFactor : Seconds(matchingBackingAnimation->delay());
                 auto duration = Seconds(matchingBackingAnimation->duration()) * reversingShorteningFactor;
-                runningTransitionsByProperty.set(property, CSSTransition::create(element, property, generationTime, *matchingBackingAnimation, &previouslyRunningTransitionCurrentStyle, afterChangeStyle, delay, duration, reversingAdjustedStartStyle, reversingShorteningFactor));
+
+                ensureRunningTransitionsByProperty(element).set(property, CSSTransition::create(element, property, generationTime, *matchingBackingAnimation, &previouslyRunningTransitionCurrentStyle, afterChangeStyle, delay, duration, reversingAdjustedStartStyle, reversingShorteningFactor));
             } else {
                 // 4. Otherwise, implementations must cancel the running transition
-                previouslyRunningTransition->cancel();
+                cancelDeclarativeAnimation(*previouslyRunningTransition);
 
                 // and start a new transition whose:
                 //   - start time is the time of the style change event plus the matching transition delay,
@@ -464,19 +476,17 @@
                 auto duration = Seconds(matchingBackingAnimation->duration());
                 auto& reversingAdjustedStartStyle = currentStyle;
                 auto reversingShorteningFactor = 1;
-                runningTransitionsByProperty.set(property, CSSTransition::create(element, property, generationTime, *matchingBackingAnimation, &previouslyRunningTransitionCurrentStyle, afterChangeStyle, delay, duration, reversingAdjustedStartStyle, reversingShorteningFactor));
+                ensureRunningTransitionsByProperty(element).set(property, CSSTransition::create(element, property, generationTime, *matchingBackingAnimation, &previouslyRunningTransitionCurrentStyle, afterChangeStyle, delay, duration, reversingAdjustedStartStyle, reversingShorteningFactor));
             }
         }
     }
-
 }
 
-void AnimationTimeline::cancelOrRemoveDeclarativeAnimation(RefPtr<DeclarativeAnimation> animation)
+void AnimationTimeline::cancelDeclarativeAnimation(DeclarativeAnimation& animation)
 {
-    ASSERT(animation);
-    animation->cancel();
-    animationWasRemovedFromElement(*animation, animation->target());
-    removeAnimation(*animation);
+    animation.cancelFromStyle();
+    removeAnimation(animation);
+    m_allAnimations.remove(&animation);
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/animation/AnimationTimeline.h (237725 => 237726)


--- trunk/Source/WebCore/animation/AnimationTimeline.h	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/Source/WebCore/animation/AnimationTimeline.h	2018-11-02 07:54:19 UTC (rev 237726)
@@ -54,10 +54,7 @@
     std::optional<double> bindingsCurrentTime();
     virtual std::optional<Seconds> currentTime() { return m_currentTime; }
 
-    virtual void timingModelDidChange() { };
-
     enum class Ordering { Sorted, Unsorted };
-    const ListHashSet<RefPtr<WebAnimation>>& animations() const { return m_animations; }
     Vector<RefPtr<WebAnimation>> animationsForElement(Element&, Ordering ordering = Ordering::Unsorted) const;
     void elementWasRemoved(Element&);
     void removeAnimationsForElement(Element&);
@@ -64,10 +61,14 @@
     void cancelDeclarativeAnimationsForElement(Element&);
     virtual void animationWasAddedToElement(WebAnimation&, Element&);
     virtual void animationWasRemovedFromElement(WebAnimation&, Element&);
+    void removeDeclarativeAnimationFromListsForOwningElement(WebAnimation&, Element&);
 
     void updateCSSAnimationsForElement(Element&, const RenderStyle* currentStyle, const RenderStyle& afterChangeStyle);
     void updateCSSTransitionsForElement(Element&, const RenderStyle& currentStyle, const RenderStyle& afterChangeStyle);
 
+    using ElementToAnimationsMap = HashMap<Element*, ListHashSet<RefPtr<WebAnimation>>>;
+    using PropertyToTransitionMap = HashMap<CSSPropertyID, RefPtr<CSSTransition>>;
+
     virtual ~AnimationTimeline();
 
 protected:
@@ -79,21 +80,22 @@
 
     explicit AnimationTimeline(ClassType);
 
+    ListHashSet<WebAnimation*> m_allAnimations;
     ListHashSet<RefPtr<WebAnimation>> m_animations;
+    HashMap<Element*, PropertyToTransitionMap> m_elementToCompletedCSSTransitionByCSSPropertyID;
 
 private:
-    HashMap<Element*, ListHashSet<RefPtr<WebAnimation>>>& relevantMapForAnimation(WebAnimation&);
-    void cancelOrRemoveDeclarativeAnimation(RefPtr<DeclarativeAnimation>);
     RefPtr<WebAnimation> cssAnimationForElementAndProperty(Element&, CSSPropertyID);
+    PropertyToTransitionMap& ensureRunningTransitionsByProperty(Element&);
+    void cancelDeclarativeAnimation(DeclarativeAnimation&);
 
     ClassType m_classType;
     std::optional<Seconds> m_currentTime;
-    HashMap<Element*, ListHashSet<RefPtr<WebAnimation>>> m_elementToAnimationsMap;
-    HashMap<Element*, ListHashSet<RefPtr<WebAnimation>>> m_elementToCSSAnimationsMap;
-    HashMap<Element*, ListHashSet<RefPtr<WebAnimation>>> m_elementToCSSTransitionsMap;
+    ElementToAnimationsMap m_elementToAnimationsMap;
+    ElementToAnimationsMap m_elementToCSSAnimationsMap;
+    ElementToAnimationsMap m_elementToCSSTransitionsMap;
     HashMap<Element*, HashMap<String, RefPtr<CSSAnimation>>> m_elementToCSSAnimationByName;
-    HashMap<Element*, HashMap<CSSPropertyID, RefPtr<CSSTransition>>> m_elementToRunningCSSTransitionByCSSPropertyID;
-    HashMap<Element*, HashMap<CSSPropertyID, RefPtr<CSSTransition>>> m_elementToCompletedCSSTransitionByCSSPropertyID;
+    HashMap<Element*, PropertyToTransitionMap> m_elementToRunningCSSTransitionByCSSPropertyID;
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/animation/CSSAnimation.cpp (237725 => 237726)


--- trunk/Source/WebCore/animation/CSSAnimation.cpp	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/Source/WebCore/animation/CSSAnimation.cpp	2018-11-02 07:54:19 UTC (rev 237726)
@@ -31,10 +31,10 @@
 
 namespace WebCore {
 
-Ref<CSSAnimation> CSSAnimation::create(Element& target, const Animation& backingAnimation, const RenderStyle* oldStyle, const RenderStyle& newStyle)
+Ref<CSSAnimation> CSSAnimation::create(Element& owningElement, const Animation& backingAnimation, const RenderStyle* oldStyle, const RenderStyle& newStyle)
 {
-    auto result = adoptRef(*new CSSAnimation(target, backingAnimation, newStyle));
-    result->initialize(target, oldStyle, newStyle);
+    auto result = adoptRef(*new CSSAnimation(owningElement, backingAnimation, newStyle));
+    result->initialize(oldStyle, newStyle);
     return result;
 }
 

Modified: trunk/Source/WebCore/animation/CSSTransition.cpp (237725 => 237726)


--- trunk/Source/WebCore/animation/CSSTransition.cpp	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/Source/WebCore/animation/CSSTransition.cpp	2018-11-02 07:54:19 UTC (rev 237726)
@@ -32,10 +32,10 @@
 
 namespace WebCore {
 
-Ref<CSSTransition> CSSTransition::create(Element& target, CSSPropertyID property, MonotonicTime generationTime, const Animation& backingAnimation, const RenderStyle* oldStyle, const RenderStyle& newStyle, Seconds delay, Seconds duration, const RenderStyle& reversingAdjustedStartStyle, double reversingShorteningFactor)
+Ref<CSSTransition> CSSTransition::create(Element& owningElement, CSSPropertyID property, MonotonicTime generationTime, const Animation& backingAnimation, const RenderStyle* oldStyle, const RenderStyle& newStyle, Seconds delay, Seconds duration, const RenderStyle& reversingAdjustedStartStyle, double reversingShorteningFactor)
 {
-    auto result = adoptRef(*new CSSTransition(target, property, generationTime, backingAnimation, newStyle, reversingAdjustedStartStyle, reversingShorteningFactor));
-    result->initialize(target, oldStyle, newStyle);
+    auto result = adoptRef(*new CSSTransition(owningElement, property, generationTime, backingAnimation, newStyle, reversingAdjustedStartStyle, reversingShorteningFactor));
+    result->initialize(oldStyle, newStyle);
     result->setTimingProperties(delay, duration);
     return result;
 }

Modified: trunk/Source/WebCore/animation/DeclarativeAnimation.cpp (237725 => 237726)


--- trunk/Source/WebCore/animation/DeclarativeAnimation.cpp	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/Source/WebCore/animation/DeclarativeAnimation.cpp	2018-11-02 07:54:19 UTC (rev 237726)
@@ -37,11 +37,11 @@
 
 namespace WebCore {
 
-DeclarativeAnimation::DeclarativeAnimation(Element& target, const Animation& backingAnimation)
-    : WebAnimation(target.document())
-    , m_target(target)
+DeclarativeAnimation::DeclarativeAnimation(Element& owningElement, const Animation& backingAnimation)
+    : WebAnimation(owningElement.document())
+    , m_owningElement(&owningElement)
     , m_backingAnimation(const_cast<Animation&>(backingAnimation))
-    , m_eventQueue(target)
+    , m_eventQueue(owningElement)
 {
 }
 
@@ -51,10 +51,31 @@
 
 void DeclarativeAnimation::tick()
 {
+    bool wasRelevant = isRelevant();
+    
     WebAnimation::tick();
     invalidateDOMEvents();
+
+    // If a declarative animation transitions from a non-idle state to an idle state, it means it was
+    // canceled using the Web Animations API and it should be disassociated from its owner element.
+    // From this point on, this animation is like any other animation and should not appear in the
+    // maps containing running CSS Transitions and CSS Animations for a given element.
+    if (wasRelevant && playState() == WebAnimation::PlayState::Idle) {
+        disassociateFromOwningElement();
+        m_eventQueue.close();
+    }
 }
 
+void DeclarativeAnimation::disassociateFromOwningElement()
+{
+    if (!m_owningElement)
+        return;
+
+    if (auto* animationTimeline = timeline())
+        animationTimeline->removeDeclarativeAnimationFromListsForOwningElement(*this, *m_owningElement);
+    m_owningElement = nullptr;
+}
+
 bool DeclarativeAnimation::needsTick() const
 {
     return WebAnimation::needsTick() || m_eventQueue.hasPendingEvents();
@@ -72,7 +93,7 @@
     syncPropertiesWithBackingAnimation();
 }
 
-void DeclarativeAnimation::initialize(const Element& target, const RenderStyle* oldStyle, const RenderStyle& newStyle)
+void DeclarativeAnimation::initialize(const RenderStyle* oldStyle, const RenderStyle& newStyle)
 {
     // We need to suspend invalidation of the animation's keyframe effect during its creation
     // as it would otherwise trigger invalidation of the document's style and this would be
@@ -79,8 +100,10 @@
     // incorrect since it would happen during style invalidation.
     suspendEffectInvalidation();
 
-    setEffect(KeyframeEffectReadOnly::create(target));
-    setTimeline(&target.document().timeline());
+    ASSERT(m_owningElement);
+
+    setEffect(KeyframeEffectReadOnly::create(*m_owningElement));
+    setTimeline(&m_owningElement->document().timeline());
     downcast<KeyframeEffectReadOnly>(effect())->computeDeclarativeAnimationBlendingKeyframes(oldStyle, newStyle);
     syncPropertiesWithBackingAnimation();
     if (backingAnimation().playState() == AnimationPlayState::Playing)
@@ -184,6 +207,12 @@
     invalidateDOMEvents(cancelationTime);
 }
 
+void DeclarativeAnimation::cancelFromStyle()
+{
+    cancel();
+    disassociateFromOwningElement();
+}
+
 AnimationEffectReadOnly::Phase DeclarativeAnimation::phaseWithoutEffect() const
 {
     // This shouldn't be called if we actually have an effect.
@@ -199,6 +228,9 @@
 
 void DeclarativeAnimation::invalidateDOMEvents(Seconds elapsedTime)
 {
+    if (!m_owningElement)
+        return;
+    
     auto* animationEffect = effect();
 
     auto isPending = pending();
@@ -282,11 +314,12 @@
 
 void DeclarativeAnimation::enqueueDOMEvent(const AtomicString& eventType, Seconds elapsedTime)
 {
+    ASSERT(m_owningElement);
     auto time = secondsToWebAnimationsAPITime(elapsedTime) / 1000;
     if (is<CSSAnimation>(this))
         m_eventQueue.enqueueEvent(AnimationEvent::create(eventType, downcast<CSSAnimation>(this)->animationName(), time));
     else if (is<CSSTransition>(this))
-        m_eventQueue.enqueueEvent(TransitionEvent::create(eventType, downcast<CSSTransition>(this)->transitionProperty(), time, PseudoElement::pseudoElementNameForEvents(m_target.pseudoId())));
+        m_eventQueue.enqueueEvent(TransitionEvent::create(eventType, downcast<CSSTransition>(this)->transitionProperty(), time, PseudoElement::pseudoElementNameForEvents(m_owningElement->pseudoId())));
 }
 
 void DeclarativeAnimation::stop()

Modified: trunk/Source/WebCore/animation/DeclarativeAnimation.h (237725 => 237726)


--- trunk/Source/WebCore/animation/DeclarativeAnimation.h	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/Source/WebCore/animation/DeclarativeAnimation.h	2018-11-02 07:54:19 UTC (rev 237726)
@@ -42,9 +42,10 @@
 
     bool isDeclarativeAnimation() const final { return true; }
 
-    Element& target() const { return m_target; }
+    Element* owningElement() const { return m_owningElement; }
     const Animation& backingAnimation() const { return m_backingAnimation; }
     void setBackingAnimation(const Animation&);
+    void cancelFromStyle();
 
     std::optional<double> startTime() const final;
     void setStartTime(std::optional<double>) final;
@@ -66,11 +67,12 @@
 protected:
     DeclarativeAnimation(Element&, const Animation&);
 
-    virtual void initialize(const Element&, const RenderStyle* oldStyle, const RenderStyle& newStyle);
+    virtual void initialize(const RenderStyle* oldStyle, const RenderStyle& newStyle);
     virtual void syncPropertiesWithBackingAnimation();
     void invalidateDOMEvents(Seconds elapsedTime = 0_s);
 
 private:
+    void disassociateFromOwningElement();
     void flushPendingStyleChanges() const;
     AnimationEffectReadOnly::Phase phaseWithoutEffect() const;
     void enqueueDOMEvent(const AtomicString&, Seconds);
@@ -81,7 +83,7 @@
     void resume() final;
     void stop() final;
 
-    Element& m_target;
+    Element* m_owningElement;
     Ref<Animation> m_backingAnimation;
     bool m_wasPending { false };
     AnimationEffectReadOnly::Phase m_previousPhase { AnimationEffectReadOnly::Phase::Idle };

Modified: trunk/Source/WebCore/animation/DocumentTimeline.cpp (237725 => 237726)


--- trunk/Source/WebCore/animation/DocumentTimeline.cpp	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/Source/WebCore/animation/DocumentTimeline.cpp	2018-11-02 07:54:19 UTC (rev 237726)
@@ -27,7 +27,9 @@
 #include "DocumentTimeline.h"
 
 #include "AnimationPlaybackEvent.h"
+#include "CSSAnimation.h"
 #include "CSSPropertyAnimation.h"
+#include "CSSTransition.h"
 #include "DOMWindow.h"
 #include "DeclarativeAnimation.h"
 #include "Document.h"
@@ -34,7 +36,9 @@
 #include "GraphicsLayer.h"
 #include "KeyframeEffect.h"
 #include "Microtasks.h"
+#include "Node.h"
 #include "Page.h"
+#include "PseudoElement.h"
 #include "RenderElement.h"
 #include "RenderLayer.h"
 #include "RenderLayerBacking.h"
@@ -81,20 +85,108 @@
     m_document = nullptr;
 }
 
+static inline bool compareDeclarativeAnimationOwningElementPositionsInDocumentTreeOrder(Element* lhsOwningElement, Element* rhsOwningElement)
+{
+    // With regard to pseudo-elements, the sort order is as follows:
+    //     - element
+    //     - ::before
+    //     - ::after
+    //     - element children
+    
+    // We could be comparing two pseudo-elements that are hosted on the same element.
+    if (is<PseudoElement>(lhsOwningElement) && is<PseudoElement>(rhsOwningElement)) {
+        auto* lhsPseudoElement = downcast<PseudoElement>(lhsOwningElement);
+        auto* rhsPseudoElement = downcast<PseudoElement>(rhsOwningElement);
+        if (lhsPseudoElement->hostElement() == rhsPseudoElement->hostElement())
+            return lhsPseudoElement->isBeforePseudoElement();
+    }
+
+    // Or comparing a pseudo-element that is compared to another non-pseudo element, in which case
+    // we want to see if it's hosted on that other element, and if not use its host element to compare.
+    if (is<PseudoElement>(lhsOwningElement)) {
+        auto* lhsHostElement = downcast<PseudoElement>(lhsOwningElement)->hostElement();
+        if (rhsOwningElement == lhsHostElement)
+            return false;
+        lhsOwningElement = lhsHostElement;
+    }
+
+    if (is<PseudoElement>(rhsOwningElement)) {
+        auto* rhsHostElement = downcast<PseudoElement>(rhsOwningElement)->hostElement();
+        if (lhsOwningElement == rhsHostElement)
+            return true;
+        rhsOwningElement = rhsHostElement;
+    }
+
+    return lhsOwningElement->compareDocumentPosition(*rhsOwningElement) & Node::DOCUMENT_POSITION_FOLLOWING;
+}
+
 Vector<RefPtr<WebAnimation>> DocumentTimeline::getAnimations() const
 {
     ASSERT(m_document);
 
-    // FIXME: Filter and order the list as specified (webkit.org/b/179535).
+    Vector<RefPtr<WebAnimation>> cssTransitions;
+    Vector<RefPtr<WebAnimation>> cssAnimations;
+    Vector<RefPtr<WebAnimation>> webAnimations;
+
+    // First, let's get all qualifying animations in their right group.
+    for (const auto& animation : m_allAnimations) {
+        if (!animation || !animation->isRelevant() || animation->timeline() != this || !is<KeyframeEffectReadOnly>(animation->effect()))
+            continue;
+
+        auto* target = downcast<KeyframeEffectReadOnly>(animation->effect())->target();
+        if (!target || !target->isDescendantOf(*m_document))
+            continue;
+
+        if (is<CSSTransition>(animation) && downcast<CSSTransition>(animation)->owningElement())
+            cssTransitions.append(animation);
+        else if (is<CSSAnimation>(animation) && downcast<CSSAnimation>(animation)->owningElement())
+            cssAnimations.append(animation);
+        else
+            webAnimations.append(animation);
+    }
+
+    // Now sort CSS Transitions by their composite order.
+    std::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());
+
+        auto* lhsOwningElement = lhsTransition->owningElement();
+        auto* rhsOwningElement = rhsTransition->owningElement();
+
+        // If the owning element of A and B differs, sort A and B by tree order of their corresponding owning elements.
+        if (lhsOwningElement != rhsOwningElement)
+            return compareDeclarativeAnimationOwningElementPositionsInDocumentTreeOrder(lhsOwningElement, rhsOwningElement);
+
+        // Otherwise, if A and B have different transition generation values, sort by their corresponding transition generation in ascending order.
+        if (lhsTransition->generationTime() != rhsTransition->generationTime())
+            return lhsTransition->generationTime() < rhsTransition->generationTime();
+
+        // Otherwise, sort A and B in ascending order by the Unicode codepoints that make up the expanded transition property name of each transition
+        // (i.e. without attempting case conversion and such that ‘-moz-column-width’ sorts before ‘column-width’).
+        return lhsTransition->transitionProperty().utf8() < rhsTransition->transitionProperty().utf8();
+    });
+
+    // Now sort CSS Animations by their composite order.
+    std::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();
+
+        // If the owning element of A and B differs, sort A and B by tree order of their corresponding owning elements.
+        if (lhsOwningElement != rhsOwningElement)
+            return compareDeclarativeAnimationOwningElementPositionsInDocumentTreeOrder(lhsOwningElement, rhsOwningElement);
+
+        // Otherwise, sort A and B based on their position in the computed value of the animation-name property of the (common) owning element.
+        // In our case, this matches the time at which the animations were created and thus their relative position in m_allAnimations.
+        return false;
+    });
+
+    // Finally, we can concatenate the sorted CSS Transitions, CSS Animations and Web Animations in their relative composite order.
     Vector<RefPtr<WebAnimation>> animations;
-    for (const auto& animation : m_animations) {
-        if (animation->isRelevant() && is<KeyframeEffectReadOnly>(animation->effect())) {
-            if (auto* target = downcast<KeyframeEffectReadOnly>(animation->effect())->target()) {
-                if (target->isDescendantOf(*m_document))
-                    animations.append(animation);
-            }
-        }
-    }
+    animations.appendRange(cssTransitions.begin(), cssTransitions.end());
+    animations.appendRange(cssAnimations.begin(), cssAnimations.end());
+    animations.appendRange(webAnimations.begin(), webAnimations.end());
     return animations;
 }
 
@@ -271,6 +363,7 @@
     // 1. Update the current time of all timelines associated with doc passing now as the timestamp.
 
     Vector<RefPtr<WebAnimation>> animationsToRemove;
+    Vector<RefPtr<CSSTransition>> completedTransitions;
 
     for (auto& animation : m_animations) {
         if (animation->timeline() != this) {
@@ -285,6 +378,12 @@
 
         if (!animation->isRelevant() && !animation->needsTick())
             animationsToRemove.append(animation);
+
+        if (!animation->needsTick() && is<CSSTransition>(animation) && animation->playState() == WebAnimation::PlayState::Finished) {
+            auto* transition = downcast<CSSTransition>(animation.get());
+            if (transition->owningElement())
+                completedTransitions.append(transition);
+        }
     }
 
     // 2. Perform a microtask checkpoint.
@@ -316,9 +415,28 @@
     for (auto& animation : animationsToRemove)
         removeAnimation(*animation);
 
+    // Now that animations that needed removal have been removed, let's update the list of completed transitions.
+    // This needs to happen after dealing with the list of animations to remove as the animation may have been
+    // removed from the list of completed transitions otherwise.
+    for (auto& completedTransition : completedTransitions)
+        transitionDidComplete(completedTransition);
+
     applyPendingAcceleratedAnimations();
 }
 
+void DocumentTimeline::transitionDidComplete(RefPtr<CSSTransition> transition)
+{
+    ASSERT(transition);
+    removeAnimation(*transition);
+    if (is<KeyframeEffectReadOnly>(transition->effect())) {
+        if (auto* target = downcast<KeyframeEffectReadOnly>(transition->effect())->target()) {
+            m_elementToCompletedCSSTransitionByCSSPropertyID.ensure(target, [] {
+                return HashMap<CSSPropertyID, RefPtr<CSSTransition>> { };
+            }).iterator->value.set(transition->property(), transition);
+        }
+    }
+}
+
 bool DocumentTimeline::computeExtentOfAnimation(RenderElement& renderer, LayoutRect& bounds) const
 {
     if (!renderer.element())

Modified: trunk/Source/WebCore/animation/DocumentTimeline.h (237725 => 237726)


--- trunk/Source/WebCore/animation/DocumentTimeline.h	2018-11-02 07:15:13 UTC (rev 237725)
+++ trunk/Source/WebCore/animation/DocumentTimeline.h	2018-11-02 07:54:19 UTC (rev 237726)
@@ -95,6 +95,7 @@
     void performEventDispatchTask();
     void maybeClearCachedCurrentTime();
     void updateListOfElementsWithRunningAcceleratedAnimationsForElement(Element&);
+    void transitionDidComplete(RefPtr<CSSTransition>);
 
     RefPtr<Document> m_document;
     Seconds m_originTime;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to