Bug 136417 – Pruning of live cached images should not remove images on the display.

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebCore Misc.

[Daewoong Jang]

This is the test case for bug https://bugs.webkit.org/show_bug.cgi?id=136259. Ensures images on the display won't be destroyed when the memory cache has exceeded its size limit.

[Daewoong Jang]

Created attachment 237427 [details]
test case

[Daewoong Jang]

Created attachment 237472 [details]
updated patch

[Darin Adler]

Comment on attachment 237472 [details]
updated patch

If this test successfully demonstrated the problem, then I would have expected it to fail on the EWS server, since the fix for the bug has not yet been uploaded to the server.

[Daewoong Jang]

(In reply to comment #3)
> (From update of attachment 237472 [details])
> If this test successfully demonstrated the problem, then I would have expected it to fail on the EWS server, since the fix for the bug has not yet been uploaded to the server.

I'm not sure, since the problem only appeared on WinCairo WebKit with cURL enabled. I have tested this case on Win/OS X WebKit and they had no problem.

[Pratik Solanki]

Comment on attachment 237472 [details]
updated patch

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

> LayoutTests/loader/prune-live-cached-image.html:41
> +    <!-- Assumes capacity of WebCore::MemoryCache == 8M(Which is from WebCore::cDefaultCacheCapacity). -->

It's actually much higher and is based on the amount of RAM on the machine. We call MemoryCache::setCapacities() to bump it up. For WebKit2, see Source/WebKit2/Shared/CacheModel.cpp. It also depends on the mode you're in e.g. Safari will have a bigger cache (128MB on a machine with more than 2GB RAM) than an app using a WKWebView (96MB).

I tried hardcoding a limit of 8MB but was unable to reproduce the crash on Mac.

[Daewoong Jang]

(In reply to comment #5)
> (From update of attachment 237472 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=237472&action=review
> 
> > LayoutTests/loader/prune-live-cached-image.html:41
> > +    <!-- Assumes capacity of WebCore::MemoryCache == 8M(Which is from WebCore::cDefaultCacheCapacity). -->
> 
> It's actually much higher and is based on the amount of RAM on the machine. We call MemoryCache::setCapacities() to bump it up. For WebKit2, see Source/WebKit2/Shared/CacheModel.cpp. It also depends on the mode you're in e.g. Safari will have a bigger cache (128MB on a machine with more than 2GB RAM) than an app using a WKWebView (96MB).
> 
> I tried hardcoding a limit of 8MB but was unable to reproduce the crash on Mac.

This test case is not meant to reproduce crash. The rectangle under the other rectangle will be displayed in all red if the built WebKit has the problem that I reported.

I'll try to solve the thing with cache size you've mentioned about, but I don't believe OS X WebKit would have the problem that this test case verifies. Thank you for your help!

[Daewoong Jang]

I'm thinking of updating test case, make it to set cache limit to exact amount of memory before run the test. However it seems there is not a way to do this in DumpRenderTree. What should I do? Should I have to implement a new function in TestRunner.cpp?

[Gyuyoung Kim]

Comment on attachment 237472 [details]
updated patch

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

> LayoutTests/loader/prune-live-cached-image-expected.txt:4
> +Top and bottom should look the same.

How does WTR know top and bottom result look same ? I wonder if you need to add image expected result too.

[Gyuyoung Kim]

Comment on attachment 237472 [details]
updated patch

Regardless of my question, this patch is too out of date. So cleared review? from attachment  237472 [details] so that this bug does not appear in http://webkit.org/pending-review. Please upload new patch based on latest trunk if you want to get review again.