Bug 104782

Summary: [EFL][WK2] Implement ewk_context_resource_cache_clear
Product: WebKit Reporter: Joone Hur <joone>
Component: WebKit EFLAssignee: Joone Hur <joone>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, d-r, gyuyoung.kim, kenneth, laszlo.gombos, lucas.de.marchi, rakuco, svillar, tmpsantos, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 104665    
Attachments:
Description Flags
Patch
none
Updated Patch none

Description Joone Hur 2012-12-12 01:57:39 PST
A lot of cache files are created while running the API unit tests, which makes performance slow.
https://bugs.webkit.org/show_bug.cgi?id=104665

So, we need to implement ewk_context_cache_clear in order to clear the caches before running the tests.
Comment 1 Kenneth Rohde Christiansen 2012-12-12 02:10:12 PST
ewk_context_cache_clear is a very vague name? What cache are we talking about? page cache, http cache, etc; there are lots of caches around
Comment 2 Sergio Villar Senin 2012-12-12 02:16:19 PST
(In reply to comment #1)
> ewk_context_cache_clear is a very vague name? What cache are we talking about? page cache, http cache, etc; there are lots of caches around

In this particular case we're talking about the HTTP cache.
Comment 3 Joone Hur 2012-12-12 02:39:24 PST
(In reply to comment #2)
> (In reply to comment #1)
> > ewk_context_cache_clear is a very vague name? What cache are we talking about? page cache, http cache, etc; there are lots of caches around
> 
> In this particular case we're talking about the HTTP cache.

Yes, this API will call the following method:

void WebProcess::platformClearResourceCaches(ResourceCachesToClear cachesToClear)
{
    if (cachesToClear == InMemoryResourceCachesOnly)
        return;

    SoupSession* session = WebCore::ResourceHandle::defaultSession();
    soup_cache_clear(SOUP_CACHE(soup_session_get_feature(session, SOUP_TYPE_CACHE)));
}

The API name is very vague, but WebKitGtk+ also has the same API.
void webkit_web_context_clear_cache(WebKitWebContext* context)

Anyway, we can use ewk_context_http_cache_clear, which is more explicit.
Comment 4 Joone Hur 2012-12-23 11:48:24 PST
Created attachment 180627 [details]
Patch
Comment 5 Kenneth Rohde Christiansen 2012-12-24 01:53:25 PST
Comment on attachment 180627 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:317
> + * Clears all resources currently cached in memory and local storage for @a context.
> + *
> + * @param context context object to clear all resource caches.

I think this needs more explanation.

Why clear, not purge or so... clear doesnt mean remove, but could be just clearing (zeroing out or similar).

Clears all resources cached in memory such as local storage, HTTP cache,  ...
Comment 6 Joone Hur 2012-12-24 12:08:34 PST
(In reply to comment #5)
> (From update of attachment 180627 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=180627&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_context.h:317
> > + * Clears all resources currently cached in memory and local storage for @a context.
> > + *
> > + * @param context context object to clear all resource caches.
> 
> I think this needs more explanation.
> 
> Why clear, not purge or so... clear doesnt mean remove, but could be just clearing (zeroing out or similar).
> 
> Clears all resources cached in memory such as local storage, HTTP cache,  ...

The reason why I use "Clear" is that this API calls WebProcess::clearResourceCaches in the end.

All resources mean images, CSS, JavaScript, XSL, and fonts, which are cached in memory and they are also cached as HTTP caches in local storage.
http://trac.webkit.org/browser/trunk/Source/WebCore/loader/cache/MemoryCache.cpp#L693

How about this API description?

Clears HTTP caches in local storage and all resources cached in memory such as images, CSS, JavaScript, XSL, and fonts.
Comment 7 Kenneth Rohde Christiansen 2012-12-25 03:19:20 PST
Sounds good to me
Comment 8 Thiago Marcos P. Santos 2012-12-27 06:12:03 PST
(In reply to comment #7)
> Sounds good to me

+1
Comment 9 Chris Dumez 2012-12-27 06:14:18 PST
Comment on attachment 180627 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:339
> +void ewk_context_resource_cache_clear(Ewk_Context *ewkContext)

star on wrong side
Comment 10 Gyuyoung Kim 2012-12-27 17:52:53 PST
Comment on attachment 180627 [details]
Patch

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

>>> Source/WebKit2/UIProcess/API/efl/ewk_context.h:317
>>> + * @param context context object to clear all resource caches.
>> 
>> I think this needs more explanation.
>> 
>> Why clear, not purge or so... clear doesnt mean remove, but could be just clearing (zeroing out or similar).
>> 
>> Clears all resources cached in memory such as local storage, HTTP cache,  ...
> 
> The reason why I use "Clear" is that this API calls WebProcess::clearResourceCaches in the end.
> 
> All resources mean images, CSS, JavaScript, XSL, and fonts, which are cached in memory and they are also cached as HTTP caches in local storage.
> http://trac.webkit.org/browser/trunk/Source/WebCore/loader/cache/MemoryCache.cpp#L693
> 
> How about this API description?
> 
> Clears HTTP caches in local storage and all resources cached in memory such as images, CSS, JavaScript, XSL, and fonts.

Nit: We don't use . at the end of @param field.

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:318
> + *

This line isn't needed.
Comment 11 Joone Hur 2012-12-28 10:42:01 PST
Thanks for your comments. I will update the patch soon.
Comment 12 Joone Hur 2012-12-28 12:19:06 PST
Created attachment 180892 [details]
Updated Patch
Comment 13 Chris Dumez 2012-12-28 12:23:17 PST
Comment on attachment 180892 [details]
Updated Patch

LGTM.
Comment 14 WebKit Review Bot 2012-12-28 18:26:48 PST
Comment on attachment 180892 [details]
Updated Patch

Clearing flags on attachment: 180892

Committed r138554: <http://trac.webkit.org/changeset/138554>
Comment 15 WebKit Review Bot 2012-12-28 18:26:54 PST
All reviewed patches have been landed.  Closing bug.