[webkit-reviews] review granted: [Bug 119905] [iOS] Upstream Source/WTF : [Attachment 208944] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 119905: [iOS] Upstream Source/WTF
https://bugs.webkit.org/show_bug.cgi?id=119905

Attachment 208944: [PATCH] Proposed Fix
https://bugs.webkit.org/attachment.cgi?id=208944&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=208944&action=review


> Source/WTF/wtf/Assertions.h:98
> +/* For project uses WTF but has no config.h, we need to explicitly set the
export defines here. */

Wrong comment style.

> Source/WTF/wtf/Platform.h:1009
> +#if (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 70000) ||
((PLATFORM(MAC) || (OS(WINDOWS) && USE(CG))) && !PLATFORM(IOS) &&
__MAC_OS_X_VERSION_MIN_REQUIRED >= 1090)

Do we really need && __IPHONE_OS_VERSION_MIN_REQUIRED >= 70000?
It's not like we support anything before that.

You should get rid of the !PLATFORM(IOS) later in the list, it makes the #ifdef
hard to follow.

> Source/WTF/wtf/text/StringStatics.cpp:86
> +	   // On iOS WebKit, isMainThread() tests for the Web Thread, so use
pthread_main_np() to ensure this is the real main thread.
> +	   ASSERT(pthread_main_np());

Isn't isUIThread() solving this problem?
You could just replace ASSERT(isMainThread()); by ASSERT(isUIThread());