[Webkit-unassigned] [Bug 119905] [iOS] Upstream Source/WTF

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=119905





--- Comment #4 from Joseph Pecoraro <joepeck at webkit.org>  2013-08-19 12:21:32 PST ---
(In reply to comment #3)
> (From update of attachment 208944 [details])
> 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.

All comments in Assertions.h use this style. Though I will update the grammar as estes had mentioned to me.


> > 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.

I will remove this. I don't think anyone is compiling older iOS WebKits.

That said, the PLATFORM(IOS) later in the statement should only have been needed for iOS versions older then 70000, so I can now remove that.


> > 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());

Sure, I will make this change.

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