[webkit-reviews] review denied: [Bug 44759] [EFL] Add custom network resource handler : [Attachment 105627] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 6 09:37:02 PDT 2011

Martin Robinson <mrobinson at webkit.org> has denied Flavio Ceolin
<flavio.ceolin at profusion.mobi>'s request for review:
Bug 44759: [EFL] Add custom network resource handler

Attachment 105627: Patch

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=105627&action=review

Looks good, but I have a few style nits. With another iteration and the
unoffocial r+, I think this patch is fine.

> Source/WebKit/efl/ewk/ewk_protocol_handler_soup.cpp:42
> +static unsigned custom_protocol_added_count = 0;

Please either use camelCase everywhere or this_thing. WebKit style is to use

> Source/WebKit/efl/ewk/ewk_protocol_handler_soup.cpp:57
> +									       

The indent looks off here.

> Source/WebKit/efl/ewk/ewk_protocol_handler_soup.cpp:71
> +    void* buf;

I'd prefer buffer to buf.

> Source/WebKit/efl/ewk/ewk_protocol_handler_soup.cpp:145
> +    guint protocols_size;
> +    SoupSession* session = WebCore::ResourceHandle::defaultSession();

It's odd that you use camelCase above bug something_else here.

> Source/WebKit/efl/ewk/ewk_view.cpp:94
> +    struct  {

Extra space after struct.

> Source/WebKit/efl/ewk/ewk_view.cpp:96
> +	   void* ctxt;
> +	   Ewk_View_Resource_Handler_Cb func;

"context" and "function" In Webkit we do not typically abbreviate variable

> Source/WebKit/efl/ewk/ewk_view.cpp:3618
> +Eina_Bool ewk_view_protocol_handler_set(Evas_Object* o, const char**
protocols, Ewk_View_Resource_Handler_Cb handler, void* ctxt)


> Source/WebKit/efl/ewk/ewk_view.h:2227
> +						void* ctxt);


More information about the webkit-reviews mailing list