Bug 22052 – Assert and eventual crash when destroying element in an animation CB

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Chris Marrin]

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.

[Dean Jackson]

Created attachment 24864 [details]
testcase that crashes sometimes

[Simon Fraser (smfr)]

Created attachment 24871 [details]
Testcase in loadable form

[Chris Marrin]

Created attachment 24875 [details]
Patch, including LayoutTest file

[Simon Fraser (smfr)]

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]

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)]

<rdar://problem/6401110>

[Dave Hyatt]

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

r=me

[Chris Marrin]

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.