Bug 111845 – [SOUP] Soup disk cache should respect the diskCacheDirectory from the process initial parameters

• Component: WebKit2
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

We currently set the disk cache directory in the web process unconditionally in both GTK and EFL.

[Carlos Garcia Campos]

Created attachment 192199 [details]
Patch

[Carlos Garcia Campos]

Adding WebKit2 owners.

[Chris Dumez]

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

[Xan Lopez]

Comment on attachment 192199 [details]
Patch

Looks good to me.

[Carlos Garcia Campos]

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.

[Thiago Marcos P. Santos]

Comment on attachment 192202 [details]
Updated patch

LGTM, thanks!

[Benjamin Poulain]

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?

[Carlos Garcia Campos]

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

[Carlos Garcia Campos]

ping owners

[Carlos Garcia Campos]

Committed r148886: <http://trac.webkit.org/changeset/148886>