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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Jan 24 23:26:27 PST 2009

Dmitry Titov <dimich at chromium.org> has asked  for review:
Bug 23488: Make TimerBase thread-aware. For use in Workers.

Attachment 27008: Review comments addressed.

------- 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::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
- updated ChangeLog

More information about the webkit-reviews mailing list