Bug 22052

Summary:

Assert and eventual crash when destroying element in an animation CB

Product:

WebKit

Reporter:

Chris Marrin <cmarrin>

Component:

CSS

Assignee:

Chris Marrin <cmarrin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dino, simon.fraser

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

OS X 10.5

Attachments:

Description	Flags
testcase that crashes sometimes	none
Testcase in loadable form	none
Patch, including LayoutTest file	none
Replacement patch with more descriptive changelog	hyatt: review+

Chris Marrin

Reported 2008-11-03 12:03:26 PST

Dean has an example which occasionally asserts and later crashes when destroying an element in an animation end callback. It looks like it is crashing when the element happens to get GC'ed during the callback. The assert is at AnimationBase.cpp:475. I've asked Dean to submit the preliminary test case. I will try to get a layout test which causes the crash when I do the patch.

Attachments
testcase that crashes sometimes (2.17 KB, patch) 2008-11-03 13:52 PST, Dean Jackson	no flags	Details Formatted Diff Diff
Testcase in loadable form (1.46 KB, text/html) 2008-11-03 15:03 PST, Simon Fraser (smfr)	no flags	Details
Patch, including LayoutTest file (8.99 KB, patch) 2008-11-03 18:07 PST, Chris Marrin	no flags	Details Formatted Diff Diff
Replacement patch with more descriptive changelog (9.56 KB, patch) 2008-11-13 11:31 PST, Chris Marrin	hyatt: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Dean Jackson

Comment 1 2008-11-03 13:52:06 PST

Created attachment 24864 [details] testcase that crashes sometimes

Simon Fraser (smfr)

Comment 2 2008-11-03 15:03:26 PST

Created attachment 24871 [details] Testcase in loadable form

Chris Marrin

Comment 3 2008-11-03 18:07:10 PST

Created attachment 24875 [details] Patch, including LayoutTest file

Simon Fraser (smfr)

Comment 4 2008-11-03 20:05:38 PST

Comment on attachment 24875 [details] Patch, including LayoutTest file >- // |this| may be deleted here when we've been called from timerFired() Isn't this comment still valid? >+ // Toss the ref to all animations ... the refs.. (plural)

Chris Marrin

Comment 5 2008-11-13 11:31:01 PST

Created attachment 25126 [details] Replacement patch with more descriptive changelog I fixed the comment made by Simon. The [this] pointer really is always valid at the point where I removed the comment. That is the point of the previous change I made to refcount AnimationBase objects. I also added details to the Changelog about the fix.

Simon Fraser (smfr)

Comment 6 2008-11-25 21:22:04 PST

<rdar://problem/6401110>

Dave Hyatt

Comment 7 2008-12-01 14:30:49 PST

Comment on attachment 25126 [details] Replacement patch with more descriptive changelog r=me

Chris Marrin

Comment 8 2008-12-01 15:06:20 PST

Committed r38768 M WebCore/ChangeLog M WebCore/page/animation/AnimationBase.h M WebCore/page/animation/CompositeAnimation.cpp M WebCore/page/animation/AnimationController.cpp M WebCore/page/animation/CompositeAnimation.h M WebCore/page/animation/AnimationBase.cpp M LayoutTests/ChangeLog A LayoutTests/animations/transform-animation-event-destroy-element.html A LayoutTests/animations/transform-animation-event-destroy-element-expected.txt A LayoutTests/transitions/transform-transition-event-destroy-element-expected.txt A LayoutTests/transitions/transform-transition-event-destroy-element.html r38768 = 7c14de362f15d6dc75bbd7914f4b8db76e2c1430 (trunk) I added a couple more tests.

Note You need to log in before you can comment on or make changes to this bug.