[webkit-reviews] review requested: [Bug 23488] Make TimerBase thread-aware. For use in Workers. : [Attachment 27008] Review comments addressed.

[bugzilla-daemon at webkit.org]

Dmitry Titov <dimich at chromium.org> has asked  for review:
Bug 23488: Make TimerBase thread-aware. For use in Workers.
https://bugs.webkit.org/show_bug.cgi?id=23488

Attachment 27008: Review comments addressed.
https://bugs.webkit.org/attachment.cgi?id=27008&action=review

------- Additional Comments from Dmitry Titov <dimich at chromium.org>
Thanks for review!
I think this patch addresses all the comments:

- added vertical space to SharedTimer.h
- moved fireTimersInNestedEventLoop back as static to TimerBase. It redirects
to ThreadTimers internally. So other source files do not need to know about
ThreadTimers.
- ThreadTimers::timerHeap() now returns reference, not a pointer. Removed
checks for zero in callers.
- ditto for the ThreadTimers::timersReadyToFire, but with a twist: this one had
a dual purpose. It was used to point to a stack-allocated set of timers and
also serve as reentrancy guard (this is why it could be 0 before and some
checks for 0 were necessary). However, this wasn't very good for readability so
I've split this into 2 variables - bool m_firingTimers and m_timersReadyToFire
that is never 0 (but can be empty).
- split ThreadTimers off into a separate .h and .cpp files. Couldn't make it
private since it has to be used in ThreadGlobalData.cpp as well as in
Timer.cpp.
- updated ChangeLog