588174 - Add optional callback parameter to mozRequestAnimationFrame

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

A commenter on hacks.mozilla.org suggested that instead of the pattern of adding a MozBeforePaint Listener and then calling mozRequestAnimationFrame, it would be slightly simpler if mozRequestAnimationFrame took a callback parameter that would be run at the next frame and then discarded. So the example here:
http://hacks.mozilla.org/2010/08/more-efficient-javascript-animations-with-mozrequestanimationframe/
would become

var start = window.mozAnimationStartTime;
function step(event) {
  var progress = event.timeStamp - start;
  d.style.left = Math.min(progress/10, 200) + "px";
  if (progress < 2000) {
    window.mozRequestAnimationFrame(step);
  }
}
window.mozRequestAnimationFrame(step);

which is a bit simpler and avoids the possibility of the author forgetting to unregister the event listener.

We could leave the MozBeforePaint event support in --- it's likely to be useful for other things --- and make the callback parameter optional, so authors could use either pattern.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I suppose the callback should probably just take the timestamp as a parameter, so the example would be

var start = window.mozAnimationStartTime;
function step(timeStamp) {
  var progress = timeStamp - start;
  d.style.left = Math.min(progress/10, 200) + "px";
  if (progress < 2000) {
    window.mozRequestAnimationFrame(step);
  }
}
window.mozRequestAnimationFrame(step);

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I guess we need to get this in for beta6 if we get it in at all. If it slips, we can do it post-FF4.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Make it possible to pass an explicit callback function to mozRequestAnimationFrame.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 473384 [details] [diff] [review]
Make it possible to pass an explicit callback function to mozRequestAnimationFrame.

I made sure that if you only use the callback version we don't also fire the event.  If it's ok to fire the event in addition to calling the callbacks, then I can probably simplify the code a little bit... but this seemed like cleaner api for web pages.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #4)
> I made sure that if you only use the callback version we don't also fire the
> event.  If it's ok to fire the event in addition to calling the callbacks, then
> I can probably simplify the code a little bit... but this seemed like cleaner
> api for web pages.

Actually I think we should fire the event for every paint.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #5)
> Actually I think we should fire the event for every paint.

I mean, not now, but eventually.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 473384 [details] [diff] [review]
Make it possible to pass an explicit callback function to mozRequestAnimationFrame.

Is that the right place for nsIAnimationFrameListener? I expected dom/interfaces somewhere.

ForgetAnimationFrameListeners should be called TakeAnimationFrameListeners, maybe?

Why does ScheduleAnimationFrameListeners return a value? I suggest it can just return void.

This should probably get super-review. Hopefully dbaron's around...

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 473384 [details] [diff] [review]
Make it possible to pass an explicit callback function to mozRequestAnimationFrame.

Do the other [function] interfaces exposed to the Web not have classinfo?  It seems a little odd, but I guess I can't see how it's harmful.

Put a newline at the end of the file, though.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> Do the other [function] interfaces exposed to the Web not have classinfo?

Yes.  Classinfo is per-implementation, and the only expected implementation of this interface is a JS Function object.  Classinfo doesn't even make sense there.

> Put a newline at the end of the file, though.

Done.

Comment 10

[Boris Zbarsky [:bzbarsky]]

> Actually I think we should fire the event for every paint.

Per irc discussion, leaving this part as is for now.

> Is that the right place for nsIAnimationFrameListener? 

Excellent question.  I could put it in dom/interface/base, I suppose.  We don't have a good dumping ground for that sort of thing.

> ForgetAnimationFrameListeners should be called TakeAnimationFrameListeners,

Sold.

> Why does ScheduleAnimationFrameListeners return a value?

It doesn't really need to given the existing consumer code, but if we ever want to know whether the listener really got added...  I can remove it for now; easy enough to restore if we need it.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/c05c386324c0

Comment 12

[Eric Shepherd [:sheppy]]

I've updated the doc here:

https://developer.mozilla.org/en/DOM/window.mozRequestAnimationFrame