RESOLVED FIXED 135875
[EFL] Sets the persistent storage for cookies in MiniBrowser
https://bugs.webkit.org/show_bug.cgi?id=135875
Summary [EFL] Sets the persistent storage for cookies in MiniBrowser
Jinwoo Song
Reported 2014-08-12 22:54:04 PDT
SSIA.
Attachments
Patch (1.80 KB, patch)
2014-08-12 22:57 PDT, Jinwoo Song
no flags
Patch (2.13 KB, patch)
2014-08-13 00:34 PDT, Jinwoo Song
no flags
Patch (2.16 KB, patch)
2014-08-13 03:56 PDT, Jinwoo Song
no flags
Jinwoo Song
Comment 1 2014-08-12 22:57:44 PDT
Gyuyoung Kim
Comment 2 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); } }
Grzegorz Czajkowski
Comment 3 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 ?
Jinwoo Song
Comment 4 2014-08-13 00:34:45 PDT
Created attachment 236503 [details] Patch Applied Gyuyoung's comment.
Jinwoo Song
Comment 5 2014-08-13 03:56:03 PDT
Created attachment 236512 [details] Patch Applied Grzeg's comments.
Grzegorz Czajkowski
Comment 6 2014-08-13 04:07:29 PDT
Thanks for the changes. LGTM as well
WebKit Commit Bot
Comment 7 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>
WebKit Commit Bot
Comment 8 2014-08-13 05:11:08 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.