Fix animation processing ordering and reenable animations/basic-transition.html test

[bholley]

In #12623, we added some hacky machinery that causes us to mutate style on non-dirty nodes during restyle to tick animations. This causes all kinds of trouble for the new incremental restyle architecture, since we're not guaranteed to have mutable style data for non-dirty nodes (the style might be owned by the layout frame).

Emilio thinks the right approach is to mark the node as dirty when processing transitionend. However, that still doesn't cause us to pass the test, because Servo doesn't properly implement the processing model in the spec [1], and fires requestAnimationFrame callbacks before animation events, rather than after.

Emilio is trying to fix the processing model a bit in #13864, but says that more refactoring will need to be done to pass the test. So we decided to disable the test for now to unblock things.

CC @emilio @notriddle

[1] https://html.spec.whatwg.org/#event-loop-processing-model:run-the-animation-frame-callbacks

[emilio]

Yeah, to clarify, the problem (apart from the processing order) is the following.

With that fixed, marking nodes as dirty after transitionend works for most cases, except when the node's style is accessed again before the transitionend event is processed, in which case it fails. This is that last kind of failure.

Multiple things can be done here. First one is making animations synchronized with script and properly marking the node as dirty in update_animation_state. This would be the preferable option IMO.

The other thing we might do as a workaround is keeping a list of UnsafeNodes to dirty the next reflow, and filling it with the expired nodes in update_animation_state, but that feels like a bit of a hack.

[notriddle]

Multiple things can be done here. First one is making animations synchronized with script and properly marking the node as dirty in update_animation_state. This would be the preferable option IMO.

If we have to go with this solution, could we at least have this predicated on a Big Global Switch that turns off the synchronization if there aren't any outstanding requestAnimationFrame tasks? Otherwise, we end up blocking animations on GC and our smoothness tanks.

[pcwalton]

Please don't disable OMT animations because of some spec language. The spec shouldn't be forbidding them.

[emilio]

This is not about completely disable OMT animations, but about properly synchronize the computed values back to the element.

Mainly, the current animations code relied in the DOM being fully restyled (which was no longer the case as restyling optimizations started kicking in), since it is in the restyling code where we checked for expired animations.

#12623 worked around that fixing some of the most common but not all the cases.

With "fixing animation processing ordering" I explicitly intend to properly trigger transition events, not disabling OMT transitions completely.

[nox]

Did this change with the Stylo effort?