[webkit-reviews] review granted: [Bug 25348] Change WTF::ThreadIdentifier to be an actual (but wrapped) thread id, remove ThreadMap. : [Attachment 30078] Latest - with Win .def files and deprecated functions.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu May 7 02:03:23 PDT 2009


Alexey Proskuryakov <ap at webkit.org> has granted Dmitry Titov
<dimich at chromium.org>'s request for review:
Bug 25348: Change WTF::ThreadIdentifier to be an actual (but wrapped) thread
id, remove ThreadMap.
https://bugs.webkit.org/show_bug.cgi?id=25348

Attachment 30078: Latest - with Win .def files and deprecated functions.
https://bugs.webkit.org/attachment.cgi?id=30078&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
+	 which it uses. Next time Safari 4 builds, it will pick up new
functions and the depricated ones

"deprecated"

+// In WebKit.def file, these functions are 'renamed' so their name does not
have 'Depricated' part - this

Ditto.

\ No newline at end of file

Please add one (to three files).

+// Platform-independent wrapper of ThreadId. Assignable and copyable.

There is no such thing as ThreadId.

+// It happens that for all current implementations '0' works fine as 'invalid
value'.
+// Comparing is delegated to the platform implementations because it needs to
+// compare in a platform-dependent way.

I'm not sure if these comments add anything - e.g., it's immediately obvious
that comparing is implemented differently for different platforms by looking at
the code alone. Though it is true that it may be slightly surprising that the
only substantial difference from a typedef is a custom operator==.

But I'm still somewhat worried about losing the guarantee that a
ThreadIdentifier's native thread id is null after the thread exits. It is
probably worth adding a comment that ThreadIdentifier is only valid for as long
as its thread is alive, and after that, platformId may be invalid, or may
reference a completely unrelated thread.

r=me


More information about the webkit-reviews mailing list