Bug 126577 – [SOUP] [WK2] - Disable MemoryCache when the DOCUMENT_VIEWER cache model is set

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Tomas Popela]

As in http://trac.webkit.org/changeset/152483, but this time for WK2. When WEBKIT_CACHE_MODEL_DOCUMENT_VIEWER is set we need to disable the MemoryCache completely (based on cacheMinDeadCapacity and cacheMaxDeadCapacity values) to prevent caching the resources.

[Tomas Popela]

Created attachment 220518 [details]
Proposed patch

[Carlos Garcia Campos]

In r152483 we also check that cacheTotalCapacity is 0, don't we need that in wk2 as well?

[Carlos Garcia Campos]

Comment on attachment 220518 [details]
Proposed patch

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

> Source/WebKit2/WebProcess/soup/WebProcessSoup.cpp:107
> +    bool disableCache = !cacheMinDeadCapacity && !cacheMaxDeadCapacity;
> +    WebCore::memoryCache()->setDisabled(disableCache);

This is not specific to GTK, it also affects EFL, so I would confirm with EFL guys that this is the desired behaviour for them too, otherwise we should need to do this inside a platform ifdef.

[Tomas Popela]

(In reply to comment #2)
> In r152483 we also check that cacheTotalCapacity is 0, don't we need that in wk2 as well?

Actually the calculateCacheSizes will return != 0 for cacheTotalCapacity even for WEBKIT_CACHE_MODEL_DOCUMENT_VIEWER. Another solution will be just asking straight for cache model.

[Carlos Garcia Campos]

(In reply to comment #4)
> (In reply to comment #2)
> > In r152483 we also check that cacheTotalCapacity is 0, don't we need that in wk2 as well?
> 
> Actually the calculateCacheSizes will return != 0 for cacheTotalCapacity even for WEBKIT_CACHE_MODEL_DOCUMENT_VIEWER. Another solution will be just asking straight for cache model.

Yes, in the end what we want is to make sure that memory cache is disabled for DocumentViewer cache model, because that's what our public API documentation says. So, I would do something like this:

#if PLATFORM(GTK)
WebCore::memoryCache()->setDisabled(cacheModel == CacheModelDocumentViewer);
#endif

It's also more obvious and it's the only model that returns cacheMinDeadCapacity = cacheMaxDeadCapacity = 0 after all.

[Tomas Popela]

Created attachment 221158 [details]
Proposed patch v2

[Sergio Villar Senin]

Comment on attachment 221158 [details]
Proposed patch v2

Same comment, have you checked with the EFL guys if they want this behavior? I'm almost sure they want, so we wouldn't need the ifdef

[Ryuan Choi]

(In reply to comment #7)
> (From update of attachment 221158 [details])
> Same comment, have you checked with the EFL guys if they want this behavior? I'm almost sure they want, so we wouldn't need the ifdef

I also think that macro is not necessary.

[Tomas Popela]

Created attachment 221237 [details]
Proposed patch v3

Disable MemoryCache even for EFL (remove the GTK ifdef)

[Carlos Garcia Campos]

Comment on attachment 221237 [details]
Proposed patch v3

Thanks

[WebKit Commit Bot]

Comment on attachment 221237 [details]
Proposed patch v3

Clearing flags on attachment: 221237

Committed r162059: <http://trac.webkit.org/changeset/162059>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.