Bug 68769 – [mac] Timestamp parameter to requestAnimationFrame is busted in USE(REQUEST_ANIMATION_FRAME_TIMER) path

• Component: New Bugs
• Status: RESOLVED FIXED

[James Robinson]

The time parameter for requestAnimationFrame should be roughly equivalent to Date.now(), but in the USE(REQUEST_ANIMATION_FRAME_TIMER) path it has the wrong value.  This is because animationTimerFired() passes a double value from WTF::currentTime(), which is in seconds since the epoch, to serviceScriptedAnimations() which expects to receive a DOMTimeStamp, which should be an unsigned long of milliseconds since the epoch.

To convert the return value of WTF::currentTime() to a double you should use the helper convertSecondsToDOMTimeStamp()

[James Robinson]

Created attachment 108595 [details]
Patch

[WebKit Review Bot]

Comment on attachment 108595 [details]
Patch

Attachment 108595 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9837495

New failing tests:
fast/animation/request-animation-frame-timestamps-advance.html

[James Robinson]

Created attachment 108601 [details]
test that passes in chromium DRT

[Simon Fraser (smfr)]

Comment on attachment 108601 [details]
test that passes in chromium DRT

View in context: https://bugs.webkit.org/attachment.cgi?id=108601&action=review

> LayoutTests/fast/animation/script-tests/request-animation-frame-timestamps-advance.js:24
> +    if (window.layoutTestController)
> +        layoutTestController.display();

display() doesn't do anything for rAF in Safari/Mac. We need to figure out a way to make rAF tests that work in Safari and Chromium.

[James Robinson]

Comment on attachment 108601 [details]
test that passes in chromium DRT

This test does pass in both, the .display()s trigger the rAF in chromium and yielding to the message loop triggers the rAF in Safari.

I think we should add a layoutTestController hook to trigger rAF callbacks, but it's a big pain in the butt to wire up layoutTestController additions to every build system so I haven't undertaken that task yet.

[WebKit Review Bot]

Comment on attachment 108601 [details]
test that passes in chromium DRT

Clearing flags on attachment: 108601

Committed r95937: <http://trac.webkit.org/changeset/95937>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

(In reply to comment #5)

> I think we should add a layoutTestController hook to trigger rAF callbacks, but it's a big pain in the butt to wire up layoutTestController additions to every build system so I haven't undertaken that task yet.

Would adding the hook to window.internals make sense?

[James Robinson]

I think yes - I was thinking of adding something to layoutTestController since that's where .display() is, but since this is supposed to exercise WebCore logic exclusively I guess window.internals would make sense.