[webkit-reviews] review denied: [Bug 84130] [SOUP] Add a way to register custom uri schemes in WebKit2 : [Attachment 137504] Another build fix
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Apr 19 15:30:16 PDT 2012
Martin Robinson <mrobinson at webkit.org> has denied Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 84130: [SOUP] Add a way to register custom uri schemes in WebKit2
https://bugs.webkit.org/show_bug.cgi?id=84130
Attachment 137504: Another build fix
https://bugs.webkit.org/attachment.cgi?id=137504&action=review
------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
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.
More information about the webkit-reviews
mailing list