[Webkit-unassigned] [Bug 15955] WebCore should use threading abstraction

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=15955


beidson at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #17203|review?                     |review-
               Flag|                            |




------- Comment #2 from beidson at apple.com  2007-11-12 09:34 PDT -------
(From update of attachment 17203)
I wanted to get to this ever since we first landed the abstraction - thanks for
taking it on!
I have a few nitpicks:

> createThread( IconDatabase::iconDatabaseSyncThreadStart, this );  
No space inside the parens.  Occurs in a few places.

> if(!m_syncThread)
Space after the if.

> -#if PLATFORM(DARWIN)
> -    ASSERT(pthread_main_np());
> -#endif
This block of code is valuable, and I would like to see it eventually expanded
to include all platforms.
The Threading abstraction doesn't yet support a "getMainThreadID()" so there's
not (yet) a drop in replacement, so I'm a little torn on what to do with it
now...  Since adding such an interface would be an exercise in adding platform
specific code whereas this patch is an exercise in removing it!  
I think leaving this block for now (along with the pthread.h include for DARWIN
only) and filing a bug with a FIXME would be sufficient.

> -#ifndef NDEBUG
> -    memset(&m_openingThread, 0, sizeof(pthread_t));
> -#endif
It might not be critically necessary since it's debug-only code, but I'd like
to see the thread identifier zero'ed out in the initialization list in place of
this code.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.