In WebKit2 the soup session and all its features are in the WebProcess, so we can let apps add their own features to the session to implement things like we do in WebKit1. In order to allow applications to register their own uri schemes, we need to implement SoupRequest feature ni the web process and provide IPC mesassages to register schemes and handle uri scheme requests.
Created attachment 137488 [details] Patch This patch introduces new C API for ports using soup. It will be tested by GTK+ unit tests.
Attachment 137488 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'GNUmakefile.am', u'Source/WT..." exit_code: 1 Source/WTF/wtf/gobject/GRefPtr.h:211: The parameter name "ptr" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WTF/wtf/gobject/GRefPtr.h:212: The parameter name "ptr" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 32 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #2) > Attachment 137488 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'GNUmakefile.am', u'Source/WT..." exit_code: 1 > Source/WTF/wtf/gobject/GRefPtr.h:211: The parameter name "ptr" adds no information, so it should be removed. [readability/parameter_name] [5] > Source/WTF/wtf/gobject/GRefPtr.h:212: The parameter name "ptr" adds no information, so it should be removed. [readability/parameter_name] [5] > Total errors found: 2 in 32 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. All other methods in that file use the same pattern, so we should either fix the style in that file or add an exception to the style checker.
Comment on attachment 137488 [details] Patch Attachment 137488 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12418519
Created attachment 137501 [details] Updated patch to fix the build I forgot to git add the .messages.in files.
Attachment 137501 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'GNUmakefile.am', u'Source/WT..." exit_code: 1 Source/WTF/wtf/gobject/GRefPtr.h:211: The parameter name "ptr" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WTF/wtf/gobject/GRefPtr.h:212: The parameter name "ptr" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 137501 [details] Updated patch to fix the build Attachment 137501 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12423174
Created attachment 137504 [details] Another build fix There was another file missing in the patch, sorry.
Attachment 137504 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'GNUmakefile.am', u'Source/WT..." exit_code: 1 Source/WTF/wtf/gobject/GRefPtr.h:211: The parameter name "ptr" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WTF/wtf/gobject/GRefPtr.h:212: The parameter name "ptr" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #3) > All other methods in that file use the same pattern, so we should either fix the style in that file or add an exception to the style checker. It's probably best to write the code in a way that makes the style bot happy now and worry about fixing the old code later.
Comment on attachment 137504 [details] Another build fix View in context: https://bugs.webkit.org/attachment.cgi?id=137504&action=review This looks pretty good, but I think renaming the IPC message and avoiding __BUILDING_SOUP warrant a new version of the patch. >> Source/WTF/wtf/gobject/GRefPtr.h:211 >> +template <> GPtrArray* refGPtr(GPtrArray* ptr); > > The parameter name "ptr" adds no information, so it should be removed. [readability/parameter_name] [5] As I said above, we write new code so that it doesn't cause style errors. Likely the style bot was updated to catch these errors, so now we must avoid them. > Source/WebKit2/UIProcess/API/C/WKAPICast.h:366 > #if defined(BUILDING_GTK__) > #include "WKAPICastGtk.h" > #endif > + > +#if defined(BUILDING_SOUP__) > +#include "WKAPICastSoup.h" > +#endif > + In these cases we should be probably be using PLATFORM(SOUP/GTK) or WTF_PLATFORM_SOUP/GTK instead of duplicating the meaning of that macro with BUILDING_SOUP__. > Source/WebKit2/WebProcess/soup/WebSoupRequestManager.cpp:74 > + genericRequestClass->schemes = const_cast<const char**>(reinterpret_cast<char**>(m_schemes->pdata)); The interior cast can be a static_cast here instead of a reinterpret_cast, I believe. > Source/WebKit2/WebProcess/soup/WebSoupRequestManager.cpp:78 > +void WebSoupRequestManager::sendURIRequest(const CoreIPC::DataReference& requestData, const String& mimeType, uint64_t requestID) You might consider renaming this message. I was confused a while, because I wondered why the UIProcess would be sending a URI request. Then I realized it was just the result of naming the message "sendURIRequest." Perhaps something like handleCustomURIRequest would make sense on both sides of the IPC channel. > Tools/Scripts/webkitpy/style/checkers/cpp.py:3112 > + and not modified_identifier.startswith('webkit_soup') Nice.
(In reply to comment #11) > (From update of attachment 137504 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=137504&action=review > > This looks pretty good, but I think renaming the IPC message and avoiding __BUILDING_SOUP warrant a new version of the patch. Thanks for reviewing! > >> Source/WTF/wtf/gobject/GRefPtr.h:211 > >> +template <> GPtrArray* refGPtr(GPtrArray* ptr); > > > > The parameter name "ptr" adds no information, so it should be removed. [readability/parameter_name] [5] > > As I said above, we write new code so that it doesn't cause style errors. Likely the style bot was updated to catch these errors, so now we must avoid them. Fair enough > > Source/WebKit2/UIProcess/API/C/WKAPICast.h:366 > > #if defined(BUILDING_GTK__) > > #include "WKAPICastGtk.h" > > #endif > > + > > +#if defined(BUILDING_SOUP__) > > +#include "WKAPICastSoup.h" > > +#endif > > + > > In these cases we should be probably be using PLATFORM(SOUP/GTK) or WTF_PLATFORM_SOUP/GTK instead of duplicating the meaning of that macro with BUILDING_SOUP__. This is a public header, that's why it uses BUILDING_GTK__ also instead of PLATFORM(GTK) > > Source/WebKit2/WebProcess/soup/WebSoupRequestManager.cpp:74 > > + genericRequestClass->schemes = const_cast<const char**>(reinterpret_cast<char**>(m_schemes->pdata)); > > The interior cast can be a static_cast here instead of a reinterpret_cast, I believe. I think I tried that, but I'll try again to be sure. > > Source/WebKit2/WebProcess/soup/WebSoupRequestManager.cpp:78 > > +void WebSoupRequestManager::sendURIRequest(const CoreIPC::DataReference& requestData, const String& mimeType, uint64_t requestID) > > You might consider renaming this message. I was confused a while, because I wondered why the UIProcess would be sending a URI request. Then I realized it was just the result of naming the message "sendURIRequest." Perhaps something like handleCustomURIRequest would make sense on both sides of the IPC channel. Ok, it was motivated by the SoupRequest interface, but I agree it can be confusing.
(In reply to comment #12) > > In these cases we should be probably be using PLATFORM(SOUP/GTK) or WTF_PLATFORM_SOUP/GTK instead of duplicating the meaning of that macro with BUILDING_SOUP__. > > This is a public header, that's why it uses BUILDING_GTK__ also instead of PLATFORM(GTK) Is this truly the case? I notice that the header is C++ (it uses the class keyword) and also that it uses other WTF macros such as ENABLE(INSPECTOR).
(In reply to comment #13) > (In reply to comment #12) > > > > In these cases we should be probably be using PLATFORM(SOUP/GTK) or WTF_PLATFORM_SOUP/GTK instead of duplicating the meaning of that macro with BUILDING_SOUP__. > > > > This is a public header, that's why it uses BUILDING_GTK__ also instead of PLATFORM(GTK) > > Is this truly the case? I notice that the header is C++ (it uses the class keyword) and also that it uses other WTF macros such as ENABLE(INSPECTOR). Oh, I'll check, in that case it's much better to simply use USE(SOUP) and avoid adding new macros
(In reply to comment #12) > > > Source/WebKit2/UIProcess/API/C/WKAPICast.h:366 > > > #if defined(BUILDING_GTK__) > > > #include "WKAPICastGtk.h" > > > #endif > > > + > > > +#if defined(BUILDING_SOUP__) > > > +#include "WKAPICastSoup.h" > > > +#endif > > > + > > > > In these cases we should be probably be using PLATFORM(SOUP/GTK) or WTF_PLATFORM_SOUP/GTK instead of duplicating the meaning of that macro with BUILDING_SOUP__. > > This is a public header, that's why it uses BUILDING_GTK__ also instead of PLATFORM(GTK) Ok, switched to use USE(SOUP) in WkAPICast.h, but BUILDING_SOUP is still needed for WKBase.h > > > Source/WebKit2/WebProcess/soup/WebSoupRequestManager.cpp:74 > > > + genericRequestClass->schemes = const_cast<const char**>(reinterpret_cast<char**>(m_schemes->pdata)); > > > > The interior cast can be a static_cast here instead of a reinterpret_cast, I believe. > > I think I tried that, but I'll try again to be sure. error: invalid static_cast from type 'void**' to type 'char**' Only Chuck Norris knows which C++ style cast to use without trying them all until one works :-D
Created attachment 138059 [details] Updated patch according to review comments
Committed r115411: <http://trac.webkit.org/changeset/115411>