Bug 97866 – Memory increase by uncontrolled decoded Image data after user's clearing cache

• Component: WebCore Misc.
• Status: UNCONFIRMED

[Keunyong Lee]

I found critical problem for controlling image decoded data after user selected clear cache menu.
This problem can be detected with above webpage address on chrome and safari browser.

[Step]
1. Access above address. That page contains a lot of jpeg images.
2. Scroll whole page.
3. Select "cache clear" menu on each browser menu.
4. Scroll whole page again.
5. Compare memory size of step2 with that of step4.

Memory increase of each tab page
[chrome]
180MB -> 500MB
[safari]
520MB -> 780MB

The cause of this problem is that all elements are removed from resource list after calling evictResources() although some resources of current page cannot be deleted.
As you know, decoded Image Data should be controlled by prune() or any other method.
However, because there is no elements in resource list after clear cache, memoryCache cannot remove old decoded data and stack all decoded data of current page.

I attach a patch for this problem.

[Keunyong Lee]

Created attachment 166182 [details]
patch for uncontrolled decoded image data

[Benjamin Poulain]

Comment on attachment 166182 [details]
patch for uncontrolled decoded image data

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

No opinion on the correctness at this time. Why is there no test?

> Source/WebCore/ChangeLog:14
> +        The cause of this problem is that all elements are removed from
> +        resource list after calling evictResources() although some resources
> +        of current page cannot be deleted. As you know, decoded Image Data
> +        should be controlled by prune() or any other method.However, because
> +        there is no elements in resource list after clear cache, memoryCache
> +        cannot remove old decoded data and stack all decoded data of
> +        current page.

You should either add a test, or give a very good reason no to have one.

> Source/WebCore/loader/cache/MemoryCache.cpp:752
> +    // In case of memory cache clearing, MemoryCache::setDisabled cause uncontrolled decoded data problem.
> +    // See <https://bugs.webkit.org/show_bug.cgi?id=97866>.

Such comments should not be here, it does not help understanding the following code.

Giving "why" comment is good, you should not just cite a particular problem in comments.

> Source/WebCore/loader/cache/MemoryCache.cpp:755
> +    CachedResourceMap::iterator e = m_resources.end();

"e" is a bad name.

> Source/WebCore/loader/cache/MemoryCache.cpp:756
> +    for (CachedResourceMap::iterator i = m_resources.begin(); i != e; ++i) {

"i" is also a bad name.

[Keunyong Lee]

I think it is better to keep same expressions for loop because "e" and "i" are already used in other functions of MemoryCache.cpp.

I will attach a test for this patch.

Thanks for your opinion.

[Benjamin Poulain]

(In reply to comment #3)
> [...] because "e" and "i" are already used in other functions of MemoryCache.cpp.

That does not mean that's a good idea.