[webkit-reviews] review granted: [Bug 64544] Timer scheduling should be based off the monotonic clock : [Attachment 101080] ready for review, chromium DEPS have rolled

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted James Robinson
<jamesr at chromium.org>'s request for review:
Bug 64544: Timer scheduling should be based off the monotonic clock
https://bugs.webkit.org/show_bug.cgi?id=64544

Attachment 101080: ready for review, chromium DEPS have rolled
https://bugs.webkit.org/attachment.cgi?id=101080&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=101080&action=review


> Source/WebCore/platform/ThreadTimers.cpp:84
> -	   m_sharedTimer->setFireTime(m_timerHeap.first()->m_nextFireTime);
> +	   m_sharedTimer->setFireInterval(m_timerHeap.first()->m_nextFireTime -
monotonicallyIncreasingTime());

Will setFireInterval correctly handle a negative number? Do we need to add code
to handle that case?

It seems to me that half of the functions check against zero and another half
don't. Could we hoist that check for negative numbers into this function?

Seems lame that monotonicallyIncreasingTime is subtracted here and then added
back inside setFireInterval.

> Source/WebCore/platform/wince/SharedTimerWinCE.cpp:95
> +    double intervalMillis = intervalSeconds * 1000.;

Why not use the real name, milliseconds, or the standard abbreviation, ms,
rather than the coined term millis?