[GTK][EFL][WK2] Fix WebKit2 build after r160028
Created attachment 218338 [details] Patch
It's still failing due to changes from bug 125178. ../../Source/WebKit2/Shared/API/c/WKDeclarationSpecifiers.h:49:48: error: missing binary operator before token "(" #if defined(__has_extension) && __has_extension(enumerator_attributes) && __has_extension(attribute_unavailable_with_message) ^ ../../Source/WebKit2/Shared/API/c/WKDeclarationSpecifiers.h:55:48: error: missing binary operator before token "(" #if defined(__has_extension) && __has_extension(enumerator_attributes) && __has_extension(attribute_unavailable_with_message) ^
You can probably try something like #ifndef __has_extension #define __has_extension(x) 0 #endif
(In reply to comment #3) > You can probably try something like > > #ifndef __has_extension > #define __has_extension(x) 0 > #endif From bug 116352, it looks like we should use "__has_feature" instead of "__has_extension". So should we add #ifndef to the WKDeclarationSpecifiers.h or to use "__has_feature" which already is properly handled in Compiler.h ?
(In reply to comment #4) > (In reply to comment #3) > > You can probably try something like > > > > #ifndef __has_extension > > #define __has_extension(x) 0 > > #endif > > From bug 116352, it looks like we should use "__has_feature" instead of "__has_extension". So should we add #ifndef to the WKDeclarationSpecifiers.h or to use "__has_feature" which already is properly handled in Compiler.h ? This is in the API headers so we can’t use ifdefs from Compiler.h
Created attachment 218349 [details] Patch Also adding fix after r160036
Created attachment 218354 [details] Patch Renaming APIClient to API::Client
*** Bug 125187 has been marked as a duplicate of this bug. ***
(In reply to comment #7) > Created an attachment (id=218354) [details] > Patch > > Renaming APIClient to API::Client EFL build is still broken with this patch, some error log: /home/oszi/WebKit/Source/WebKit2/UIProcess/efl/WebUIPopupMenuClient.h:41:112: error: wrong number of template arguments (2, should be 1) /home/oszi/WebKit/Source/WebKit2/Shared/APIClient.h:36:42: error: provided for 'template<class ClientInterface> class API::Client' ...
It seems you will need similar changes to http://trac.webkit.org/changeset/159988
Comment on attachment 218354 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218354&action=review > Source/WebKit2/Shared/API/c/WKDeclarationSpecifiers.h:49 > +#ifndef __has_extension Hey nick, maybe would be cleaner to do some workaround like I did in https://bugs.webkit.org/show_bug.cgi?id=125187, instead of adding some code that does nothing. What do you think? Ossy?
(In reply to comment #9) > (In reply to comment #7) > > Created an attachment (id=218354) [details] [details] > > Patch > > > > Renaming APIClient to API::Client > > EFL build is still broken with this patch, some error log: > > /home/oszi/WebKit/Source/WebKit2/UIProcess/efl/WebUIPopupMenuClient.h:41:112: error: wrong number of template arguments (2, should be 1) > /home/oszi/WebKit/Source/WebKit2/Shared/APIClient.h:36:42: error: provided for 'template<class ClientInterface> class API::Client' > ... Yeah, the changes needed are less trivial I thought initially. ClientTraits was reimplemented..
(In reply to comment #10) > It seems you will need similar changes to http://trac.webkit.org/changeset/159988 Hmm, I'll take a look at it. thanks.
(In reply to comment #11) > (From update of attachment 218354 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=218354&action=review > > > Source/WebKit2/Shared/API/c/WKDeclarationSpecifiers.h:49 > > +#ifndef __has_extension > > Hey nick, maybe would be cleaner to do some workaround like I did in https://bugs.webkit.org/show_bug.cgi?id=125187, instead of adding some code that does nothing. > What do you think? Ossy? Hmm, good. But that's still not enough to solve the build issues :)
(In reply to comment #13) > (In reply to comment #10) > > It seems you will need similar changes to http://trac.webkit.org/changeset/159988 > > Hmm, I'll take a look at it. thanks. Hey Ossy, you're right. We'll have to do similar things to that patch to get it working! So we'll need lots of changes to get the following classes working according to the new API::Client implementation: * WebBatteryProvider * WebNetworkInfoProvider * WebTextCheckerClient * WebVibrationProvider * efl/WebUIPopupMenuClient * gtk/WebFullScreenClientGtk * gtk/WebInspectorClientGtk * soup/WebSoupRequestManagerClient
(In reply to comment #14) > (In reply to comment #11) > > (From update of attachment 218354 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=218354&action=review > > > > > Source/WebKit2/Shared/API/c/WKDeclarationSpecifiers.h:49 > > > +#ifndef __has_extension > > > > Hey nick, maybe would be cleaner to do some workaround like I did in https://bugs.webkit.org/show_bug.cgi?id=125187, instead of adding some code that does nothing. > > What do you think? Ossy? > > Hmm, good. But that's still not enough to solve the build issues :) I know, it is just for that specific part of the code
Created attachment 218373 [details] Patch Moving to the new API::Client
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
FYI, EFL will be addressed in bug 125206.
Comment on attachment 218373 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218373&action=review > Source/WebKit2/UIProcess/API/gtk/WebKitFullscreenClient.cpp:48 > + WKViewSetFullScreenClientGtk(toAPI(WEBKIT_WEB_VIEW_BASE(webView)), reinterpret_cast<WKFullScreenClientGtkBase*>(&wkFullScreenClient)); I'm not sure if that's the right thing to do here :( @andersca, could you help us on this? > Source/WebKit2/UIProcess/API/gtk/WebKitTextChecker.cpp:110 > + WKTextCheckerSetClient(reinterpret_cast<WKTextCheckerClientBase*>(&wkTextCheckerClient)); ditto. > Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:332 > + WKInspectorSetInspectorClientGtk(toAPI(webInspector), reinterpret_cast<WKInspectorClientGtkBase*>(&wkInspectorClientGtk)); ditto.
Comment on attachment 218373 [details] Patch The build is still broken with this patch :(
(In reply to comment #19) > FYI, EFL will be addressed in bug 125206. What happened? This bug was to fix EFL and GTK too originally. Why do we have two separated bug report now? And there isn't any patch with r? in the other bug report. :(
This seems to have been made obsolete by the patch on the other bug. *** This bug has been marked as a duplicate of bug 125206 ***
Comment on attachment 218373 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218373&action=review >> Source/WebKit2/UIProcess/API/gtk/WebKitFullscreenClient.cpp:48 >> + WKViewSetFullScreenClientGtk(toAPI(WEBKIT_WEB_VIEW_BASE(webView)), reinterpret_cast<WKFullScreenClientGtkBase*>(&wkFullScreenClient)); > > I'm not sure if that's the right thing to do here :( > > @andersca, could you help us on this? This should just be &wkFullScreenClient.base. >> Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:332 >> + WKInspectorSetInspectorClientGtk(toAPI(webInspector), reinterpret_cast<WKInspectorClientGtkBase*>(&wkInspectorClientGtk)); > > ditto. Same thing here.
(In reply to comment #24) > (From update of attachment 218373 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=218373&action=review > > >> Source/WebKit2/UIProcess/API/gtk/WebKitFullscreenClient.cpp:48 > >> + WKViewSetFullScreenClientGtk(toAPI(WEBKIT_WEB_VIEW_BASE(webView)), reinterpret_cast<WKFullScreenClientGtkBase*>(&wkFullScreenClient)); > > > > I'm not sure if that's the right thing to do here :( > > > > @andersca, could you help us on this? > > This should just be &wkFullScreenClient.base. > > >> Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:332 > >> + WKInspectorSetInspectorClientGtk(toAPI(webInspector), reinterpret_cast<WKInspectorClientGtkBase*>(&wkInspectorClientGtk)); > > > > ditto. > > Same thing here. Ok, thanks. It will be fixed in bug 125231.