Move cookiePersistentStoragePath and cookiePersistentStorageType from NetworkProcess to NetworkSession to reduce globals. This removes two of the six soup-specific variables in NetworkProcessCreationParameters. More to come....
Created attachment 360687 [details] Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment on attachment 360687 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=360687&action=review > Source/WebKit/UIProcess/API/glib/WebKitCookieManager.cpp:169 > + auto sessionID = webkitWebsiteDataManagerGetDataStore(manager->priv->dataManager).websiteDataStore().sessionID(); We should not allow to set a persistent cookie storage in ephemeral sessions. Return early here if sessionID is ephemeral. That's the reason why we used the default session ID unconditionally, for now in soup based ports all sessions but the default one are ephemeral. > Source/WebKit/UIProcess/soup/WebCookieManagerProxySoup.cpp:44 > + auto pair = m_cookiePersistentStorageMap.get(sessionID); > + storagePath = pair.first; > + storageType = static_cast<uint32_t>(pair.second); You should check the return value in case the sessionID is not present in the map. Or add an assert if it should always be.
(In reply to Carlos Garcia Campos from comment #3) > > Source/WebKit/UIProcess/API/glib/WebKitCookieManager.cpp:169 > > + auto sessionID = webkitWebsiteDataManagerGetDataStore(manager->priv->dataManager).websiteDataStore().sessionID(); > > We should not allow to set a persistent cookie storage in ephemeral > sessions. Return early here if sessionID is ephemeral. That's the reason why > we used the default session ID unconditionally, for now in soup based ports > all sessions but the default one are ephemeral. OK. > > Source/WebKit/UIProcess/soup/WebCookieManagerProxySoup.cpp:44 > > + auto pair = m_cookiePersistentStorageMap.get(sessionID); > > + storagePath = pair.first; > > + storageType = static_cast<uint32_t>(pair.second); > > You should check the return value in case the sessionID is not present in > the map. Or add an assert if it should always be. If it's not present then an empty value gets used (I think empty string and the first storage type policy) so the end result would be the same, right?
(In reply to Michael Catanzaro from comment #4) > If it's not present then an empty value gets used (I think empty string and > the first storage type policy) so the end result would be the same, right? Verified it returns empty string for path, and 0 (SoupCookiePersistentStorageText) for type.
Committed r240785: <https://trac.webkit.org/changeset/240785>