Bug 223368 – Release assert in compareAnimationsByCompositeOrder in KeyframeEffectStack::ensureEffectsAreSorted via Style::TreeResolver::createAnimatedElementUpdate

• Component: Animations
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Created attachment 423453 [details]
Test

e.g.

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x000000011ab8b91b WTFCrashWithInfo(int, char const*, char const*, int) + 27 (Assertions.h:671)
1   com.apple.WebCore             	0x000000011dbfa30d WebCore::compareCSSAnimations(WebCore::CSSAnimation const&, WebCore::CSSAnimation const&) + 717 (WebAnimationUtilities.cpp:144)
2   com.apple.WebCore             	0x000000011dbf9a59 WebCore::compareAnimationsByCompositeOrder(WebCore::WebAnimation const&, WebCore::WebAnimation const&) + 649 (WebAnimationUtilities.cpp:171)
3   com.apple.WebCore             	0x000000011dc27450 auto WebCore::KeyframeEffectStack::ensureEffectsAreSorted()::$_8::operator()<WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>, WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter> >(WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>&, WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>&) const + 96 (KeyframeEffectStack.cpp:110)
4   com.apple.WebCore             	0x000000011dc279ad void std::__1::__stable_sort_move<WebCore::KeyframeEffectStack::ensureEffectsAreSorted()::$_8&, WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>*>(WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>*, WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>*, WebCore::KeyframeEffectStack::ensureEffectsAreSorted()::$_8&, std::__1::iterator_traits<WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>*>::difference_type, std::__1::iterator_traits<WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>*>::value_type*) + 317 (algorithm:4685)
5   com.apple.WebCore             	0x000000011dc27063 void std::__1::__stable_sort<WebCore::KeyframeEffectStack::ensureEffectsAreSorted()::$_8&, WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>*>(WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>*, WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>*, WebCore::KeyframeEffectStack::ensureEffectsAreSorted()::$_8&, std::__1::iterator_traits<WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>*>::difference_type, std::__1::iterator_traits<WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>*>::value_type*, long) + 547 (algorithm:4751)
6   com.apple.WebCore             	0x000000011dbed402 void std::__1::stable_sort<WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>*, WebCore::KeyframeEffectStack::ensureEffectsAreSorted()::$_8>(WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>*, WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>*, WebCore::KeyframeEffectStack::ensureEffectsAreSorted()::$_8) + 578 (algorithm:4782)
7   com.apple.WebCore             	0x000000011dbed12e WebCore::KeyframeEffectStack::ensureEffectsAreSorted() + 94 (KeyframeEffectStack.cpp:100)
8   com.apple.WebCore             	0x000000011dbd3045 WebCore::KeyframeEffectStack::sortedEffects() + 21 (KeyframeEffectStack.cpp:91)
9   com.apple.WebCore             	0x000000011dbed858 WebCore::KeyframeEffectStack::applyKeyframeEffects(WebCore::RenderStyle&, WebCore::RenderStyle const&, WebCore::RenderStyle const*) + 344 (KeyframeEffectStack.cpp:134)
10  com.apple.WebCore             	0x000000012073063f WebCore::Styleable::applyKeyframeEffects(WebCore::RenderStyle&, WebCore::RenderStyle const&, WebCore::RenderStyle const*) const + 95 (Styleable.h:92)
11  com.apple.WebCore             	0x000000012072fc03 WebCore::Style::TreeResolver::createAnimatedElementUpdate(std::__1::unique_ptr<WebCore::RenderStyle, std::__1::default_delete<WebCore::RenderStyle> >, WebCore::Styleable const&, WebCore::Style::Change) + 1027 (StyleTreeResolver.cpp:348)
12  com.apple.WebCore             	0x000000012072e9ea WebCore::Style::TreeResolver::resolveElement(WebCore::Element&) + 874 (StyleTreeResolver.cpp:225)
13  com.apple.WebCore             	0x0000000120731201 WebCore::Style::TreeResolver::resolveComposedTree() + 1377 (StyleTreeResolver.cpp:534)
14  com.apple.WebCore             	0x0000000120732aac WebCore::Style::TreeResolver::resolve() + 1196 (StyleTreeResolver.cpp:593)
15  com.apple.WebCore             	0x000000011e4247a3 WebCore::Document::resolveStyle(WebCore::Document::ResolveStyleType) + 1523 (Document.cpp:2057)
16  com.apple.WebCore             	0x000000011e425a9c WebCore::Document::updateStyleIfNeeded() + 572 (Document.cpp:2162)
17  com.apple.WebCore             	0x000000011e448f0c WebCore::command(WebCore::Document*, WTF::String const&, bool) + 220 (Document.cpp:5673)
18  com.apple.WebCore             	0x000000011e448d96 WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) + 230 (Document.cpp:5687)
19  com.apple.WebCore             	0x000000011b62ee6a WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*) + 1130 (JSDocument.cpp:5890)
20  com.apple.WebCore             	0x000000011b62e95c long long WebCore::IDLOperation<WebCore::JSDocument>::call<&(WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*) + 252 (JSDOMOperation.h:53)
21  com.apple.WebCore             	0x000000011b619239 WebCore::jsDocumentPrototypeFunction_execCommand(JSC::JSGlobalObject*, JSC::CallFrame*) + 9 (JSDocument.cpp:5895)
22  ???                           	0x0000294afb8011d8 0 + 45402023793112
23  com.apple.JavaScriptCore      	0x0000000135286f5a llint_entry + 110482 (LowLevelInterpreter.asm:1093)
24  com.apple.JavaScriptCore      	0x0000000135286db1 llint_entry + 110057 (LowLevelInterpreter.asm:1093)
25  com.apple.JavaScriptCore      	0x000000013526bdc9 vmEntryToJavaScript + 216 (LowLevelInterpreter64.asm:316)
26  com.apple.JavaScriptCore      	0x0000000136ab47d2 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 258 (JITCodeInlines.h:42) [inlined]
27  com.apple.JavaScriptCore      	0x0000000136ab47d2 JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 1554 (Interpreter.cpp:907)
28  com.apple.JavaScriptCore      	0x000000013719dd15 JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 101 (CallData.cpp:57)
29  com.apple.JavaScriptCore      	0x000000013719de10 JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) + 224 (CallData.cpp:64)
30  com.apple.JavaScriptCore      	0x000000013719e1cc JSC::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) + 268 (CallData.cpp:85)
31  com.apple.WebCore             	0x000000011dc3c8a9 WebCore::JSExecState::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) + 233 (JSExecState.h:73)
32  com.apple.WebCore             	0x000000011dc68bab WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&) + 2731 (JSEventListener.cpp:186)
33  com.apple.WebCore             	0x000000011e576263 WebCore::EventTarget::innerInvokeEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener, WTF::RawPtrTraits<WebCore::RegisteredEventListener>, WTF::DefaultRefDerefTraits<WebCore::RegisteredEventListener> >, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>, WebCore::EventTarget::EventInvokePhase) + 1315 (EventTarget.cpp:344)
34  com.apple.WebCore             	0x000000011e575b03 WebCore::EventTarget::fireEventListeners(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) + 435 (EventTarget.cpp:276)
35  com.apple.WebCore             	0x000000011f550295 WebCore::DOMWindow::dispatchEvent(WebCore::Event&, WebCore::EventTarget*) + 773 (DOMWindow.cpp:2312)
36  com.apple.WebCore             	0x000000011f563a08 WebCore::DOMWindow::dispatchLoadEvent() + 552 (DOMWindow.cpp:2252)
37  com.apple.WebCore             	0x000000011e42e9d6 WebCore::Document::dispatchWindowLoadEvent() + 86 (Document.cpp:4943)
38  com.apple.WebCore             	0x000000011e42e3f4 WebCore::Document::implicitClose() + 756 (Document.cpp:3142)
39  com.apple.WebCore             	0x000000011f35ca09 WebCore::FrameLoader::checkCallImplicitClose() + 217 (FrameLoader.cpp:940)
40  com.apple.WebCore             	0x000000011f35be93 WebCore::FrameLoader::checkCompleted() + 691 (FrameLoader.cpp:881)
41  com.apple.WebCore             	0x000000011f35cbea WebCore::FrameLoader::completed() + 362 (FrameLoader.cpp:1181)
42  com.apple.WebCore             	0x000000011f35beb6 WebCore::FrameLoader::checkCompleted() + 726 (FrameLoader.cpp:885)
43  com.apple.WebCore             	0x000000011f357e95 WebCore::FrameLoader::finishedParsing() + 453 (FrameLoader.cpp:786)
44  com.apple.WebCore             	0x000000011e44ec84 WebCore::Document::finishedParsing() + 612 (Document.cpp:6001)
45  com.apple.WebCore             	0x000000011edb78c5 WebCore::HTMLConstructionSite::finishedParsing() + 37 (HTMLConstructionSite.cpp:419)
46  com.apple.WebCore             	0x000000011ee1bd3e WebCore::HTMLTreeBuilder::finished() + 30 (HTMLTreeBuilder.cpp:2843)

<rdar://74600723>

[Ryosuke Niwa]

I was able to reproduce this issue using the test case in <rdar://75152056> with ASAN release build of WebKitTestRunner.

[Ryosuke Niwa]

I was able to reproduce this issue (with the attached test case) with ASAN release build of WebKitTestRunner and DumpRenderTree.

[Ryosuke Niwa]

(both at r274459).

[Frédéric Wang (:fredw)]

Created attachment 423881 [details]
Reduced testcase

Will take a look.

[Frédéric Wang (:fredw)]

Inside compareCSSAnimations() we are hitting ASSERT(!cssAnimationList->isEmpty()); in debug mode and RELEASE_ASSERT_NOT_REACHED(); in release mode. This happens when resolving the style of the moved <animate> element, its associated KeyframeEffectStack contains 2 effects (with associated animations) but an empty animation list:

BODY	0x7fc879e30010 (renderer 0x7fc879e61800)  (child needs style recalc)
	#text	0x7fc879e300a0 "\n  "
	ANIMATE	0x7fc879e30110 (renderer 0x7fc879e30a30) 
	#text	0x7fc879e301a0 "\n  "
	DIV	0x7fc879e30210 (renderer 0x7fc879e30b90) 
		#text	0x7fc879e302a0 "\n    "
		#text	0x7fc879e303a0 "\n  "
	#text	0x7fc879e28068 "\n\n"
*	ANIMATE	0x7fc879e30310 (renderer 0x7fc879e30dd0)  (needs style recalc)

#0  0x00007fc886c48b9b in WebCore::compareCSSAnimations(WebCore::CSSAnimation const&, WebCore::CSSAnimation const&) (a=..., b=...)
    at ../../Source/WebCore/animation/WebAnimationUtilities.cpp:131
#1  0x00007fc886c48e8f in WebCore::compareAnimationsByCompositeOrder(WebCore::WebAnimation const&, WebCore::WebAnimation const&) (a=..., b=...)
    at ../../Source/WebCore/animation/WebAnimationUtilities.cpp:171
#2  0x00007fc886c4c595 in operator()<WTF::WeakPtr<WebCore::KeyframeEffect>, WTF::WeakPtr<WebCore::KeyframeEffect> >(WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>&, WTF::WeakPtr<WebCore::KeyframeEffect, WTF::EmptyCounter>&) const (__closure=0x7ffdfd0fc827, lhs=..., rhs=...)
    at ../../Source/WebCore/animation/KeyframeEffectStack.cpp:110
...
#9  0x00007fc886c4184d in WebCore::KeyframeEffectStack::ensureEffectsAreSorted() (this=0x7fc8302dafc0) at ../../Source/WebCore/animation/KeyframeEffectStack.cpp:100
#10 0x00007fc886c417d2 in WebCore::KeyframeEffectStack::sortedEffects() (this=0x7fc8302dafc0) at ../../Source/WebCore/animation/KeyframeEffectStack.cpp:91
#11 0x00007fc886c41a42 in WebCore::KeyframeEffectStack::applyKeyframeEffects(WebCore::RenderStyle&, WebCore::RenderStyle const&, WebCore::RenderStyle const*) (this=0x7fc8302dafc0, targetStyle=..., previousLastStyleChangeEventStyle=..., parentElementStyle=0x7fc879e61858) at ../../Source/WebCore/animation/KeyframeEffectStack.cpp:134
#12 0x00007fc888711504 in WebCore::Styleable::applyKeyframeEffects(WebCore::RenderStyle&, WebCore::RenderStyle const&, WebCore::RenderStyle const*) const (this=0x7ffdfd0fcbf0, targetStyle=..., previousLastStyleChangeEventStyle=..., parentElementStyle=0x7fc879e61858) at ../../Source/WebCore/style/Styleable.h:92
#13 0x00007fc88870ac9a in WebCore::Style::TreeResolver::createAnimatedElementUpdate(std::unique_ptr<WebCore::RenderStyle, std::default_delete<WebCore::RenderStyle> >, WebCore::Styleable const&, WebCore::Style::Change) (this=0x7ffdfd0ff660, newStyle=std::unique_ptr<WebCore::RenderStyle> = {...}, styleable=..., parentChange=WebCore::Style::Change::None) at ../../Source/WebCore/style/StyleTreeResolver.cpp:348
c#14 0x00007fc88870a0ca in WebCore::Style::TreeResolver::resolveElement(WebCore::Element&) (this=0x7ffdfd0ff660, element=...) at ../../Source/WebCore/style/StyleTreeResolver.cpp:225
...
...
#32 0x00007fc887a40de5 in WebCore::DocumentLoader::finishedLoading() (this=0x7fc8324f2400) at ../../Source/WebCore/loader/DocumentLoader.cpp:480

[Frédéric Wang (:fredw)]

Running back to when the animation list was emptied, we found it happenned in AnimationTimeline::removeCSSAnimationCreatedByMarkup, when the <animate> element is moved from the inner div to the body by the JS code. At that time, the KeyframeEffectStack only contains 1 effect, which is removed when the animation is later canceled in cancelDeclarativeAnimationsForStyleable.

#0  WebCore::AnimationTimeline::removeCSSAnimationCreatedByMarkup(WebCore::Styleable const&, WebCore::CSSAnimation&)
    (this=0x7fc8302b1000, styleable=..., cssAnimation=...) at ../../Source/WebCore/animation/AnimationTimeline.cpp:154
#1  0x00007fc88900dab2 in WebCore::AnimationTimeline::cancelDeclarativeAnimationsForStyleable(WebCore::Styleable const&, WebCore::WebAnimation::Silently) (this=0x7fc8302b1000, styleable=..., silently=WebCore::WebAnimation::Silently::Yes)
    at ../../Source/WebCore/animation/AnimationTimeline.cpp:179
#2  0x00007fc88900d8e6 in WebCore::AnimationTimeline::elementWasRemoved(WebCore::Styleable const&)
    (this=0x7fc8302b1000, styleable=...) at ../../Source/WebCore/animation/AnimationTimeline.cpp:162
#3  0x00007fc88720b08f in WebCore::Element::removedFromAncestor(WebCore::Node::RemovalType, WebCore::ContainerNode&)
    (this=0x7fc879e30310, removalType=..., oldParentOfRemovedTree=...) at ../../Source/WebCore/dom/Element.cpp:2321
#4  0x00007fc88712203c in WebCore::notifyNodeRemovedFromDocument(WebCore::ContainerNode&, WebCore::TreeScopeChange, WebCore::Node&) (oldParentOfRemovedTree=..., treeScopeChange=WebCore::TreeScopeChange::Changed, node=...)
    at ../../Source/WebCore/dom/ContainerNodeAlgorithms.cpp:126
#5  0x00007fc887122507 in WebCore::notifyChildNodeRemoved(WebCore::ContainerNode&, WebCore::Node&)
    (oldParentOfRemovedTree=..., child=...) at ../../Source/WebCore/dom/ContainerNodeAlgorithms.cpp:178
#6  0x00007fc887107828 in WebCore::ContainerNode::removeNodeWithScriptAssertion(WebCore::Node&, WebCore::ContainerNode::ChildChange::Source) (this=0x7fc879e30210, childToRemove=..., source=WebCore::ContainerNode::ChildChange::Source::API)
    at ../../Source/WebCore/dom/ContainerNode.cpp:182
#7  0x00007fc887100eaa in WebCore::ContainerNode::removeChild(WebCore::Node&) (this=0x7fc879e30210, oldChild=...)
    at ../../Source/WebCore/dom/ContainerNode.cpp:614
#8  0x00007fc8870feb9d in WebCore::ContainerNode::removeSelfOrChildNodesForInsertion(WebCore::Node&, WTF::Vector<WTF::Ref<WebCore::Node, WTF::RawPtrTraits<WebCore::Node> >, 11ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&)
    (this=0x7fc879e30010, child=..., nodesForInsertion=...) at ../../Source/WebCore/dom/ContainerNode.cpp:255
#9  0x00007fc8871018c6 in WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck(WebCore::Node&)
    (this=0x7fc879e30010, newChild=...) at ../../Source/WebCore/dom/ContainerNode.cpp:739
#10 0x00007fc8871017bb in WebCore::ContainerNode::appendChild(WebCore::Node&) (this=0x7fc879e30010, newChild=...)
    at ../../Source/WebCore/dom/ContainerNode.cpp:731
#11 0x00007fc88729cb76 in WebCore::Node::appendChild(WebCore::Node&) (this=0x7fc879e30010, newChild=...)
    at ../../Source/WebCore/dom/Node.cpp:511

However, one effect is added later when the JS code calls pause():

#3  0x00007fc886c41590 in WebCore::KeyframeEffectStack::addEffect(WebCore::KeyframeEffect&)
    (this=0x7fc8302dafc0, effect=...) at ../../Source/WebCore/animation/KeyframeEffectStack.cpp:52
#4  0x00007fc886c3c8ad in WebCore::KeyframeEffect::updateEffectStackMembership() (this=0x7fc8302b7000)
    at ../../Source/WebCore/animation/KeyframeEffect.cpp:1130
#5  0x00007fc886c3c7ee in WebCore::KeyframeEffect::animationTimingDidChange() (this=0x7fc8302b7000)
    at ../../Source/WebCore/animation/KeyframeEffect.cpp:1119
#6  0x00007fc886c45100 in WebCore::WebAnimation::timingDidChange(WebCore::WebAnimation::DidSeek, WebCore::WebAnimation::SynchronouslyNotify, WebCore::WebAnimation::Silently)
    (this=0x7fc879e307d0, didSeek=WebCore::WebAnimation::DidSeek::No, synchronouslyNotify=WebCore::WebAnimation::SynchronouslyNotify::No, silently=WebCore::WebAnimation::Silently::No) at ../../Source/WebCore/animation/WebAnimation.cpp:794
#7  0x00007fc886c46c7d in WebCore::WebAnimation::pause() (this=0x7fc879e307d0)

and a second effect is added when the startTime is changed by the JS code:

#3  0x00007fc886c41590 in WebCore::KeyframeEffectStack::addEffect(WebCore::KeyframeEffect&)
    (this=0x7fc8302dafc0, effect=...) at ../../Source/WebCore/animation/KeyframeEffectStack.cpp:52
#4  0x00007fc886c3c8ad in WebCore::KeyframeEffect::updateEffectStackMembership() (this=0x7fc8302b7100)
    at ../../Source/WebCore/animation/KeyframeEffect.cpp:1130
#5  0x00007fc886c3c7ee in WebCore::KeyframeEffect::animationTimingDidChange() (this=0x7fc8302b7100)
    at ../../Source/WebCore/animation/KeyframeEffect.cpp:1119
#6  0x00007fc886c45100 in WebCore::WebAnimation::timingDidChange(WebCore::WebAnimation::DidSeek, WebCore::WebAnimation::SynchronouslyNotify, WebCore::WebAnimation::Silently)
    (this=0x7fc879e30900, didSeek=WebCore::WebAnimation::DidSeek::Yes, synchronouslyNotify=WebCore::WebAnimation::SynchronouslyNotify::No, silently=WebCore::WebAnimation::Silently::No) at ../../Source/WebCore/animation/WebAnimation.cpp:794
#7  0x00007fc886c431c2 in WebCore::WebAnimation::setStartTime(WTF::Optional<WTF::Seconds>)
    (this=0x7fc879e30900, newStartTime=...) at ../../Source/WebCore/animation/WebAnimation.cpp:355

So we end up in the bad state reached in comment 5.

[Ryosuke Niwa]

Am I so confused here. Is WebAnimation object created from style sheet supposed to survive even when an element is removed? Like... is it supposed to continue to do things? That is very strange.

If not, don't we just need to make sure pause(), startTime, etc... are no-op when it's no longer associated with an element?

So maybe the issue here is that when an element is removed, we're not clearing the owning element. Namely, AnimationTimeline::elementWasRemoved calls cancelDeclarativeAnimationsForStyleable but that calls WebAnimation::cancel instead of DeclarativeAnimation::cancelFromStyle() which should be clearing the owning element.

[Frédéric Wang (:fredw)]

Created attachment 424107 [details]
Reduced testcase

This is exactly the same test case as attachment 423881 [details], but replacing the <animate> element with a <p>, so it's less confusing :-)

[Frédéric Wang (:fredw)]

Created attachment 424110 [details]
(proof-of-concept) Patch

(In reply to Ryosuke Niwa from comment #7)
> Am I so confused here. Is WebAnimation object created from style sheet
> supposed to survive even when an element is removed? Like... is it supposed
> to continue to do things? That is very strange.

I'm not really familiar with this WebAnimation thing... @Dean or @Antoine maybe you can provide some advice?

BTW in attachment 424107 [details], when we do the appendChild() (and so the removal), this changes the p:only-of-type for the two <p> elements and so the -webkit-animation properties no longer apply to them.

> If not, don't we just need to make sure pause(), startTime, etc... are no-op
> when it's no longer associated with an element?
> 
> So maybe the issue here is that when an element is removed, we're not
> clearing the owning element. Namely, AnimationTimeline::elementWasRemoved
> calls cancelDeclarativeAnimationsForStyleable but that calls
> WebAnimation::cancel instead of DeclarativeAnimation::cancelFromStyle()
> which should be clearing the owning element.

Here is a basic patch doing something like that. It indeed prevents the crash, but I'm not sure it is "correct" or has other consequences. Let's check EWS...

I'll try to investigate a bit more.

[Antoine Quint]

Comment on attachment 424110 [details]
(proof-of-concept) Patch

This looks good, thanks Fred! I'll do a little refactoring to remove the Silently parameter afterwards and move that whole method to Styleable since it has nothing to do on the timeline.

[EWS]

Unable to find any modified ChangeLog in Attachment 424110 [details]

[Frédéric Wang (:fredw)]

Created attachment 424674 [details]
Patch

[EWS]

Committed r275228: <https://commits.webkit.org/r275228>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424674 [details].

[Antoine Quint]

Filed https://bugs.webkit.org/show_bug.cgi?id=223992 to refactor the code now that Silently is no longer used by AnimationTimeline::cancelDeclarativeAnimationsForStyleable().