[Webkit-unassigned] [Bug 144560] [GTK] Fix combinations of PLATFORM(GTK) and OS(DARWIN)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 11 09:24:02 PDT 2015


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

--- Comment #9 from Darin Adler <darin at apple.com> ---
Comment on attachment 252836
  --> https://bugs.webkit.org/attachment.cgi?id=252836
Patch

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

Most of those OS(DARWIN) -> PLATFORM(COCOA) changes don’t seem right to me. I added comments about a few of them. Others sound comment on the use inside WebKit2.

> Source/WTF/wtf/WorkQueue.h:39
> -#if OS(DARWIN)
> +#if PLATFORM(COCOA)
>  #include <dispatch/dispatch.h>
>  #endif

It’s a little sad to go in this direction. Better to use the OS ones than the PLATFORM ones whenever possible.

> Source/WTF/wtf/WorkQueue.h:76
> -#if OS(DARWIN)
> +#if PLATFORM(COCOA)
>      dispatch_queue_t dispatchQueue() const { return m_dispatchQueue; }
>  #elif PLATFORM(GTK)

Better to just put PLATFORM(GTK) first.

> Source/WTF/wtf/WorkQueue.h:106
> -#if OS(DARWIN)
> +#if PLATFORM(COCOA)
>      static void executeFunction(void*);
>      dispatch_queue_t m_dispatchQueue;
>  #elif PLATFORM(GTK)

Better to just put PLATFORM(GTK) first.

> Source/WebCore/platform/graphics/PlatformDisplay.cpp:49
>  #if PLATFORM(GTK)
> +#include <gdk/gdk.h>
> +#if ENABLE(X11_TARGET)
>  #include <gdk/gdkx.h>
>  #endif
> +#endif

To keep includes clean we prefer this style:

    #if PLATFORM(GTK)
    #include <gdk/gdk.h>
    #endif

    #if PLATFORM(GTK) && ENABLE(X11_TARGET)
    #ijnclude <gdk/gdkx.h>
    #endif

The nested includes are harder to read and have no other obvious advantages.

> Source/WebCore/platform/graphics/opentype/OpenTypeMathData.cpp:45
> +#if PLATFORM(COCOA)
>  const FourCharCode MATHTag = 'MATH';
>  #else

Just get rid of this and use the other version everywhere, including Cocoa/Darwin.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150511/40b1417c/attachment-0001.html>


More information about the webkit-unassigned mailing list