[Webkit-unassigned] [Bug 84130] [SOUP] Add a way to register custom uri schemes in WebKit2

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=84130





--- Comment #12 from Carlos Garcia Campos <cgarcia at igalia.com>  2012-04-19 23:20:22 PST ---
(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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.