Bug 47167 - [Qt] Webkit2 MacOS build fix
Summary: [Qt] Webkit2 MacOS build fix
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Normal
Assignee: Luiz Agostini
URL:
Keywords: Qt
Depends on: 47177 47589
Blocks:
  Show dependency treegraph
 
Reported: 2010-10-05 04:26 PDT by Luiz Agostini
Modified: 2010-10-13 06:44 PDT (History)
6 users (show)

See Also:


Attachments
patch (3.26 KB, patch)
2010-10-05 04:41 PDT, Luiz Agostini
koivisto: review+
Details | Formatted Diff | Diff
patch (5.22 KB, patch)
2010-10-07 05:46 PDT, Luiz Agostini
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Luiz Agostini 2010-10-05 04:26:55 PDT
Webkit2 MacOS build fix
Comment 1 Luiz Agostini 2010-10-05 04:41:12 PDT
Created attachment 69770 [details]
patch
Comment 2 Luiz Agostini 2010-10-05 06:19:36 PDT
Committed r69103: <http://trac.webkit.org/changeset/69103>
Comment 3 Adam Roben (:aroben) 2010-10-05 06:43:22 PDT
This change broke the Windows build: <http://build.webkit.org/builders/Windows%20Debug%20%28Build%29/builds/25146/steps/compile-webkit/logs/stdio>

This is breaking Windows because WKNativeEvent.h is an API header, but PLATFORM() is not API. We need to remove the use of PLATFORM from this public header.
Comment 4 Ademar Reis 2010-10-05 06:56:36 PDT
Please correct me if I'm wrong, but there's no plan to support webkit2 in qt-webkit-2.1, so I'm removing the block of Bug 39121 (qt-webkit-2.1 master bug).
Comment 5 Luiz Agostini 2010-10-05 07:59:05 PDT
WKNativeEvent.h:

#ifdef __APPLE__
#ifdef __OBJC__
@class NSEvent;
#else
struct NSEvent;
#endif
typedef NSEvent *WKNativeEventPtr;
#elif defined(WIN32) || defined(_WIN32)
typedef const struct tagMSG* WKNativeEventPtr;
#else
typedef const void* WKNativeEventPtr;
#endif

When building Qt in macos it ends up using typedef NSEvent *WKNativeEventPtr; what breaks Qt build. As this is a public file the PLATFORM() macro cannot be used. 
How can we have a Qt specific typedef without using PLATFORM?
Comment 6 Luiz Agostini 2010-10-05 09:20:12 PDT
I am considering that file WKNativeEvent will not be included by QtWebKit users and that it will be wrapped by QtWebKit.
Then it is enough to put a definition in WebKit2.pro. 

[10:48am] lca: kenneth_: I was thinking about defining something in our .pro and using ifdef to check if it is defined
[10:48am] aroben: lca: you'd have to do that for all projects that include WebKit2 headers, too
[10:48am] kenneth_: how would that help a user ot QtWebKit2
[10:48am] kenneth_: exactly
[10:49am] kenneth_: everyone would have to put that in their .pro

Kenneth, Adam, is WKNativeEvent really public API? Should not Qt wrap it?
Comment 7 Adam Roben (:aroben) 2010-10-05 09:50:54 PDT
(In reply to comment #6)
> Kenneth, Adam, is WKNativeEvent really public API? Should not Qt wrap it?

It is used by WKPage. But since you wrap WKPage it seems find to wrap WKNativeEvent, too.
Comment 8 Kenneth Rohde Christiansen 2010-10-05 11:41:02 PDT
Thought we might expose the C API as well.
Comment 9 Luiz Agostini 2010-10-07 05:46:08 PDT
Created attachment 70070 [details]
patch

I rolled out last patch because it broke some builds. The problem was that PLATFORM() macro may not be used in file WKNativeEvent.h.

Qt build is broken in MacOS because of the use of __APPLE__ macro in that file. As it is a public file it is not easy conciliate the use of __APPLE__
and Qt when building Qt in MacOS. A solution is to have a Qt specific WKNativeEvent.h that only has the typedef needed by Qt.
Comment 10 Luiz Agostini 2010-10-07 06:14:04 PDT
Committed r69304: <http://trac.webkit.org/changeset/69304>
Comment 11 Balazs Kelemen 2010-10-07 06:28:34 PDT
I do not like that we need a new file. Why not just change #ifdef __APPLE__ to
#ifdef __APPLE__ && !defined(BUILDING_QT__) ?
Comment 12 Luiz Agostini 2010-10-07 08:21:34 PDT
(In reply to comment #11)
> I do not like that we need a new file. Why not just change #ifdef __APPLE__ to
> #ifdef __APPLE__ && !defined(BUILDING_QT__) ?

Because it is a public file. QtWebKit users would need to define BUILDING_QT__ if they want to include WKNativeEvent.h.