Bug 23312 – Implement MessageQueue::waitForMessageTimed()

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Dmitry Titov]

Yet another part of 'timers in workers' (bug 22718).
The Win32 version of ThreadCondition::waitTimed() still needs to be implemented, that will be a separate patch.

[Dmitry Titov]

Created attachment 26704 [details]
Proposed patch

Also changed ThreadCondition::timedWait() to take absolute time, as discussed on webkit-dev.
Built Mac and Win.

[Alexey Proskuryakov]

Comment on attachment 26704 [details]
Proposed patch

+        (WTF::):

It's best to correct the script in cases like this to have more reasonable ChangeLogs.

+    enum MessageQueueWaitResult {

MessageQueue itself is made visible in global namespace by a using directive, you should do the same for the result type and its values. We generally prefer to put "using" declarations for "public" WTF symbols, and leave ones that are implementation details in the WTF namespace without importing them. Taking this approach has the benefit that no one is tempted to say "using namespace WTF", so risk of collision with the non-public symbols is reduced.

Historically, there was some disagreement on this point, so many public symbols do not have "using" declarations at the moment.

+    // The absoluteTime is in seconds, starting on January 1, 1970.

It could be helpful to clarify what time zone is used (maybe by saying that the time is measured in the same units as currentTime() return value?)

+    // QT defines wait for up to ULONG_MAX milliseconds.

QT is QuickTime, and this file is for Qt.

-    // Empty for now
+    // FIXME: Implement.

There's also notImplemented() - but I guess that you are probably going to implement this soon anyway.

r=me, but please consider my nitpicks.

[Alexey Proskuryakov]

> There's also notImplemented()

Sorry, that lives in WebCore/platform.

[Dmitry Titov]

Created attachment 26725 [details]
Updated patch

Updated with Alexey's comments:
- updated ChangeLog to be more informative
- added 'using WTF::MessageQueueWaitResult', also for its values
- fixed typo /QT/Qt/
- left FIXME (and yes, I'll implement this very soon)

It seems this one is ready for landing.

[Alexey Proskuryakov]

Committed revision 39908.