23088 – CSS Transition property hangs Webkit under certain conditions

Faruk Ates

Reported 2009-01-03 01:53:59 PST

This line of CSS for a Transition appears to crash the latest Webkit nightly: -webkit-transition: opacity, width .45s, .15s ease-out; AFAICT that is totally acceptable syntax. It works in Safari 3.1.2 and it used to work in all the Nightlies I've tried it in over the past 8-9 months or so. Removing the distinct property definitions and replacing it with "all" (and only one duration) doesn't crash webkit. Testcase attached.

Faruk Ates

Comment 1 2009-01-03 01:54:53 PST

Created attachment 26391 [details] TESTCASE: CSS Transition that crashes latest webkit nightly

Simon Fraser (smfr)

Comment 2 2009-01-03 10:36:48 PST

Faruk: can you attach a crash log?

Simon Fraser (smfr)

Comment 3 2009-01-03 10:37:43 PST

I don't get a crash, but I do see a redraw issue.

Simon Fraser (smfr)

Comment 4 2009-01-03 10:44:29 PST

BTW the correct syntax is: -webkit-transition: opacity .45s ease-out, width .15s ease-out;

Simon Fraser (smfr)

Comment 5 2009-01-03 10:52:57 PST

I filed bug 23090 on the redraw issue. I see no crash, so closing.

Faruk Ates

Comment 6 2009-01-03 19:04:56 PST

Comment on attachment 26391 [details] TESTCASE: CSS Transition that crashes latest webkit nightly ><!DOCTYPE HTML PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> ><HTML><HEAD> > > > <META content="text/html; charset=utf-8" http-equiv="Content-type"/> > <TITLE>CSS Transition test case</TITLE> > <STYLE media="screen" type="text/css"> > ul { > border: 1px solid #ccc; > width: 600px; > } > ul li { > margin: 9px 0; > } > ul li a { > background: blue; > display: block; > color: white; > height: 63px; > opacity: .666; > width: 232px; > > -webkit-transition: opacity, width .45s, .15s ease-out; > } > ul li a:focus, > ul li a:hover { > opacity: 1; > width: 500px; > } > </STYLE> ></HEAD><BODY> > <UL> > <LI><A rel="me" href="">foo</A></LI> > <LI><A rel="me" href="">bar</A></LI> > <LI><A rel="me" href="">blaat</A></LI> > <LI><A rel="me" href=""><STRONG>facebook.com</STRONG></A></LI> > <LI><A rel="me" href=""><STRONG>faruk.newsvine.com</STRONG></A></LI> > </UL> ></BODY></HTML>

Faruk Ates

Comment 7 2009-01-03 19:05:19 PST

Comment on attachment 26391 [details] TESTCASE: CSS Transition that crashes latest webkit nightly ><!DOCTYPE HTML PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> ><HTML><HEAD> > > > <META content="text/html; charset=utf-8" http-equiv="Content-type"/> > <TITLE>CSS Transition test case</TITLE> > <STYLE media="screen" type="text/css"> > ul { > border: 1px solid #ccc; > width: 600px; > } > ul li { > margin: 9px 0; > } > ul li a { > background: blue; > display: block; > color: white; > height: 63px; > opacity: .666; > width: 232px; > > -webkit-transition: opacity, width .45s, .15s ease-out; > } > ul li a:focus, > ul li a:hover { > opacity: 1; > width: 500px; > } > </STYLE> ></HEAD><BODY> > <UL> > <LI><A rel="me" href="">foo</A></LI> > <LI><A rel="me" href="">bar</A></LI> > <LI><A rel="me" href="">blaat</A></LI> > <LI><A rel="me" href=""><STRONG>facebook.com</STRONG></A></LI> > <LI><A rel="me" href=""><STRONG>faruk.newsvine.com</STRONG></A></LI> > </UL> ></BODY></HTML>

Faruk Ates

Comment 8 2009-01-03 19:15:32 PST

Ack, the upload ended up using the wrong version of the test case (the version where it _doesn't_ crash)! I'm adding the proper test case which does make it crash. Also, I don't seem to be getting crash reports other than notifications in console.app

Faruk Ates

Comment 9 2009-01-03 19:16:20 PST

Created attachment 26401 [details] The proper test case. Test case wherein it does crash (sigh).

Faruk Ates

Comment 10 2009-01-03 19:20:31 PST

I see that the syntax has changed since I last checked the spec proposal for that (this could've been a looooong time ago mind you), so here's what I'm thinking: even though I'm evidently using the wrong syntax, it a) obviously shouldn't crash the browser and b) it should _probably_ not function as a transition at all, even though it does in Safari 3.1

Simon Fraser (smfr)

Comment 11 2009-01-03 19:29:28 PST

Thanks, I can reproduce a hang now.

Simon Fraser (smfr)

Comment 12 2009-01-03 19:37:27 PST

The problem is that Document::updateDocumentsRendering() gets stuck in an infinite loop, because the call to doc->updateRendering() puts the document back into the changedDocuments hash via this stack: #0 WebCore::Document::setDocumentChanged (this=0x6918200, b=true) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/Document.cpp:1078 #1 0x03940c2c in WebCore::Node::setChanged (this=0x1bc2cdb0, changeType=WebCore::AnimationStyleChange) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/Node.cpp:600 #2 0x03380366 in WebCore::AnimationBase::setChanged (node=0x1bc2cdb0) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/page/animation/AnimationBase.cpp:474 #3 0x03380786 in WebCore::AnimationBase::updateStateMachine (this=0x1bc99260, input=WebCore::AnimationBase::AnimationStateInputStartTimerFired, param=0) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/page/animation/AnimationBase.cpp:569 #4 0x03381116 in WebCore::AnimationBase::fireAnimationEventsIfNeeded (this=0x1bc99260) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/page/animation/AnimationBase.cpp:724 #5 0x03730b4e in WebCore::ImplicitAnimation::animate (this=0x1bc99260, animation=0x1bc2d680, renderer=0x1bc2d23c, currentStyle=0x1bc6f390, targetStyle=0x1bc69080, animatedStyle=@0xbfffe2b4) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/page/animation/ImplicitAnimation.cpp:84 #6 0x034a6bbb in WebCore::CompositeAnimationPrivate::animate (this=0x1bc2d2d0, renderer=0x1bc2d23c, currentStyle=0x1bc6f390, targetStyle=0x1bc69080) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/page/animation/CompositeAnimation.cpp:270 #7 0x034a6db5 in WebCore::CompositeAnimation::animate (this=0x1bc2d680, renderer=0x1bc2d23c, currentStyle=0x1bc6f390, targetStyle=0x1bc69080) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/page/animation/CompositeAnimation.cpp:596 #8 0x0338c7f9 in WebCore::AnimationController::updateAnimations (this=0x6862a08, renderer=0x1bc2d23c, newStyle=0x1bc69080) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/page/animation/AnimationController.cpp:336 #9 0x03a0437d in WebCore::RenderObject::setAnimatableStyle (this=0x1bc2d23c, style=@0xbfffe3ec) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/rendering/RenderObject.cpp:2206 #10 0x0394178b in WebCore::Node::setRenderStyle (this=0x1bc2cdb0, s=@0xbfffe43c) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/Node.cpp:1248 #11 0x035f7941 in WebCore::Element::recalcStyle (this=0x1bc2cdb0, change=WebCore::Node::NoChange) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/Element.cpp:717 #12 0x035f7bfe in WebCore::Element::recalcStyle (this=0x1bc2cb90, change=WebCore::Node::NoChange) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/Element.cpp:747 #13 0x035f7bfe in WebCore::Element::recalcStyle (this=0x1bc29a90, change=WebCore::Node::NoChange) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/Element.cpp:747 #14 0x035f7bfe in WebCore::Element::recalcStyle (this=0x1ab4d800, change=WebCore::Node::NoChange) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/Element.cpp:747 #15 0x035f7bfe in WebCore::Element::recalcStyle (this=0xaa9610, change=WebCore::Node::NoChange) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/Element.cpp:747 #16 0x035b0965 in WebCore::Document::recalcStyle (this=0x6918200, change=WebCore::Node::NoChange) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/Document.cpp:1152 #17 0x035a416d in WebCore::Document::updateRendering (this=0x6918200) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/Document.cpp:1175 #18 0x035a1828 in WebCore::Document::updateDocumentsRendering () at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/Document.cpp:1193 #19 0x0360cd08 in WebCore::EventTargetNode::dispatchGenericEvent (this=0xaa9610, prpEvent=@0xbfffe7fc, ec=@0xbfffe8bc) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/EventTargetNode.cpp:395 #20 0x0360cea0 in WebCore::EventTargetNode::dispatchEvent (this=0xaa9610, e=@0xbfffe8d8, ec=@0xbfffe8bc) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/EventTargetNode.cpp:273 #21 0x0360f7a3 in WebCore::EventTargetNode::dispatchMouseEvent (this=0xaa9610, eventType=@0xaac26c, button=-1, detail=0, pageX=223, pageY=371, screenX=263, screenY=504, ctrlKey=false, altKey=false, shiftKey=false, metaKey=false, isSimulated=false, relatedTargetArg=0x1bc2cdb0, underlyingEvent=@0xbfffe994) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/EventTargetNode.cpp:580 #22 0x0360ffdb in WebCore::EventTargetNode::dispatchMouseEvent (this=0xaa9610, event=@0xbfffecd4, eventType=@0xaac26c, detail=0, relatedTarget=0x1bc2cdb0) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/dom/EventTargetNode.cpp:489 #23 0x035fe3cc in WebCore::EventHandler::updateMouseEventTargetNode (this=0x68628f8, targetNode=0x1bc2cdb0, mouseEvent=@0xbfffecd4, fireMouseOverOut=true) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/page/EventHandler.cpp:1510 #24 0x0360166a in WebCore::EventHandler::dispatchMouseEvent (this=0x68628f8, eventType=@0xaac268, targetNode=0x1bc2cdb0, cancelable=false, clickCount=0, mouseEvent=@0xbfffecd4, setUnder=true) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/page/EventHandler.cpp:1524 #25 0x03604544 in WebCore::EventHandler::handleMouseMoveEvent (this=0x68628f8, mouseEvent=@0xbfffecd4, hoveredNode=0xbfffec6c) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/page/EventHandler.cpp:1246 #26 0x036045e5 in WebCore::EventHandler::mouseMoved (this=0x68628f8, event=@0xbfffecd4) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/page/EventHandler.cpp:1149 #27 0x036086fb in WebCore::EventHandler::mouseMoved (this=0x68628f8, event=0x1ab28db0) at /Volumes/Cassoulet/WebKit/WebKit.git/WebCore/page/mac/EventHandlerMac.mm:623 #28 0x0027957c in -[WebHTMLView(WebPrivate) _updateMouseoverWithEvent:] (self=0xaa67f0, _cmd=0x30cad1, event=0x1ab28db0) at /Volumes/Cassoulet/WebKit/WebKit.git/WebKit/mac/WebView/WebHTMLView.mm:1469 #29 0x00269828 in -[WebHTMLView mouseMovedNotification:] (self=0xaa67f0, _cmd=0x30ba30, notification=0xa1ec00) at /Volumes/Cassoulet/WebKit/WebKit.git/WebKit/mac/WebView/WebHTMLView.mm:3304 #30 0x91965e1a in _nsnote_callback ()

Oliver Hunt

Comment 13 2009-01-03 22:20:13 PST

Created attachment 26407 [details] Reduced the testcase to an automatic hang

Chris Marrin

Comment 14 2009-01-12 13:43:23 PST

Created attachment 26648 [details] Patch, including LayoutTest file Fixed https://bugs.webkit.org/show_bug.cgi?id=23088. This was happening because I was calling setChanged() from inside updateRendering() which causes an infinite loop. I fixed this by deferring the setChanged to the next run loop iteration. That made it not infinite loop, but it still retriggers the transition forever. The problem is that there is both an 'all' and specific transition on 'opacity'. This tickled a bug in AnimationController which causes the opacity transition to get constantly cancelled and then retriggered. The problem is that the specific opacity transition has a duration of 0. I got rid of the logic to flush out 0 duration transitions and it is no longer constantly triggered. The logic to flush them was just an optimization, and you really need to keep them around to make the logic to override earlier animations by later ones work. And there is very little overhead in this case anyway, so the optimization was not that useful. I made a LayoutTest from the original testcase which tests both the infinite loop and constantly triggering animation cases.

Chris Marrin

Comment 15 2009-01-15 11:16:07 PST

Created attachment 26762 [details] Patch to update Windows project file

Chris Marrin

Comment 16 2009-01-15 11:17:07 PST

Comment on attachment 26762 [details] Patch to update Windows project file Wrong bug

Dave Hyatt

Comment 17 2009-01-15 11:21:09 PST

Comment on attachment 26648 [details] Patch, including LayoutTest file r=me

Chris Marrin

Comment 18 2009-01-16 10:27:42 PST

Sending LayoutTests/ChangeLog Sending LayoutTests/transforms/2d/cssmatrix-interface-expected.txt Adding LayoutTests/transitions/hang-with-bad-transition-list-expected.txt Adding LayoutTests/transitions/hang-with-bad-transition-list.html Sending WebCore/ChangeLog Sending WebCore/page/animation/AnimationBase.cpp Sending WebCore/page/animation/AnimationController.cpp Sending WebCore/page/animation/AnimationController.h Sending WebCore/page/animation/CompositeAnimation.cpp Sending WebCore/rendering/style/RenderStyle.cpp Transmitting file data .......... Committed revision 39974.

Attachments
TESTCASE: CSS Transition that crashes latest webkit nightly (938 bytes, text/html) 2009-01-03 01:54 PST, Faruk Ates	no flags	Details
The proper test case. (928 bytes, text/html) 2009-01-03 19:16 PST, Faruk Ates	no flags	Details
Reduced the testcase to an automatic hang (245 bytes, text/html) 2009-01-03 22:20 PST, Oliver Hunt	no flags	Details
Patch, including LayoutTest file (13.01 KB, patch) 2009-01-12 13:43 PST, Chris Marrin	hyatt: review+	Details Formatted Diff Diff
Patch to update Windows project file (1.95 KB, patch) 2009-01-15 11:16 PST, Chris Marrin	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.