Summary: | Unify WebsiteDataStore::defaultDataStoreConfiguration across ports | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Don Olmstead <don.olmstead> | ||||||||||||||
Component: | WebKit Misc. | Assignee: | Don Olmstead <don.olmstead> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | berto, bfulgham, cgarcia, commit-queue, ews-watchlist, gustavo, Hironori.Fujii, mcatanzaro, webkit-bug-importer, youennf | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Don Olmstead
2019-02-19 17:08:33 PST
Created attachment 362456 [details]
Patch
Created attachment 362463 [details]
Patch
Not implementing device id hash salt.
Do you want to add following, too? configuration->setCacheStorageDirectory(defaultCacheStorageDirectory()); configuration->setMediaCacheDirectory(defaultMediaCacheDirectory()); configuration->setJavaScriptConfigurationDirectory(defaultJavaScriptConfigurationDirectory()); Do you want to implement following, too? WebsiteDataStore::defaultMediaCacheDirectory() WebsiteDataStore::defaultJavaScriptConfigurationDirectory() Created attachment 362465 [details]
Patch
Then, you should implement following. WebsiteDataStore::defaultMediaCacheDirectory() WebsiteDataStore::defaultJavaScriptConfigurationDirectory() Changing up the focus of this bug. There are very very slight variations in each port's implementation of defaultDataStoreConfiguration. Honestly this seems to be just bit rot and theres no real reason to have things it in a platform specific source file. Created attachment 363359 [details]
Patch
Created attachment 363360 [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 363360 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363360&action=review > Source/WebKit/UIProcess/API/glib/APIWebsiteDataStoreGLib.cpp:-193 > - configuration->setDeviceIdHashSaltsStorageDirectory(defaultDeviceIdHashSaltsStorageDirectory()); This one might be good to add as well above. Comment on attachment 363360 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363360&action=review > Source/WebKit/ChangeLog:18 > + (API::WebsiteDataStore::defaultDataStoreConfiguration): Deleted. Ooo! Nice. >> Source/WebKit/UIProcess/API/glib/APIWebsiteDataStoreGLib.cpp:-193 >> - configuration->setDeviceIdHashSaltsStorageDirectory(defaultDeviceIdHashSaltsStorageDirectory()); > > This one might be good to add as well above. Should we have a "setPlatformConfiguration<Mumble>" that adds any platform-specific things? Maybe only glib has this need at the moment. Comment on attachment 363360 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363360&action=review >>> Source/WebKit/UIProcess/API/glib/APIWebsiteDataStoreGLib.cpp:-193 >>> - configuration->setDeviceIdHashSaltsStorageDirectory(defaultDeviceIdHashSaltsStorageDirectory()); >> >> This one might be good to add as well above. > > Should we have a "setPlatformConfiguration<Mumble>" that adds any platform-specific things? > > Maybe only glib has this need at the moment. I take it back -- it's in common code, so let's just add it in the unified location. Created attachment 363363 [details]
Patch
Good catch on missing call to the Glib device id hash bits. Fixed that.
Comment on attachment 363363 [details] Patch Clearing flags on attachment: 363363 Committed r242289: <https://trac.webkit.org/changeset/242289> All reviewed patches have been landed. Closing bug. |