WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
194844
Unify WebsiteDataStore::defaultDataStoreConfiguration across ports
https://bugs.webkit.org/show_bug.cgi?id=194844
Summary
Unify WebsiteDataStore::defaultDataStoreConfiguration across ports
Don Olmstead
Reported
2019-02-19 17:08:33 PST
Currently not all directories are set within the defaultdataStoreConnection. For example ServiceWorker support will crash since setServiceWorkerRegistrationDirectory is not set.
Attachments
Patch
(2.92 KB, patch)
2019-02-19 17:13 PST
,
Don Olmstead
no flags
Details
Formatted Diff
Diff
Patch
(1.94 KB, patch)
2019-02-19 17:47 PST
,
Don Olmstead
no flags
Details
Formatted Diff
Diff
Patch
(2.14 KB, patch)
2019-02-19 18:14 PST
,
Don Olmstead
fujii.hironori
: review-
Details
Formatted Diff
Diff
Patch
(7.16 KB, patch)
2019-03-01 12:46 PST
,
Don Olmstead
no flags
Details
Formatted Diff
Diff
Patch
(7.32 KB, patch)
2019-03-01 12:46 PST
,
Don Olmstead
youennf
: review+
Details
Formatted Diff
Diff
Patch
(8.80 KB, patch)
2019-03-01 13:10 PST
,
Don Olmstead
no flags
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Don Olmstead
Comment 1
2019-02-19 17:13:56 PST
Created
attachment 362456
[details]
Patch
Don Olmstead
Comment 2
2019-02-19 17:47:06 PST
Created
attachment 362463
[details]
Patch Not implementing device id hash salt.
Fujii Hironori
Comment 3
2019-02-19 18:00:01 PST
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
Comment 4
2019-02-19 18:14:35 PST
Created
attachment 362465
[details]
Patch
Fujii Hironori
Comment 5
2019-02-19 18:17:45 PST
Then, you should implement following. WebsiteDataStore::defaultMediaCacheDirectory() WebsiteDataStore::defaultJavaScriptConfigurationDirectory()
Don Olmstead
Comment 6
2019-03-01 12:40:34 PST
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
Comment 7
2019-03-01 12:46:10 PST
Created
attachment 363359
[details]
Patch
Don Olmstead
Comment 8
2019-03-01 12:46:59 PST
Created
attachment 363360
[details]
Patch
EWS Watchlist
Comment 9
2019-03-01 12:52:09 PST
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 10
2019-03-01 13:00:06 PST
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 11
2019-03-01 13:00:55 PST
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 12
2019-03-01 13:04:23 PST
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
Comment 13
2019-03-01 13:10:27 PST
Created
attachment 363363
[details]
Patch Good catch on missing call to the Glib device id hash bits. Fixed that.
WebKit Commit Bot
Comment 14
2019-03-01 14:18:27 PST
Comment on
attachment 363363
[details]
Patch Clearing flags on attachment: 363363 Committed
r242289
: <
https://trac.webkit.org/changeset/242289
>
WebKit Commit Bot
Comment 15
2019-03-01 14:18:29 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 16
2019-03-01 14:19:24 PST
<
rdar://problem/48522763
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug