After some discussion, the sort order for animations has changed as described here: https://github.com/w3c/web-animations/issues/99 In summary: * Animations should be ordered based on creation order. * Instead of referring to sequence numbers, we now refer to a global animation list.[1] * CSS animations / transitions have their own ordering. Once they are disassociated from markup their sort order is undefined until they next transition out of the idle state (which effectively acts as their creation time).[2] I'd like to fix this at the same time as updating our sorting so that it is always defined. In bug 1183461 I'm occasionally running into trouble because we use the animation sort order as part of sorting events but by the point we sort events an animation might have become idle which currently triggers an assertion. We could probably find a way to avoid hitting this condition for now but when we go to implement animationcancel events we will need to know how to sort events from idle animations so we should just make this sorting always defined. [1] http://w3c.github.io/web-animations/#global-animation-list [2] https://rawgit.com/shans/csswg-drafts/animations-2/css-animations-2/Overview.html#animation-composite-order. Equivalent spec text for transitions has yet to be written.

The Web Animations specification has replaced the term "sequence number" with references to a global animation list. This patch applies similar naming to our animation structures.

This doesn't really save us much, but we don't need this method so we may as well drop it.

In the new composite order arrangement CSSAnimations and CSSTransition have the following life-cycle: 1. Animation created by markup => composite order determined by markup (e.g. CSS animations use tree order and animation-name order; CSS transitions use transition trigger order) 2. Animation cancelled by changing markup => composite order is undefined 3. Animation is played again using the API => composite order is defined by when the animation is first played. Another way of saying this is that, at the point when the animation is played, it is appended to the "global animation list". 4. Animation is subsequently cancelled / played => no change We need a way to know when we are going from 2 to 3. It would seem like we could do that by setting mAnimationIndex to some sentinel value while it is in 2. However, even when in 2, although the spec doesn't define the composite order animations at this point (from an API point of view you can't access these objects and they don't contribute to style so it doesn't need to be defined), we sometimes will need to establish an order. This can happen, for example, when an animation queues events and then is later cancelled before the events are dispatched. Because we sort events based on their associated animation at the time of dispatch (for performance reasons) we need a deterministic order for these idle animations. We do that (in a subsequent patch in this series) by setting mAnimationIndex when we transition from 1 to 2. That is, these idle animations are effectively ordered by when they became idle (which always happens in a deterministic fashion).

Hi David, I thought you might want to check these patches to see if you agree with this behaviour. I'm sure heycam or dholbert would be suitable reviewers, however, if you're busy.

I'm ok with the behavior, and you should switch reviewers to one of heycam or dholbert.

Comment on attachment 8659745 [details] [diff] [review] part 1 - Rename sequence number to animation index Review of attachment 8659745 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/animation/Animation.h @@ +411,4 @@ > > + // The position of this animation within the global animation list. > + // This is kNoIndex while the animation is in the idle state and is updated > + // each time the animation transitions out of the idle state. The "animation index" isn't an index into a hypothetical array of animations, unless you're allowed to have holes in the array, since new indexes are always greater than previous ones. Would it make sense to call it the "relative position of this animation"? Should you mention something here about how transitions can have the same animation index? (That wasn't something I realised before I saw the code in CSSTransition::HasLowerCompositeOrderThan.)

Comment on attachment 8659749 [details] [diff] [review] part 5 - Remove IsUsingCustomCompositeOrder Review of attachment 8659749 [details] [diff] [review]: ----------------------------------------------------------------- That's fine, though do we have a term for Animation objects for CSS Transitions/Animations that haven't been disconnected from them? It might make sense to still have a method named after such a term to make the code clearer in HasLowerCompositeOrderThan. Either way.

Can we test these changes?

(In reply to Cameron McCormack (:heycam) from comment #11) > Can we test these changes? Yes!

https://hg.mozilla.org/integration/mozilla-inbound/rev/f9b2ef0b4946 https://hg.mozilla.org/integration/mozilla-inbound/rev/5196bf0855ad https://hg.mozilla.org/integration/mozilla-inbound/rev/b7af8129961f https://hg.mozilla.org/integration/mozilla-inbound/rev/37b6deedaab6 https://hg.mozilla.org/integration/mozilla-inbound/rev/44462cfe3134 https://hg.mozilla.org/integration/mozilla-inbound/rev/1444325d4840

https://hg.mozilla.org/mozilla-central/rev/f9b2ef0b4946 https://hg.mozilla.org/mozilla-central/rev/5196bf0855ad https://hg.mozilla.org/mozilla-central/rev/b7af8129961f https://hg.mozilla.org/mozilla-central/rev/37b6deedaab6 https://hg.mozilla.org/mozilla-central/rev/44462cfe3134 https://hg.mozilla.org/mozilla-central/rev/1444325d4840