Bug 66132 - Chromium: expose MemoryCache::prune and FontCache::purgeInactiveFontData
Summary: Chromium: expose MemoryCache::prune and FontCache::purgeInactiveFontData
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mikhail Naganov
URL:
Keywords:
Depends on: 66259
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-12 04:11 PDT by Mikhail Naganov
Modified: 2011-08-25 08:07 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.64 KB, application/octet-stream)
2011-08-12 04:13 PDT, Mikhail Naganov
mnaganov: commit-queue-
Details
Patch (2.64 KB, patch)
2011-08-12 04:17 PDT, Mikhail Naganov
mnaganov: commit-queue-
Details | Formatted Diff | Diff
Fixed style and compilation issues (2.68 KB, patch)
2011-08-12 04:51 PDT, Mikhail Naganov
fishd: review-
mnaganov: commit-queue-
Details | Formatted Diff | Diff
Comments addressed (3.16 KB, patch)
2011-08-15 02:10 PDT, Mikhail Naganov
fishd: review+
mnaganov: commit-queue-
Details | Formatted Diff | Diff
Fixed Windows Chromium port compilation issue (6.06 KB, patch)
2011-08-25 07:30 PDT, Mikhail Naganov
tonyg: review+
mnaganov: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Naganov 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.
Comment 1 Mikhail Naganov 2011-08-12 04:13:53 PDT
Created attachment 103757 [details]
Patch
Comment 2 WebKit Review Bot 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.
Comment 3 Mikhail Naganov 2011-08-12 04:17:29 PDT
Created attachment 103758 [details]
Patch
Comment 4 WebKit Review Bot 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.
Comment 5 WebKit Review Bot 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
Comment 6 WebKit Review Bot 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
Comment 7 Mikhail Naganov 2011-08-12 04:51:35 PDT
Created attachment 103761 [details]
Fixed style and compilation issues
Comment 8 Tony Gentilcore 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.
Comment 9 Darin Fisher (:fishd, Google) 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.
Comment 10 Mikhail Naganov 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.
Comment 11 Mikhail Naganov 2011-08-15 02:10:00 PDT
Created attachment 103894 [details]
Comments addressed
Comment 12 Darin Fisher (:fishd, Google) 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!
Comment 13 Mikhail Naganov 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):
Comment 14 Tony Chang 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)
Comment 15 Adam Barth 2011-08-15 15:51:08 PDT
I've rolled out this commit because of the Windows build failures.
Comment 16 Mikhail Naganov 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.
Comment 17 Mikhail Naganov 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