Bug 135875 - [EFL] Sets the persistent storage for cookies in MiniBrowser
Summary: [EFL] Sets the persistent storage for cookies in MiniBrowser
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jinwoo Song
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-12 22:54 PDT by Jinwoo Song
Modified: 2014-08-13 05:11 PDT (History)
3 users (show)

See Also:


Attachments
Patch (1.80 KB, patch)
2014-08-12 22:57 PDT, Jinwoo Song
no flags Details | Formatted Diff | Diff
Patch (2.13 KB, patch)
2014-08-13 00:34 PDT, Jinwoo Song
no flags Details | Formatted Diff | Diff
Patch (2.16 KB, patch)
2014-08-13 03:56 PDT, Jinwoo Song
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jinwoo Song 2014-08-12 22:54:04 PDT
SSIA.
Comment 1 Jinwoo Song 2014-08-12 22:57:44 PDT
Created attachment 236499 [details]
Patch
Comment 2 Gyuyoung Kim 2014-08-13 00:07:01 PDT
Comment on attachment 236499 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=236499&action=review

> Tools/MiniBrowser/efl/main.c:2106
>          ewk_cookie_manager_accept_policy_set(ewk_context_cookie_manager_get(context), parse_cookies_policy(cookies_policy_string));

parse_cookies_policy() is called twice. So, how about changing as below ?

if (cookies_policy_string) {
    Ewk_Cookie_Accept_Policy cookie_policy = parse_cookies_policy(cookies_policy_string);
    ewk_cookie_manager_accept_policy_set(ewk_context_cookie_manager_get(context), cookie_policy);

    if (cookie_policy == EWK_COOKIE_ACCEPT_POLICY_ALWAYS) {
        char storageDirectory[] = "/tmp/ewebkit2_minibrowser_cookie";
        mkdir(storageDirectory, S_IRWXU);
        char textStorage[64];
        snprintf(textStorage, sizeof(textStorage), "%s/txt-cookie", storageDirectory);
        ewk_cookie_manager_persistent_storage_set(ewk_context_cookie_manager_get(context), textStorage, EWK_COOKIE_PERSISTENT_STORAGE_TEXT);
    }
}
Comment 3 Grzegorz Czajkowski 2014-08-13 00:25:25 PDT
Comment on attachment 236499 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=236499&action=review

> Tools/MiniBrowser/efl/main.c:2108
> +    if (parse_cookies_policy(cookies_policy_string) == EWK_COOKIE_ACCEPT_POLICY_ALWAYS) {

parse_cookies_policy() assumes non-null string. I also suggest merging this block with above one.

> Tools/MiniBrowser/efl/main.c:2109
> +        char storageDirectory[] = "/tmp/ewebkit2_minibrowser_cookie";

1) nit: const,
2) cookieStorageDirectory ?
3) Would be nice to end the string with '/' as this is directory.

> Tools/MiniBrowser/efl/main.c:2111
> +        char textStorage[64];

storageName ?
Comment 4 Jinwoo Song 2014-08-13 00:34:45 PDT
Created attachment 236503 [details]
Patch

Applied Gyuyoung's comment.
Comment 5 Jinwoo Song 2014-08-13 03:56:03 PDT
Created attachment 236512 [details]
Patch

Applied Grzeg's comments.
Comment 6 Grzegorz Czajkowski 2014-08-13 04:07:29 PDT
Thanks for the changes. LGTM as well
Comment 7 WebKit Commit Bot 2014-08-13 05:11:02 PDT
Comment on attachment 236512 [details]
Patch

Clearing flags on attachment: 236512

Committed r172519: <http://trac.webkit.org/changeset/172519>
Comment 8 WebKit Commit Bot 2014-08-13 05:11:08 PDT
All reviewed patches have been landed.  Closing bug.