[CMake] Some macros need to be defined/undefined, rather than ON/OFF

[Philip Chimento]

I ran into these three errors while compiling the 2.9.1 GTK port with no X11, with MiniBrowser turned on but developer mode off:

In file included from /Users/fliep/gtk/source/webkitgtk-2.9.1/Source/WebCore/bindings/js/ScriptController.cpp:39:
In file included from /Users/fliep/gtk/source/webkitgtk-2.9.1/Source/WebCore/bridge/NP_jsobject.h:31:
In file included from /Users/fliep/gtk/source/webkitgtk-2.9.1/Source/WebCore/bridge/npruntime_internal.h:28:
/Users/fliep/gtk/source/webkitgtk-2.9.1/Source/WebCore/plugins/npapi.h:89:10: fatal error: 
      'X11/Xlib.h' file not found
#include <X11/Xlib.h>
         ^
-------------
/Users/fliep/gtk/source/webkitgtk-2.9.1/Source/WebKit2/PluginProcess/unix/PluginProcessMainUnix.cpp:60:25: error: 
      unknown type name 'Display'; did you mean 'WebCore::EDisplay'?
static int webkitXError(Display* xdisplay, XErrorEvent* error)
                        ^~~~~~~
                        WebCore::EDisplay
/Users/fliep/gtk/source/webkitgtk-2.9.1/Source/WebCore/rendering/style/RenderStyleConstants.h:536:6: note: 
      'WebCore::EDisplay' declared here
enum EDisplay {
     ^
/Users/fliep/gtk/source/webkitgtk-2.9.1/Source/WebKit2/PluginProcess/unix/PluginProcessMainUnix.cpp:60:44: error: 
      unknown type name 'XErrorEvent'
static int webkitXError(Display* xdisplay, XErrorEvent* error)
                                           ^
---------------
/Users/fliep/gtk/source/webkitgtk-2.9.1/Tools/MiniBrowser/gtk/main.c:260:45: error: 
      use of undeclared identifier 'WEBKIT_INJECTED_BUNDLE_PATH'
    g_setenv("WEBKIT_INJECTED_BUNDLE_PATH", WEBKIT_INJECTED_BUNDLE_PATH, FALSE);
                                            ^

This is because of the preprocessor variables MOZ_X11, XP_UNIX, and DEVELOPMENT_BUILD, respectively, being set to the values ON or OFF by the CMake build system, whereas the code expects them to be either defined or undefined. In the common case where they were ON, they were naturally also defined, and so the problem was not immediately apparent.

[Philip Chimento]

The latter failure (DEVELOPMENT_BUILD) has already been changed on master but Tools/MiniBrowser/gtk/main.c:260 may still need to read #if ENABLE(DEVELOPER_MODE) instead of #if ENABLE_DEVELOPER_MODE - I'm not exactly sure how that WTF mechanism works.

[Philip Chimento]

Created attachment 252822 [details]
Patch

[Michael Catanzaro]

Thanks for your patches. I'd like to change the code to be more robust to this, checking values rather than definitions. But I don't see value in fighting that fight in npapi.h, so this is probably the best approach. Let's see what Martin thinks. There should for sure be a warning comment, though, or someone will revert this patch in passing as a cleanup.

(In reply to comment #1)
> The latter failure (DEVELOPMENT_BUILD) has already been changed on master
> but Tools/MiniBrowser/gtk/main.c:260 may still need to read #if
> ENABLE(DEVELOPER_MODE) instead of #if ENABLE_DEVELOPER_MODE - I'm not
> exactly sure how that WTF mechanism works.

ENABLE(DEVELOPER_MODE) is true if ENABLE_DEVELOPER_MODE is defined and its value is not 0, and false otherwise. Since MiniBrowser uses the WebKitGTK+ public API and not WTF internals, ENABLE(DEVELOPER_MODE) is unavailable to it, so it just checks ENABLE_DEVELOPER_MODE directly.

[Philip Chimento]

Created attachment 252827 [details]
Patch

[Philip Chimento]

(In reply to comment #3)
> Thanks for your patches. I'd like to change the code to be more robust to
> this, checking values rather than definitions. But I don't see value in
> fighting that fight in npapi.h, so this is probably the best approach. Let's
> see what Martin thinks. There should for sure be a warning comment, though,
> or someone will revert this patch in passing as a cleanup.

Sure thing. I did kind of assume that npapi.h was off limits. I've added a warning comment.

[Martin Robinson]

Comment on attachment 252827 [details]
Patch

Thank you for fixing this.

[Philip Chimento]

You're welcome. Thanks for the reviews.

[WebKit Commit Bot]

Comment on attachment 252827 [details]
Patch

Clearing flags on attachment: 252827

Committed r184062: <http://trac.webkit.org/changeset/184062>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.