[webkit-reviews] review granted: [Bug 170625] Start using MonotonicTime / Seconds in Timer class : [Attachment 306546] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Chris Dumez
<cdumez at apple.com>'s request for review:
Bug 170625: Start using MonotonicTime / Seconds in Timer class
https://bugs.webkit.org/show_bug.cgi?id=170625

Attachment 306546: Patch

https://bugs.webkit.org/attachment.cgi?id=306546&action=review




--- Comment #2 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 306546
  --> https://bugs.webkit.org/attachment.cgi?id=306546
Patch

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

> Source/WebCore/page/SuspendableTimer.cpp:118
> +	   m_savedRepeatInterval = Seconds { 0 };

0_s

> Source/WebCore/page/SuspendableTimer.cpp:128
>      return 0;

Why not change this function to return a Seconds?

> Source/WebCore/page/SuspendableTimer.cpp:140
> +	   m_savedRepeatInterval = Seconds { 0 };

0_s

> Source/WebCore/platform/ThreadTimers.cpp:46
> +static const Seconds maxDurationOfFiringTimers { 0.050 };

50_ms

> Source/WebCore/platform/ThreadTimers.cpp:92
> +	   m_sharedTimer->setFireInterval(std::max(nextFireTime -
currentMonotonicTime, Seconds { }));

0_s

> Source/WebCore/platform/ThreadTimers.cpp:122
> +	   if (!m_firingTimers || timeToQuit < MonotonicTime::now())
>	       break;

Should we capture MonotonicTime::now() outside the loop?

> Source/WebCore/platform/Timer.cpp:216
> +    m_repeatInterval = Seconds { };

0_s

> Source/WebCore/platform/Timer.cpp:220
> +    ASSERT(m_repeatInterval == Seconds { 0 });

0_s

> Source/WebCore/platform/Timer.cpp:224
>  double TimerBase::nextFireInterval() const

Return a Seconds?

> Source/WebCore/platform/Timer.cpp:416
> +    return std::max(m_unalignedNextFireTime - MonotonicTime::now(), Seconds
{ 0 });

0_s

> Source/WebCore/platform/Timer.h:68
> +    void startOneShot(Seconds interval) { start(interval, Seconds { 0 }); }

0_s