[Webkit-unassigned] [Bug 51218] Implement mozilla's requestAnimationFrame API

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jan 11 14:46:13 PST 2011


https://bugs.webkit.org/show_bug.cgi?id=51218





--- Comment #12 from James Robinson <jamesr at chromium.org>  2011-01-11 14:46:12 PST ---
(In reply to comment #11)
> (From update of attachment 78468 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=78468&action=review
> 
> would be good to add tests for changing the display property (hiding/showing an element) from the animation callback of another element.

done

> 
> > Source/WebCore/dom/Document.cpp:4959
> > +        m_requestAnimationFrameCallbacks = new Vector<RefPtr<RequestAnimationFrameCallback> >();
> 
> nit: a typedef might be nice?
> typedef Vector<RefPtr<RequestAnimationFrameCallback> > RequestAnimationFrameCallbackList;

done (and it does help a lot)

> 
> > Source/WebCore/dom/Document.cpp:4990
> > +    // visibility so this code has to iterate carefully.
> 
> maybe just add a FIXME or something about the fact that we don't do this visibility test yet.

FIXME added

> 
> > Source/WebCore/dom/Document.cpp:5008
> > +                callback->m_enabled = false;
> 
> m_enabled might be better if named m_firedOrCancelled (inverting the sense).

done

> 
> > Source/WebCore/dom/Document.cpp:5023
> > +            m_requestAnimationFrameCallbacks->insert(callbacksAdded++, callbacks[i]);
> 
> any callback that is no longer enabled needs to be removed from this list.
> 
> maybe you even want to just loop over m_requestAnimationFrameCallbacks??

right, done

> 
> > Tools/DumpRenderTree/chromium/WebViewHost.cpp:636
> > +    webkit_support::PostDelayedTask(invokeScheduleComposite, static_cast<void*>(this), 0);
> 
> isn't this cast unnecessary since 'this' is a pointer type?

gcc says the cast is unnecessary.  I just copy-pasted that from other code - looks like the original reference to webkit_support::PostDelayedTask() had the static_cast<> (http://trac.webkit.org/changeset/66701) and I'm not sure why.  Removed

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list