[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
https://bugs.webkit.org/show_bug.cgi?id=44759

Attachment 105627: Patch
https://bugs.webkit.org/attachment.cgi?id=105627&action=review

------- 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
camelCase.

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

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
names.

> 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)

Ditto.

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

ditto.


More information about the webkit-reviews mailing list