[Webkit-unassigned] [Bug 168230] [GTK] Update cookie manager API to properly work with ephemeral sessions

[bugzilla-daemon at webkit.org]

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

--- Comment #3 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 301345
  --> https://bugs.webkit.org/attachment.cgi?id=301345
Patch

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

>> Source/WebCore/platform/network/soup/CookieJarSoup.cpp:217
>> +        deleteAllCookies(session);
> 
> Are you planning to fix this soon? I don't think we should proceed with these API changes if the API does not work properly. At least add a runtime warning using g_warning().

(Sad OK if you add the g_warning().)

> Source/WebKit2/UIProcess/API/gtk/WebKitCookieManager.cpp:115
> +    if (manager->priv->dataManager) {
> +        g_object_remove_weak_pointer(G_OBJECT(manager->priv->dataManager), reinterpret_cast<void**>(&manager->priv->dataManager));
> +
> +        auto sessionID = webkitWebsiteDataManagerGetDataStore(manager->priv->dataManager).websiteDataStore().sessionID();
> +        for (auto* processPool : webkitWebsiteDataManagerGetProcessPools(manager->priv->dataManager))
> +            processPool->supplement<WebCookieManagerProxy>()->stopObservingCookieChanges(sessionID);
> +
> +        manager->priv->dataManager = nullptr;
> +    }

Actually, I wonder if we should do this in ~_WebKitCookieManagerPrivate() instead of dispose. Because the object should technically be left in valid state after dispose, but all of the functions are going to hit criticals if priv->dataManager is null. Since we're not keeping a strong reference to it, it can't be involved in a reference cycle, so it should be safer to do in the destructor instead of dispose.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170214/c99fd528/attachment.html>