Bug 115333

Summary: requestAnimationFrame() reports future timestamp.
Product: WebKit Reporter: zalan <zalan>
Component: WebCore Misc.Assignee: zalan <zalan>
Status: RESOLVED DUPLICATE    
Severity: Major CC: anlo, buildbot, ddkilzer, dino, rniwa, sabouhallawa, simon.fraser
Priority: P2 Keywords: InRadar, PlatformOnly
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion
none
Patch simon.fraser: review-

Description zalan 2013-04-28 13:04:38 PDT
'callback FrameRequestCallback = void (DOMHighResTimeStamp time);'  reports future 'time'

according to the spec (http://www.w3.org/TR/animation-timing/#FrameRequestCallback)

"
Let time be t expressed as the number of milliseconds since 1970-01-01T00:00:00Z. 
...
Perform the steps defined in the invoke callbacks algorithm with parameters list and time
"
(For DOMHighResTimeStamp, the time value measured relative to the navigationStart instead)
Comment 1 zalan 2013-04-28 13:06:03 PDT
patch is coming.
Comment 2 zalan 2013-04-29 13:09:55 PDT
Created attachment 200039 [details]
Patch
Comment 3 zalan 2013-04-29 13:12:00 PDT
it needs https://bugs.webkit.org/show_bug.cgi?id=111244 to be landed first (to get the layouttest work)
Comment 4 David Kilzer (:ddkilzer) 2013-04-29 14:49:32 PDT
<rdar://problem/13758535>
Comment 5 David Kilzer (:ddkilzer) 2013-04-29 14:52:50 PDT
(In reply to comment #0)
> 'callback FrameRequestCallback = void (DOMHighResTimeStamp time);'  reports future 'time'

It's in the future because the timestamp is based on the wrong epoch, correct?
Comment 6 David Kilzer (:ddkilzer) 2013-04-29 15:00:07 PDT
Comment on attachment 200039 [details]
Patch

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

> Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:95
> +    MutexLocker lock(m_mutex);

The Blackberry version of this method uses a tryLock instead, so I don't think it can be replaced with a version that blocks.
Comment 7 Build Bot 2013-04-29 15:52:01 PDT
Comment on attachment 200039 [details]
Patch

Attachment 200039 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/245176

New failing tests:
fast/animation/request-animation-frame-future-timestamp-value.html
Comment 8 Build Bot 2013-04-29 15:52:02 PDT
Created attachment 200052 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2
Comment 9 Build Bot 2013-04-29 18:10:47 PDT
Comment on attachment 200039 [details]
Patch

Attachment 200039 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/249012

New failing tests:
fast/animation/request-animation-frame-future-timestamp-value.html
Comment 10 Build Bot 2013-04-29 18:10:49 PDT
Created attachment 200067 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.2
Comment 11 zalan 2013-04-30 02:08:03 PDT
(In reply to comment #10)
> Created an attachment (id=200067) [details]
> Archive of layout-test-results from webkit-ews-05 for mac-mountainlion
> 
> The attached test failures were seen while running run-webkit-tests on the mac-ews.
> Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.2
layouttest failures are because of the missing performance.now() API. (https://bugs.webkit.org/show_bug.cgi?id=111244)
Comment 12 zalan 2013-04-30 06:20:54 PDT
(In reply to comment #6)
> (From update of attachment 200039 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=200039&action=review
> 
> > Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:95
> > +    MutexLocker lock(m_mutex);
> 
> The Blackberry version of this method uses a tryLock instead, so I don't think it can be replaced with a version that blocks.

I was going to ask the RIM guys to check if it still holds true (whether they've or planning to switch to WK2 as this tryLock() call is irrelevant in WK2 context) also, I am not so sure if skipping frames is a good idea. Anyway, I'll revert those changes and deal with them separately.
Comment 13 zalan 2013-04-30 06:22:48 PDT
(In reply to comment #5)
> (In reply to comment #0)
> > 'callback FrameRequestCallback = void (DOMHighResTimeStamp time);'  reports future 'time'
> 
> It's in the future because the timestamp is based on the wrong epoch, correct?

It is in the future because the timestamp is adjusted with 'timeUntilOutput'.
Comment 14 zalan 2013-04-30 07:07:15 PDT
Created attachment 200113 [details]
Patch
Comment 15 Simon Fraser (smfr) 2013-04-30 11:30:13 PDT
Comment on attachment 200113 [details]
Patch

So we need to capture monotonicallyIncreasingTime() in a way where all the RAF callbacks use the same timestamp, and the same time is used to drive CSS animations. I'm not sure this does that.
Comment 16 Said Abou-Hallawa 2016-06-23 14:56:42 PDT

*** This bug has been marked as a duplicate of bug 159038 ***