tree 1b5b86ceac3b780d9f55abd961a762715ea4be99
parent a7cf0e702a30c1ac9edece163850bf91386874de
author graouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc> 1634379392 +0000
committer graouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc> 1634379392 +0000

CSS Animations creation and sorting is incorrect and may lead to crash
https://bugs.webkit.org/show_bug.cgi?id=231812
<rdar://problem/82980774>

Reviewed by Dean Jackson.

LayoutTests/imported/w3c:

Mark two progressions in a test that now passes entirely.

* web-platform-tests/css/css-animations/Element-getAnimations-dynamic-changes.tentative-expected.txt:

Source/WebCore:

When we parse CSS properties related to CSS Animations, we create as many Animation objects as the maximum number
of values in one of the CSS Animations properties. These Animation objects are stored in an AnimationList which
is owned by RenderStyle and that we use to store all provided values such that they can be read back through the
computed style.

Upon finishing parsing those CSS properties, RenderStyle::adjustAnimations() is called and does two things that
were not quite correct.

First, it would attempt to remove animations that were in fact created for no good reason using the
Animation::isEmpty() method. That method was not quite right as it was only checking if the animation
name wasn't set but not whether the animation was explicitly marked as a "none" animation. This meant
that "none" animations could be considered as bogus animations and any animations after that one would
be mistakenly cleared.

Second, it would finish process the list of remaning animations by making sure that mis-matched CSS properties,
for instance three values for animation-duration and four values for animation-name, would be correctly represented
throughout the Animation objects. However, the function performing this work, AnimationList::fillUnsetProperties(),
used to reset the animation's name in case it wasn't explicitly set, which meant that Animation objects that did not
have a matching animation-name would end up with a valid one. This meant that further down the line, in
Styleable::updateCSSAnimations(), we couldn't distinguish Animation objects that were created without a
matching animation-name and discard those.

We've fixed those incorrect behaviors and we now have the expected number of animations created resulting in an additional
WPT PASS result.

Additionally, this also showed some issues around sorting CSS Animations in compareCSSAnimations() where we weren't
making a pointer comparison. This is now fixed and we now pass another WPT PASS result.

Test: webanimations/css-animation-sorting-crash.html

* animation/WebAnimationUtilities.cpp:
(WebCore::compareCSSAnimations):
* platform/animation/Animation.h:
(WebCore::Animation::isEmpty const):
* platform/animation/AnimationList.cpp:
(WebCore::AnimationList::fillUnsetProperties):

LayoutTests:

Add a new test that used to crash before the source change.

* animations/animation-remove-element-crash.html: Remove a line that was now a failure since only a
single animation should be created.
* webanimations/css-animation-sorting-crash-expected.txt: Added.
* webanimations/css-animation-sorting-crash.html: Added.

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