Summary: | [SOUP] Soup disk cache should respect the diskCacheDirectory from the process initial parameters | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | andersca, ap, benjamin, cdumez, danw, gyuyoung.kim, rakuco, svillar, tmpsantos, webkit.review.bot, xan.lopez | ||||||
Priority: | P2 | Keywords: | Gtk, Soup | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 111848 | ||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2013-03-08 04:28:25 PST
Created attachment 192199 [details]
Patch
Adding WebKit2 owners. Comment on attachment 192199 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192199&action=review Looks good, thanks. > Source/WebKit2/WebProcess/soup/WebProcessSoup.cpp:167 > + ASSERT(!parameters.diskCacheDirectory.isEmpty() && !parameters.diskCacheDirectory.isNull()); The isNull() check is superfluous here as it is already checked inside isEmpty(). Comment on attachment 192199 [details]
Patch
Looks good to me.
Created attachment 192202 [details]
Updated patch
Removed the redundant isNull from assert and added a check before flushing the cache for the case when the web process finishes too early before the platformInitializeWebProcess is called.
Comment on attachment 192202 [details]
Updated patch
LGTM, thanks!
Comment on attachment 192202 [details]
Updated patch
There are two ways to set this: WebContext::platformDefaultDiskCacheDirectory and WKContextSetDiskCacheDirectory.
From a quick look, it looks like we should get rid of platformDefaultDiskCacheDirectory and unify something around WKContextSetDiskCacheDirectory. It is silly to have two path. Any opinion?
(In reply to comment #7) > (From update of attachment 192202 [details]) > There are two ways to set this: WebContext::platformDefaultDiskCacheDirectory and WKContextSetDiskCacheDirectory. > > From a quick look, it looks like we should get rid of platformDefaultDiskCacheDirectory and unify something around WKContextSetDiskCacheDirectory. It is silly to have two path. Any opinion? For me they are not two ways, platformDefaultDiskCacheDirectory() defines the default disk cache directory for the platform. WKContextSetDiskCacheDirectory overrides the default directory. The good thing of having a default disk cache directory defined in the WebContext is that you don't need to explicitly set a disk cache from the API or from apps, unless you want to override the default. Note that platformDefaultDiskCacheDirectory() is private, so it's not supposed to be a way for the API layer to set a disck cache directory, but to define an internal default. All other directories (database, local storage, cookie storage, etc.) use the same approach. ping owners Committed r148886: <http://trac.webkit.org/changeset/148886> |