Summary: | [EFL][WK2] Fix WebKit2-EFL build | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mariusz Grzegorczyk <mariusz.g> | ||||||||||||
Component: | WebKit2 | Assignee: | Ryuan Choi <ryuan.choi> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | cgarcia, g.czajkowski, mrobinson, rakuco, ryuan.choi, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Linux | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 61838 | ||||||||||||||
Attachments: |
|
Description
Mariusz Grzegorczyk
2012-04-11 08:35:08 PDT
Created attachment 136675 [details]
Patch fixing webkit2 efl build
Please rebase this patch. (In reply to comment #2) > Please rebase this patch. This patch doesn't need rebase because depends on bug 61850. Dependency/Blocks fields added. Created attachment 144546 [details]
Patch
(In reply to comment #4) > Created an attachment (id=144546) [details] > Patch Updated patch to catch latest changes and removed dependency of Bug 61850 from patch. Comment on attachment 144546 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144546&action=review Since the EWS does not test this, the most I can do is comment on the build system changes. > Source/WebKit2/CMakeLists.txt:497 > +SET(WebProcess_NAME ../bin/WebProcess) It's better to use SET_TARGET_PROPERTIES(... RUNTIME_OUTPUT_DIRECTORY ...) like other parts of the build system do. > Source/WebKit2/CMakeLists.txt:498 > +SET(WebProcess_SOURCES "") Is this empty definition needed? > Source/WebKit2/CMakeLists.txt:507 > +SET(ForwardingHeaders_NAME ) > +SET(ForwardingNetworkHeaders_NAME ) Are these empty definitions needed? > Source/cmake/OptionsEfl.cmake:42 > +ADD_DEFINITIONS(-DBUILDING_SOUP__=1) I see that this is checked in WK2, but I couldn't find any part of gtk's build system defining that. Is that expected? Created attachment 144572 [details]
Patch
(In reply to comment #6) > (From update of attachment 144546 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=144546&action=review > > Since the EWS does not test this, the most I can do is comment on the build system changes. > > > Source/WebKit2/CMakeLists.txt:497 > > +SET(WebProcess_NAME ../bin/WebProcess) > > It's better to use SET_TARGET_PROPERTIES(... RUNTIME_OUTPUT_DIRECTORY ...) like other parts of the build system do. > Thanks for the comment. I fixed. > > Source/WebKit2/CMakeLists.txt:498 > > +SET(WebProcess_SOURCES "") > > Is this empty definition needed? > > > Source/WebKit2/CMakeLists.txt:507 > > +SET(ForwardingHeaders_NAME ) > > +SET(ForwardingNetworkHeaders_NAME ) > > Are these empty definitions needed? > Removed them. > > Source/cmake/OptionsEfl.cmake:42 > > +ADD_DEFINITIONS(-DBUILDING_SOUP__=1) > > I see that this is checked in WK2, but I couldn't find any part of gtk's build system defining that. Is that expected? yes, GNUmakefile.am also defines it. Comment on attachment 144572 [details]
Patch
The build system part looks fine, the rest should supposedly be OK.
CCing mrobinson because I touched WebKit2/Gtk+ related files. The libsoup changes look fine to me, but I'm CCing Carlos as he's the original author of that code. Looks good to me too, I shoulnd't have used Gtk headers in soup file, sorry. Comment on attachment 144572 [details]
Patch
Though I don't like to mix multiple patches personally, informal rs+ on my side in order to build EFL WebKit2 now. But, I think this patch should be reviewed by GTK or WK2 reviewers.
Comment on attachment 144572 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144572&action=review Looks good, just a couple of things you can change before landing. > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:66 > +#if ENABLE(INPUT_TYPE_COLOR) > +#include "ColorChooser.h" > +#endif > + Use #include <WebCore/ColorChooser.h> like all other webcore headers in this file. You don't need the #if ENABLE(INPUT_TYPE_COLOR) because the header is already protected by that check. Remember to move the header to keep all headers sorted alpahbetically > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:78 > +#if ENABLE(WEB_INTENTS) > +#include "IntentRequest.h" > +#endif And same here for IntentRequest.h Created attachment 144749 [details]
patch for landing
Comment on attachment 144749 [details]
patch for landing
Thank you so much.
Comment on attachment 144749 [details] patch for landing Attachment 144749 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12843781 Comment on attachment 144749 [details] patch for landing Attachment 144749 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12845820 Created attachment 144765 [details]
patch for landing2
Comment on attachment 144765 [details] patch for landing2 Clearing flags on attachment: 144765 Committed r118914: <http://trac.webkit.org/changeset/118914> All reviewed patches have been landed. Closing bug. |