[Webkit-unassigned] [Bug 54752] [EFL] Soup authentication feature implementation

[bugzilla-daemon at webkit.org]

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


Leandro Pereira <leandro at profusion.mobi> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #95871|review?                     |review-
               Flag|                            |




--- Comment #12 from Leandro Pereira <leandro at profusion.mobi>  2011-06-03 09:40:11 PST ---
(From update of attachment 95871)
View in context: https://bugs.webkit.org/attachment.cgi?id=95871&action=review

I couldn't figure out how this would work. Could you please provide a simple explanation of the expected flow?

> Source/WebKit/efl/CMakeListsEfl.txt:106
> +  LIST(APPEND WebKit_SOURCES
> +    efl/ewk/ewk_auth.cpp
> +    efl/ewk/ewk_soup_auth.cpp
> +  )

I'd prefer ewk_auth_soup.cpp, so it is consistent with EFL naming scheme.

> Source/WebKit/efl/ewk/ewk_auth.cpp:27
> +ewk_auth_show_dialog_callback g_auth_show_dialog_callback = 0;

Structures/typedefs in EFL are named Using_A_Combination_Of_Camel_Case_And_Underscores. Also, this variable should be static and there's no need to prepend the name with "g_" to denote it is a global variable.

> Source/WebKit/efl/ewk/ewk_auth.cpp:42
> +void ewk_auth_credentials_set(char* username, char* password, void* data)
> +{
> +#if USE(SOUP)
> +    ewk_soup_auth_credentials_set(username, password, data);

ewk_auth_soup_credentials_set() would be better (makes it easier to grep by related functions on the source code).

> Source/WebKit/efl/ewk/ewk_auth.h:35
> +typedef void (*ewk_auth_show_dialog_callback)(const char* msg, void* data);

See remark about typedef naming above.

> Source/WebKit/efl/ewk/ewk_soup_auth.cpp:45
> +typedef struct _WebKitAuthData {
> +    SoupMessage* msg;
> +    SoupAuth* auth;
> +    SoupSession* session;
> +} WebKitAuthData;

See remarks about structure naming above. Also, prefix structures with Ewk rather than WebKit.

> Source/WebKit/efl/ewk/ewk_soup_auth.cpp:94
> +    uri = soup_message_get_uri(msg);

uri isn't used here.

> Source/WebKit/efl/ewk/ewk_soup_auth.cpp:105
> +    if (g_auth_show_dialog_callback)
> +        g_auth_show_dialog_callback(realm, auth_data);

First parameter of this function is called "msg", yet you just pass realm. How about passing instead 'url' and 'realm', and having consistent naming on the function prototype?

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