Bug 119905 – [iOS] Upstream Source/WTF

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Joseph Pecoraro]

Upstream the remaining iOS WebKit diffs in Source/WTF.

[Joseph Pecoraro]

Created attachment 208944 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

<rdar://problem/14750668>

[Benjamin Poulain]

Comment on attachment 208944 [details]
[PATCH] Proposed Fix

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

[Joseph Pecoraro]

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

[Joseph Pecoraro]

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

On second thought, I want to keep them in. It is easy to remove them later if we wish. Mac does this with their features and it seems useful for documentation. For example this one as well in Platform.h:

   #if (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 60000) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1080)
  #define WTF_USE_COREMEDIA 1
  #endif

[Joseph Pecoraro]

Committed <http://trac.webkit.org/changeset/154294>.