[webkit-reviews] review granted: [Bug 171029] [AppleWin] The procedure entry point ?waitForThreadCompletion at WTF@@YAHI at Z could not be located in the dynamic link library WebKitQuartzCoreAdditions.dll : [Attachment 307962] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted  review:
Bug 171029: [AppleWin] The procedure entry point
?waitForThreadCompletion at WTF@@YAHI at Z could not be located in the dynamic link
library WebKitQuartzCoreAdditions.dll
https://bugs.webkit.org/show_bug.cgi?id=171029

Attachment 307962: Patch

https://bugs.webkit.org/attachment.cgi?id=307962&action=review




--- Comment #10 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 307962
  --> https://bugs.webkit.org/attachment.cgi?id=307962
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=307962&action=review

> Source/WTF/ChangeLog:9
> +	   waitForThreadCompletion and createThread. This patch implements the
both on the top of the new APIs.

This patch implements BOTH ON TOP of the new APIs.

> Source/WTF/wtf/ThreadHolder.cpp:-53
> -	   threadSpecificSet(m_key, new ThreadHolder(thread));

Let's keep this ugly:
#if !OS(WINDOWS)
threadSpecificSet ...
#else
// FIXME: Remove this workaround code once <rdar://problem/31793213> is fixed.

> Source/WTF/wtf/ThreadHolder.cpp:55
> +	   platformInitialize(holder);

#endif

> Source/WTF/wtf/ThreadHolder.h:78
> +    static void platformInitialize(ThreadHolder*);

I think this should be "#if OS(WINDOWS)", too.

> Source/WTF/wtf/ThreadHolderPthreads.cpp:52
> +}

Remove this whole thing.

> Source/WTF/wtf/ThreadHolderWin.cpp:72
>  

Add a comment:

"// FIXME: Remove this workaround code once <rdar://problem/31793213> is
fixed."

> Source/WTF/wtf/Threading.h:219
> +// https://bugs.webkit.org/show_bug.cgi?id=171029

Remove this Bugzilla bug, since this is the bug where the workaround was added.

Instead, say: "// Remove this workaround code when <rdar://problem/31793213> is
fixed."

> Source/WTF/wtf/ThreadingWin.cpp:514
>  

Add a comment:

"// FIXME: Remove this workaround code once <rdar://problem/31793213> is
fixed."