Bug 104370 - [GTK] Cookies' storage path and policy are not set when the WebProcess is killed and relaunched
: [GTK] Cookies' storage path and policy are not set when the WebProcess is kil...
Status: RESOLVED FIXED
: WebKit
WebKit2
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2012-12-07 07:36 PST by
Modified: 2012-12-12 01:20 PST (History)


Attachments
Patch (12.14 KB, patch)
2012-12-07 08:22 PST, Joaquim Rocha
no flags Review Patch | Details | Formatted Diff | Diff
Patch (13.00 KB, patch)
2012-12-10 03:49 PST, Joaquim Rocha
no flags Review Patch | Details | Formatted Diff | Diff
Patch (13.03 KB, patch)
2012-12-10 04:12 PST, Joaquim Rocha
no flags Review Patch | Details | Formatted Diff | Diff
Patch (12.83 KB, patch)
2012-12-10 05:45 PST, Joaquim Rocha
no flags Review Patch | Details | Formatted Diff | Diff
Patch (12.94 KB, patch)
2012-12-11 08:38 PST, Joaquim Rocha
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-12-07 07:36:33 PST
The cookies' storage path and policy are not set when the WebProcess is killed and relaunched on Epiphany (and possibly other ports using Soup).

Steps to reproduce the storage path one on Epiphany:
1) Open Epiphany log into some website;
2) Open the Personal Data dialog and check out this website's cookie and eventually others'.
3) Kill the webprocess, reload that website and check out the cookies list in Personal Data.

Expected:
The user is still logged in in the website and the same list of cookies is observed before.

Outcome:
The user is not logged in and no cookies are found in that list.

I will upload a patch to fix this soon.
------- Comment #1 From 2012-12-07 07:43:32 PST -------
Forgot to add the steps to reproduce the policy issue:

1) Open Epiphany, go to Personal Data and clear all cookies.
2) Go to Preferences > Privacy and set it to never accept cookies.
3) Access website that has cookies e.g. google.com, open Personal Data again and verify there is no Google cookie
4) Kill the WebProcess and reload google.com
5) Open Personal Data and verify that there's now a Google cookie...
------- Comment #2 From 2012-12-07 07:57:13 PST -------
Make [gtk] tag capital case.
------- Comment #3 From 2012-12-07 08:22:56 PST -------
Created an attachment (id=178222) [details]
Patch
------- Comment #4 From 2012-12-07 08:24:04 PST -------
(From update of attachment 178222 [details])
Set cq?
------- Comment #5 From 2012-12-07 08:59:05 PST -------
(From update of attachment 178222 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=178222&action=review

I've noticed that WIN has WebContext::setInitialHTTPCookieAcceptPolicy() maybe we could reuse that and save the values in the context instead of the cookies proxy.

> Source/WebKit2/Shared/WebProcessCreationParameters.cpp:83
> +    encoder << cookieAcceptPolicy;

I think you should use encoder.encodeEnum() for enums.

> Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp:140
> +#if USE(SOUP)
> +    setCookieAcceptPolicy(policy);
> +#endif

You can simply do m_cookieAcceptPolicy = policy;

> Source/WebKit2/UIProcess/WebCookieManagerProxy.h:75
> +    void setCookieAcceptPolicy(const HTTPCookieAcceptPolicy&);

This method is confusing, because there's setHTTPCookieAcceptPolicy and setCookieAcceptPolicy, but it's not needed it at all, since we don't expect it to be used by others.

> Source/WebKit2/UIProcess/WebCookieManagerProxy.h:101
> +    HTTPCookieAcceptPolicy m_cookieAcceptPolicy;
> +    pair<String, uint32_t> m_cookiePersistentStorage;

These should be initialized in the constructor.

> Source/WebKit2/UIProcess/efl/WebContextEfl.cpp:49
> +    const pair<String, uint32_t>& storage = m_cookieManagerProxy->cookiePersistentStorage();
> +    parameters.cookiePersistentStoragePath = storage.first;
> +    parameters.cookiePersistentStorageType = storage.second;

Maybe this could be simpler if we had a method returning two parameters instead of pair? something like:

m_cookieManagerProxy->getCookiePersistentStorage(parameters.cookiePersistentStoragePath, parameters.cookiePersistentStorageType);

> Source/WebKit2/UIProcess/gtk/WebContextGtk.cpp:93
> +    const pair<String, uint32_t>& storage = m_cookieManagerProxy->cookiePersistentStorage();
> +    parameters.cookiePersistentStoragePath = storage.first;
> +    parameters.cookiePersistentStorageType = storage.second;

Ditto.

> Source/WebKit2/UIProcess/soup/WebCookieManagerProxySoup.cpp:54
> +const pair<String, uint32_t>& WebCookieManagerProxy::cookiePersistentStorage() const
> +{
> +    return m_cookiePersistentStorage;
> +}
> +
> +void WebCookieManagerProxy::setCookieAcceptPolicy(const HTTPCookieAcceptPolicy& policy)
> +{
> +    m_cookieAcceptPolicy = policy;
> +}
> +
> +const HTTPCookieAcceptPolicy& WebCookieManagerProxy::cookieAcceptPolicy() const
> +{
> +    return m_cookieAcceptPolicy;

These could be in the header.

> Source/WebKit2/WebProcess/soup/WebProcessSoup.cpp:173
> +    WebCookieManager::shared().setCookiePersistentStorage(parameters.cookiePersistentStoragePath, parameters.cookiePersistentStorageType);

You should check cookiePersistentStoragePath is not empty before calling setCookiePersistentStorage(), as it will happen the first time the web process is launched.
------- Comment #6 From 2012-12-10 03:49:20 PST -------
Created an attachment (id=178507) [details]
Patch
------- Comment #7 From 2012-12-10 04:01:05 PST -------
(From update of attachment 178507 [details])
Attachment 178507 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15227575
------- Comment #8 From 2012-12-10 04:01:50 PST -------
(From update of attachment 178507 [details])
Attachment 178507 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15242232
------- Comment #9 From 2012-12-10 04:09:31 PST -------
(From update of attachment 178507 [details])
Attachment 178507 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15222679
------- Comment #10 From 2012-12-10 04:12:23 PST -------
Created an attachment (id=178513) [details]
Patch
------- Comment #11 From 2012-12-10 05:05:36 PST -------
(From update of attachment 178513 [details])
Attachment 178513 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15239436
------- Comment #12 From 2012-12-10 05:45:09 PST -------
Created an attachment (id=178524) [details]
Patch
------- Comment #13 From 2012-12-11 08:38:18 PST -------
Created an attachment (id=178813) [details]
Patch
------- Comment #14 From 2012-12-12 01:03:14 PST -------
(From update of attachment 178813 [details])
Looks great, thanks!
------- Comment #15 From 2012-12-12 01:20:45 PST -------
(From update of attachment 178813 [details])
Clearing flags on attachment: 178813

Committed r137432: <http://trac.webkit.org/changeset/137432>
------- Comment #16 From 2012-12-12 01:20:50 PST -------
All reviewed patches have been landed.  Closing bug.