[webkit-reviews] review denied: [Bug 74246] #ifdef the parts of Mac platform which should not be used on iOS : [Attachment 118696] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Dec 10 20:08:16 PST 2011


David Kilzer (ddkilzer) <ddkilzer at webkit.org> has denied Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 74246: #ifdef the parts of Mac platform which should not be used on iOS
https://bugs.webkit.org/show_bug.cgi?id=74246

Attachment 118696: Patch
https://bugs.webkit.org/attachment.cgi?id=118696&action=review

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=118696&action=review


I don't think we should merge the WebThread ASSERT() changes yet.  Also, the
BUILDING_ON_<BIG_CAT> macros shouldn't be defined on iOS, so that change
shouldn't be necessary.

> Source/WebCore/platform/mac/EmptyProtocolDefinitions.h:40
> +#if (defined(BUILDING_ON_LEOPARD) || defined(BUILDING_ON_SNOW_LEOPARD)) &&
!PLATFORM(IOS)

Is this change necessary?  BUILDING_ON_LEOPARD and BUILDING_ON_SNOW_LEOPARD
should never be defined when building iOS.

> Source/WebCore/platform/mac/Language.mm:51
> +#if !PLATFORM(IOS)
>      ASSERT(isMainThread());
> +#else
> +    ASSERT(isMainThread() || pthread_main_np());
> +#endif

This change is related to the WebThread on iOS.

> Source/WebCore/platform/mac/Language.mm:69
> +#if !PLATFORM(IOS)
>      ASSERT(isMainThread());
> +#else
> +    ASSERT(isMainThread() || pthread_main_np());
> +#endif

Ditto.

> Source/WebCore/platform/mac/Language.mm:102
> +#if !PLATFORM(IOS)
>      ASSERT(isMainThread());
> +#else
> +    ASSERT(isMainThread() || pthread_main_np());
> +#endif

Ditto.

> Source/WebCore/platform/mac/LocalizedStringsMac.mm:42
> +    // Can be called on a dispatch queue when initializing strings on iOS
> +    // See LoadWebLocalizedStrings and <rdar://problem/7902473>.

First comment needs a period at the end of the sentence.

> Source/WebCore/platform/mac/SharedBufferMac.mm:57
> +#if !PLATFORM(IOS)
>      JSC::initializeThreading();
>  #if PLATFORM(QT) && USE(QTKIT)
>      WTF::initializeMainThread();
>  #else
>      WTF::initializeMainThreadToProcessMainThread();
>  #endif
> +#endif // !PLATFORM(IOS)

This change is related to the WebThread on iOS.


More information about the webkit-reviews mailing list