Bug 21310

Summary: Get rid of AnimationEventDispatcher
Product: WebKit Reporter: Chris Marrin <cmarrin>
Component: CSSAssignee: Chris Marrin <cmarrin>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Patch to fix bug mitz: review+

Description Chris Marrin 2008-10-02 09:55:35 PDT
Currently, when we want to send and animation event, we set a 0 duration timer and when it fires, we send the event. This is to avoid calling the event handler directly from the AnimationBase subclass (KeyframeAnimation or ImplicitAnimation), in case these objects are destroyed by the event handler. But this adds a lot of overhead, so it should be changed so these classes are ref counted. Then we can keep a ref to them across the call to the event handler and avoid them getting destroyed too soon.
Comment 1 Chris Marrin 2008-10-02 10:10:52 PDT
Created attachment 24022 [details]
Patch to fix bug
Comment 2 mitz 2008-10-02 10:37:25 PDT
Comment on attachment 24022 [details]
Patch to fix bug

Please remove this:
+        WARNING: NO TEST CASES ADDED OR CHANGED

Unnecessary newline added here:
     virtual ~AnimationBase();
 
+
     RenderObject* renderer() const { return m_object; }

isSuspended() would be a better name than suspended() and more in line with e.g. isAnimatingProperty():
+    bool suspended() const { return m_suspended; }

You can just write "if (keyframeAnim)" here:
+                if (keyframeAnim.get())

These are now out of order:
-class RenderObject;
 class RenderStyle;
+class RenderObject;

No need to initialize a RefPtr to 0:
+            RefPtr<Element> element = 0;

Again, no need to get(). I also think the parentheses are redundant.
+            ASSERT(!element.get() || (element->document() && !element->document()->inPageCache()));
+            if (!element.get())

The same is repeated in KeyframeAnimation.cpp

This looks odd. Why are you doing this?
+                setChanged(element->renderer()->element());

r=me
Comment 3 Chris Marrin 2008-10-10 11:05:11 PDT
Sending        WebCore/ChangeLog
Sending        WebCore/page/animation/AnimationBase.cpp
Sending        WebCore/page/animation/AnimationBase.h
Sending        WebCore/page/animation/AnimationController.cpp
Sending        WebCore/page/animation/CompositeAnimation.cpp
Sending        WebCore/page/animation/CompositeAnimation.h
Sending        WebCore/page/animation/ImplicitAnimation.cpp
Sending        WebCore/page/animation/ImplicitAnimation.h
Sending        WebCore/page/animation/KeyframeAnimation.cpp
Sending        WebCore/page/animation/KeyframeAnimation.h
Transmitting file data ..........
Committed revision 37484.