Add ENABLE_WEBKIT2 option in CMakeLists.txt. If all of sources are landed, it will works.
Created attachment 95870 [details] Patch
Comment on attachment 95870 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95870&action=review > Source/CMakeLists.txt:20 > +SET(DERIVED_SOURCES_WEBKIT2_DIR "${CMAKE_BINARY_DIR}/DerivedSources/WebKit2") How do you think WebKit 2 variables are only set when WEBKIT2 is enabled ?
Comment on attachment 95870 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95870&action=review > Source/CMakeLists.txt:125 > ADD_SUBDIRECTORY(WebKit) > +IF (ENABLE_WEBKIT2) > + ADD_SUBDIRECTORY(WebKit2) > +ENDIF () WebKit always be built if one only wants WebKit2. Is this the expected behaviour? > Source/CMakeLists.txt:133 > +IF (ENABLE_WEBKIT2) > + INCLUDE_IF_EXISTS(${TOOLS_DIR}/MiniBrowser/${WEBKIT_PORT_DIR}/CMakeLists${PORT}.txt) > +ENDIF () I'd change Tools/CMakeListsEfl.txt instead.
(In reply to comment #2) > (From update of attachment 95870 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95870&action=review > > > Source/CMakeLists.txt:20 > > +SET(DERIVED_SOURCES_WEBKIT2_DIR "${CMAKE_BINARY_DIR}/DerivedSources/WebKit2") > > How do you think WebKit 2 variables are only set when WEBKIT2 is enabled ? If then, we should check webkit2 option at front of Source/CMakeLists.txt. cmake/WebKitFS.cmake should know that DERIVED_SOURCES_WEBKIT2_DIR. If it's not harmful, I like it as default. (In reply to comment #3) > (From update of attachment 95870 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95870&action=review > > > Source/CMakeLists.txt:125 > > ADD_SUBDIRECTORY(WebKit) > > +IF (ENABLE_WEBKIT2) > > + ADD_SUBDIRECTORY(WebKit2) > > +ENDIF () > > WebKit always be built if one only wants WebKit2. Is this the expected behaviour? > I'll change not to build webkit when WebKit2 was enabled. My first patch is exactly what I want like WebKit2/Gtk. But now, we don't have any requirements for that. So, only one library for webkit2 looks fine. > > Source/CMakeLists.txt:133 > > +IF (ENABLE_WEBKIT2) > > + INCLUDE_IF_EXISTS(${TOOLS_DIR}/MiniBrowser/${WEBKIT_PORT_DIR}/CMakeLists${PORT}.txt) > > +ENDIF () > > I'd change Tools/CMakeListsEfl.txt instead. Ok. If then, I'll remove it and it will be covered by Bug 61850
Created attachment 96165 [details] updated_patch
Created attachment 96174 [details] updated_patch
Comment on attachment 96174 [details] updated_patch View in context: https://bugs.webkit.org/attachment.cgi?id=96174&action=review > Source/CMakeLists.txt:85 > +SET(WebKit2_LIBRARY_NAME WebKit2) I think this setting also can be enabled when WEBKIT2 is enabled. Any problems ? > Source/CMakeLists.txt:103 > +SET(WebKit2_LIBRARY_TYPE SHARED) same.
Created attachment 96205 [details] updated_patch
(In reply to comment #7) > (From update of attachment 96174 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96174&action=review > > > Source/CMakeLists.txt:85 > > +SET(WebKit2_LIBRARY_NAME WebKit2) > > I think this setting also can be enabled when WEBKIT2 is enabled. Any problems ? > > > Source/CMakeLists.txt:103 > > +SET(WebKit2_LIBRARY_TYPE SHARED) > > same. No problem. I updated.
Comment on attachment 96205 [details] updated_patch On a quick glance, looks good; but again, I'm not familiar with WebKit2 internals so I can't properly review this. I'd split this into two patches: one for the platform-independent part, and another for the EFL-related part. Makes it easier to review.
Created attachment 96358 [details] updated_patch
(In reply to comment #10) > (From update of attachment 96205 [details]) > On a quick glance, looks good; but again, I'm not familiar with WebKit2 internals so I can't properly review this. > > I'd split this into two patches: one for the platform-independent part, and another for the EFL-related part. Makes it easier to review. Thanks. As your suggestion, I split patches and created Bug 62260 for platform independent part.
Created attachment 96369 [details] updated_patch
Comment on attachment 96369 [details] updated_patch LGTM.
Created attachment 96673 [details] Patch
(In reply to comment #15) > Created an attachment (id=96673) [details] > Patch I updated to follow changes of Bug 62260, Bug 62363.
Comment on attachment 96673 [details] Patch Attachment 96673 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/8826394
Created attachment 96907 [details] Patch
(In reply to comment #17) > (From update of attachment 96673 [details]) > Attachment 96673 [details] did not pass efl-ews (efl): > Output: http://queues.webkit.org/results/8826394 I can't understand this failure. I updated same patch for checking once more.
Hello, I have made a patch that depends on your patch. It will be great if your patch could be merged.
(In reply to comment #20) > Hello, > I have made a patch that depends on your patch. It will be great if your patch could be merged. IMO, you'd better to upload new patch for this bug instead of creating new one.
(In reply to comment #21) > (In reply to comment #20) > > Hello, > > I have made a patch that depends on your patch. It will be great if your patch could be merged. > > IMO, you'd better to upload new patch for this bug instead of creating new one. OK, I will join our patches.
*** Bug 70802 has been marked as a duplicate of this bug. ***
Created attachment 112311 [details] Patch
r100362 removed WebCookieManagerEfl.cpp. (http://trac.webkit.org/changeset/100362) So, remove WebCookieManagerEfl.cpp from CMakeListEfl.txt and insert WebCookieManagerSoup.cpp, WebCookieManagerCurl.cpp If WTF_USE_SOUP is defined, WebCookieManagerSoup.cpp should be built. If WTF_USE_CURL, WebCookieManagerCurl.cpp should be built.
(In reply to comment #25) > r100362 removed WebCookieManagerEfl.cpp. (http://trac.webkit.org/changeset/100362) > So, remove WebCookieManagerEfl.cpp from CMakeListEfl.txt and insert WebCookieManagerSoup.cpp, WebCookieManagerCurl.cpp > > If WTF_USE_SOUP is defined, WebCookieManagerSoup.cpp should be built. > If WTF_USE_CURL, WebCookieManagerCurl.cpp should be built. Right, and also we decide to use PlatformEfl.cmake for platform specific things. Tomasz, Can you rebase it? Otherwise I can do.
Created attachment 115485 [details] rebased
(In reply to comment #27) > Created an attachment (id=115485) [details] > rebased Apply Jongseok's change and Bug 56705's changes which decide to use Platform${Port}.cmake instead of CMakeLists${Port}.cmake
Comment on attachment 115485 [details] rebased rs=me.
Created attachment 120228 [details] rebased
Comment on attachment 120228 [details] rebased Thanks eric. I rebased patch for applying eunmi's patch (Bug 62444)
Comment on attachment 120228 [details] rebased Clearing flags on attachment: 120228 Committed r103512: <http://trac.webkit.org/changeset/103512>
All reviewed patches have been landed. Closing bug.