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

[bugzilla-daemon at webkit.org]

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

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #255295|review?, commit-queue?      |review+, commit-queue-
              Flags|                            |

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

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

For the long term health of our configuration macros: Use of PLATFORM(XXX) is to be minimized. PLATFORM(COCOA) for low level stuff is particularly bad.

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

Rather than nesting, I prefer to write each condition out separately. It would look like this:

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

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

#if PLATFORM(GTK) && PLATFORM(WAYLAND) && !defined(GTK_API_VERSION_2)
#include <gdk/gdkwayland.h>
#endif

This is more tidy than the nested way of writing it, especially with all the "#endif // PLATFORM(GTK)" gunk.

> Source/WebCore/platform/graphics/opentype/OpenTypeMathData.cpp:46
> -#if OS(DARWIN)
> +#if PLATFORM(COCOA)
>  const FourCharCode MATHTag = 'MATH';
>  #else
>  const uint32_t MATHTag = OT_MAKE_TAG('M', 'A', 'T', 'H');

I don’t know why we need this special case for PLATFORM(COCOA); can’t we always use OT_MAKE_TAG?

> Source/WebKit2/Platform/IPC/Attachment.cpp:39
> -#if OS(DARWIN)
> +#if PLATFORM(COCOA)

This is wrong. It should match the header and say:

#if OS(DARWIN) && !USE(UNIX_DOMAIN_SOCKETS)

> Source/WebKit2/UIProcess/Launcher/ProcessLauncher.cpp:61
> -#if OS(DARWIN)
> +#if PLATFORM(COCOA)

This doesn’t seem quite right to me, but I can’t come up with anything better. PLATFORM(COCOA) is a really high level concept and it not the right conditional for “use Mach directly”.

> Source/WebKit2/UIProcess/Launcher/ProcessLauncher.h:64
> +#if PLATFORM(COCOA)

Ditto.

-- 
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/20151024/00c9865a/attachment.html>