Bug 194844 – Unify WebsiteDataStore::defaultDataStoreConfiguration across ports

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Don Olmstead]

Currently not all directories are set within the defaultdataStoreConnection. For example ServiceWorker support will crash since setServiceWorkerRegistrationDirectory is not set.

[Don Olmstead]

Created attachment 362456 [details]
Patch

[Don Olmstead]

Created attachment 362463 [details]
Patch

Not implementing device id hash salt.

[Fujii Hironori]

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()

[Don Olmstead]

Created attachment 362465 [details]
Patch

[Fujii Hironori]

Then, you should implement following.

WebsiteDataStore::defaultMediaCacheDirectory()
WebsiteDataStore::defaultJavaScriptConfigurationDirectory()

[Don Olmstead]

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.

[Don Olmstead]

Created attachment 363359 [details]
Patch

[Don Olmstead]

Created attachment 363360 [details]
Patch

[EWS Watchlist]

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

[youenn fablet]

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.

[Brent Fulgham]

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.

[Brent Fulgham]

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.

[Don Olmstead]

Created attachment 363363 [details]
Patch

Good catch on missing call to the Glib device id hash bits. Fixed that.

[WebKit Commit Bot]

Comment on attachment 363363 [details]
Patch

Clearing flags on attachment: 363363

Committed r242289: <https://trac.webkit.org/changeset/242289>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48522763>