[webkit-reviews] review granted: [Bug 21310] Get rid of AnimationEventDispatcher : [Attachment 24022] Patch to fix bug

[bugzilla-daemon at webkit.org]

mitz at webkit.org has granted Chris Marrin <cmarrin at apple.com>'s request for
review:
Bug 21310: Get rid of AnimationEventDispatcher
https://bugs.webkit.org/show_bug.cgi?id=21310

Attachment 24022: Patch to fix bug
https://bugs.webkit.org/attachment.cgi?id=24022&action=edit

------- Additional Comments from mitz at webkit.org
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