Bug 117143

Summary: Make WTR use its own temporary directory for application cache
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: New BugsAssignee: Csaba Osztrogonác <ossy>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, andersca, ap, cdumez, cmarcelo, commit-queue, gyuyoung.kim, menard, ossy, rakuco, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 82061    
Attachments:
Description Flags
first patch for EWS testing
none
suggested Mac cleanup
none
proposed patch
none
proposed patch none

Description Csaba Osztrogonác 2013-06-03 05:59:35 PDT
Similar bug report for disk cache directory, local storage directory 
and database directory: https://bugs.webkit.org/show_bug.cgi?id=89666

We need it for parallel testing and for clobbering the cache after the run 
for the next run. It would make the bots more robust and faster a little bit.
Comment 1 Csaba Osztrogonác 2013-06-03 06:07:54 PDT
Created attachment 203579 [details]
first patch for EWS testing
Comment 2 Alexey Proskuryakov 2013-06-03 09:59:42 PDT
When adding this code, please remove WebKitLocalCacheDefaultsKey from WebContextMac.mm. 

This preference key is used by DumpRenderTree for the same purpose, but it's essentially unused in WebKit2, and keeping this code in addition to what you are adding would create substantial confusion.
Comment 3 Alexey Proskuryakov 2013-06-03 10:00:34 PDT
Perhaps this patch will let us re-enable testing appcache on mac-wk2.
Comment 4 Csaba Osztrogonác 2013-06-04 04:17:19 PDT
Comment on attachment 203579 [details]
first patch for EWS testing

View in context: https://bugs.webkit.org/attachment.cgi?id=203579&action=review

> Source/WebKit2/UIProcess/WebContext.cpp:1025
> +String WebContext::applicationCacheDirectory() const
> +{
> +    if (!m_overrideApplicationCacheDirectory.isEmpty())
> +        return m_overrideApplicationCacheDirectory;
> +
> +    return platformDefaultApplicationCacheDirectory();
> +}

If m_overrideApplicationCacheDirectory.isEmpty() is true,
the return value is platformDefaultApplicationCacheDirectory()
which is in Source/WebKit2/UIProcess/mac/WebContextMac.mm,
the old WebContext::applicationCacheDirectory().

I don't understand what should be its default return 
value without any WebKitLocalCacheDefaultsKey ?
Comment 5 Alexey Proskuryakov 2013-06-04 10:12:35 PDT
Created attachment 203708 [details]
suggested Mac cleanup

It will just take the path from confstr(_CS_DARWIN_USER_CACHE_DIR), and append application's bundle identifier, as it does today. Since WebKitTestRunner doesn't set WebKitLocalCache defaults key, trying it is just dead code.

Attached is the change I'm suggesting.
Comment 6 Csaba Osztrogonác 2013-06-05 03:38:51 PDT
Created attachment 203789 [details]
proposed patch

Thanks for the Mac cleanup, I integrated it to this patch.
Comment 7 Csaba Osztrogonác 2013-06-05 03:41:25 PDT
Created attachment 203791 [details]
proposed patch

typo fix
Comment 8 Alexey Proskuryakov 2013-06-05 10:04:05 PDT
Comment on attachment 203791 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=203791&action=review

> Source/WebKit2/UIProcess/API/C/WKContextPrivate.h:62
>  // FIXME: These functions are only effective if called before the Web process is launched. But
>  // we should really change these settings to be on WebPreferences and changeable at runtime.

I would say that these functions are for WebKitTestRunner only, and would also consider going WebKit1 way, and just providing a single override for all cache files.

Perhaps there was a reason for fine-grained controls that I'm not aware of though.
Comment 9 Csaba Osztrogonác 2013-06-11 01:40:09 PDT
Comment on attachment 203791 [details]
proposed patch

Clearing flags on attachment: 203791

Committed r151429: <http://trac.webkit.org/changeset/151429>
Comment 10 Csaba Osztrogonác 2013-06-11 01:40:17 PDT
All reviewed patches have been landed.  Closing bug.