Bug 66673

Summary: [chromium] Fonts returned by FontCache::getFontDataForCharacters() are never released
Product: WebKit Reporter: Tony Gentilcore <tonyg>
Component: New BugsAssignee: Tony Gentilcore <tonyg>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, jamesr, mnaganov, msaboff, satish, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 66774    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Description Tony Gentilcore 2011-08-22 07:14:09 PDT
[chromium] Fonts returned by FontCache::getFontDataForCharacters() are never released
Comment 1 Tony Gentilcore 2011-08-22 07:20:11 PDT
Created attachment 104675 [details]
Patch
Comment 2 James Robinson 2011-08-22 20:09:13 PDT
Comment on attachment 104675 [details]
Patch

R=me. What a repetitive API (repetitive API)
Comment 3 WebKit Review Bot 2011-08-22 21:09:22 PDT
Comment on attachment 104675 [details]
Patch

Clearing flags on attachment: 104675

Committed r93579: <http://trac.webkit.org/changeset/93579>
Comment 4 WebKit Review Bot 2011-08-22 21:09:27 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Tony Gentilcore 2011-08-23 06:35:30 PDT
When this rolled into chromium it triggered an "ASSERTION FAILED: m_purgePreventCount" in the nacl tests:
http://build.chromium.org/p/chromium/builders/Linux%20Tests%20(dbg)(2)/builds/11797/steps/nacl_integration/logs/stdio

This likely means that we are missing a FontCachePurgePreventer in some nacl specific code. I'll investigate and roll forward w/ the fix.
Comment 6 James Robinson 2011-08-25 12:27:31 PDT
We saw an ASSERT() fail on the mac bot today, think it's related:  The assertion is:

ASSERTION FAILED: fontCache()->generation() == m_generation

build log here:

http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Mac10.6%20(CG)(dbg)/builds/202/steps/webkit_tests/logs/stdio

test was fast/css-generated-content/table-before-after-child-add.html.
Comment 7 Tony Gentilcore 2011-08-26 04:13:34 PDT
> We saw an ASSERT() fail on the mac bot today, think it's related:

It isn't directly related to this patch as FontCacheLinux isn't used on the mac and my original patch was rolled back almost immediately due to a nacl test failure. I've broken this issue off into https://bugs.webkit.org/show_bug.cgi?id=67031 along with some additional details.

I have a patch coming shortly which rolls this forward again along with a FontCachePurgePreventer to fix the nacl test.
Comment 8 Tony Gentilcore 2011-08-26 04:31:20 PDT
Created attachment 105345 [details]
Patch
Comment 9 Tony Gentilcore 2011-08-31 10:45:56 PDT
James, are you still willing to review the follow-up or should I look around for someone else?
Comment 10 James Robinson 2011-09-01 10:38:17 PDT
Comment on attachment 105345 [details]
Patch

R=me

sorry for the delay, was out on vacation for a week
Comment 11 WebKit Review Bot 2011-09-01 11:40:43 PDT
Comment on attachment 105345 [details]
Patch

Clearing flags on attachment: 105345

Committed r94323: <http://trac.webkit.org/changeset/94323>
Comment 12 WebKit Review Bot 2011-09-01 11:40:47 PDT
All reviewed patches have been landed.  Closing bug.