[Webkit-unassigned] [Bug 104370] [GTK] Cookies' storage path and policy are not set when the WebProcess is killed and relaunched

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 7 08:56:39 PST 2012


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


Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #178222|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #5 from Carlos Garcia Campos <cgarcia at igalia.com>  2012-12-07 08:59:05 PST ---
(From update of attachment 178222)
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.

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



More information about the webkit-unassigned mailing list