RESOLVED FIXED 66132
Chromium: expose MemoryCache::prune and FontCache::purgeInactiveFontData
https://bugs.webkit.org/show_bug.cgi?id=66132
Summary Chromium: expose MemoryCache::prune and FontCache::purgeInactiveFontData
Mikhail Naganov
Reported 2011-08-12 04:11:06 PDT
For handling low-memory situations we need to call MemoryCache::prune and FontCache::purgeInactiveFontData. Need to have them in WebCache and WebFontCache.
Attachments
Patch (2.64 KB, application/octet-stream)
2011-08-12 04:13 PDT, Mikhail Naganov
mnaganov: commit-queue-
Patch (2.64 KB, patch)
2011-08-12 04:17 PDT, Mikhail Naganov
mnaganov: commit-queue-
Fixed style and compilation issues (2.68 KB, patch)
2011-08-12 04:51 PDT, Mikhail Naganov
fishd: review-
mnaganov: commit-queue-
Comments addressed (3.16 KB, patch)
2011-08-15 02:10 PDT, Mikhail Naganov
fishd: review+
mnaganov: commit-queue-
Fixed Windows Chromium port compilation issue (6.06 KB, patch)
2011-08-25 07:30 PDT, Mikhail Naganov
tonyg: review+
mnaganov: commit-queue-
Mikhail Naganov
Comment 1 2011-08-12 04:13:53 PDT
WebKit Review Bot
Comment 2 2011-08-12 04:17:08 PDT
Attachment 103757 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1 Source/WebKit/chromium/src/WebCache.cpp:113: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mikhail Naganov
Comment 3 2011-08-12 04:17:29 PDT
WebKit Review Bot
Comment 4 2011-08-12 04:20:02 PDT
Attachment 103758 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1 Source/WebKit/chromium/src/WebCache.cpp:113: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Review Bot
Comment 5 2011-08-12 04:22:33 PDT
Comment on attachment 103758 [details] Patch Attachment 103758 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9358590
WebKit Review Bot
Comment 6 2011-08-12 04:48:55 PDT
Comment on attachment 103758 [details] Patch Attachment 103758 [details] did not pass cr-mac-ews (chromium): Output: http://queues.webkit.org/results/9358594
Mikhail Naganov
Comment 7 2011-08-12 04:51:35 PDT
Created attachment 103761 [details] Fixed style and compilation issues
Tony Gentilcore
Comment 8 2011-08-12 04:55:17 PDT
This looks good to me. Darin should probably approve though since it is adding to the WebKit/chromium API.
Darin Fisher (:fishd, Google)
Comment 9 2011-08-12 08:58:08 PDT
Comment on attachment 103761 [details] Fixed style and compilation issues View in context: https://bugs.webkit.org/attachment.cgi?id=103761&action=review > Source/WebKit/chromium/public/WebCache.h:83 > + // Prune memory cache. nit: all of the other comments say "resource cache" instead of "memory cache" I'd probably move this method to be next to "clear", and it would be very helpful if you could document the difference between clear and prune. they sound like synonyms, but they aren't, right? makes consumers of this API have to wonder and search .cpp files for the answer :( > Source/WebKit/chromium/public/WebFontCache.h:51 > + WEBKIT_EXPORT static void purgeInactiveFontData(); would it make sense to also name this function "prune"? it seems like we might want to use consistent jargon.
Mikhail Naganov
Comment 10 2011-08-15 02:08:46 PDT
(In reply to comment #9) > (From update of attachment 103761 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=103761&action=review > > > Source/WebKit/chromium/public/WebCache.h:83 > > + // Prune memory cache. > > nit: all of the other comments say "resource cache" instead of "memory cache" > Done. > I'd probably move this method to be next to "clear", and it would be very > helpful if you could document the difference between clear and prune. they > sound like synonyms, but they aren't, right? makes consumers of this API > have to wonder and search .cpp files for the answer :( > Done. Also by searching .cpp files, I've found that "clear" actually duplicates already existing "MemoryCache::evictResources" method. > > Source/WebKit/chromium/public/WebFontCache.h:51 > > + WEBKIT_EXPORT static void purgeInactiveFontData(); > > would it make sense to also name this function "prune"? it seems like > we might want to use consistent jargon. Done.
Mikhail Naganov
Comment 11 2011-08-15 02:10:00 PDT
Created attachment 103894 [details] Comments addressed
Darin Fisher (:fishd, Google)
Comment 12 2011-08-15 11:06:06 PDT
Comment on attachment 103894 [details] Comments addressed View in context: https://bugs.webkit.org/attachment.cgi?id=103894&action=review > Source/WebKit/chromium/src/WebCache.cpp:70 > + cache->evictResources(); nice!
Mikhail Naganov
Comment 13 2011-08-15 14:29:40 PDT
Manually committed https://trac.webkit.org/changeset/93060 2011-08-15 Mikhail Naganov <mnaganov@chromium.org> Chromium: expose MemoryCache::prune and FontCache::purgeInactiveFontData. https://bugs.webkit.org/show_bug.cgi?id=66132 Reviewed by Darin Fisher. * public/WebCache.h: * public/WebFontCache.h: * src/WebCache.cpp: (WebKit::WebCache::prune): * src/WebFontCache.cpp: (WebKit::WebFontCache::prune):
Tony Chang
Comment 14 2011-08-15 15:16:00 PDT
The chromium win bots on build.webkit.org seem to be failing to link with the following error: 15>webkit.lib(WebCache.obj) : error LNK2019: unresolved external symbol "public: void __thiscall WebCore::MemoryCache::pruneLiveResources(void)" (?pruneLiveResources@MemoryCache@WebCore@@QAEXXZ) referenced in function "public: void __thiscall WebCore::MemoryCache::prune(void)" (?prune@MemoryCache@WebCore@@QAEXXZ) 15>webkit.lib(WebCache.obj) : error LNK2019: unresolved external symbol "public: void __thiscall WebCore::MemoryCache::pruneDeadResources(void)" (?pruneDeadResources@MemoryCache@WebCore@@QAEXXZ) referenced in function "public: void __thiscall WebCore::MemoryCache::prune(void)" (?prune@MemoryCache@WebCore@@QAEXXZ)
Adam Barth
Comment 15 2011-08-15 15:51:08 PDT
I've rolled out this commit because of the Windows build failures.
Mikhail Naganov
Comment 16 2011-08-25 07:30:38 PDT
Created attachment 105186 [details] Fixed Windows Chromium port compilation issue For some reason, VC++ compiler assumes that 'pruneLiveResources' and 'pruneDeadResources' are public. The issue is resolved by converting inline definitions of 'prune' and 'pruneToPercentage' into regular methods defined in .cpp file.
Mikhail Naganov
Comment 17 2011-08-25 08:07:52 PDT
Manually committed http://trac.webkit.org/changeset/93782 Chromium: expose MemoryCache::prune and FontCache::purgeInactiveFontData. https://bugs.webkit.org/show_bug.cgi?id=66132 Reviewed by Tony Gentilcore. * loader/cache/MemoryCache.cpp: (WebCore::MemoryCache::prune): (WebCore::MemoryCache::pruneToPercentage): * loader/cache/MemoryCache.h: Methods moved from .h to .cpp to work around compilation problem with the Win Chromium port. * public/WebCache.h: * public/WebFontCache.h: * src/WebCache.cpp: (WebKit::WebCache::clear): (WebKit::WebCache::prune): * src/WebFontCache.cpp: (WebKit::WebFontCache::prune): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@93782 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Note You need to log in before you can comment on or make changes to this bug.