Bug 118532

Summary:

[Qt] memory leak in WebCore::FontCache::getLastResortFallbackFont

Product:

WebKit

Reporter:

Fabienne Semeria <fsemeria>

Component:

WebCore Misc.

Assignee:

Allan Sandfeld Jensen <allan.jensen>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

alikhtarev, allan.jensen, commit-queue, jbriance, jturcotte, milian.wolff, noam

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

119740

Bug Blocks:

110211

Attachments:

Description	Flags
fix memory leak in FontCache::getLastResortFallbackFont	none
Patch	jturcotte: review+

Fabienne Semeria

Reported 2013-07-10 09:07:48 PDT

memory leak in WebCore::FontCache::getLastResortFallbackFont

Attachments
fix memory leak in FontCache::getLastResortFallbackFont (1.46 KB, patch) 2013-07-10 09:12 PDT, Fabienne Semeria	no flags	Details Formatted Diff Diff
Patch (1.71 KB, patch) 2013-08-13 05:22 PDT, Allan Sandfeld Jensen	jturcotte: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Fabienne Semeria

Comment 1 2013-07-10 09:12:27 PDT

Created attachment 206393 [details] fix memory leak in FontCache::getLastResortFallbackFont

WebKit Commit Bot

Comment 2 2013-07-11 03:29:39 PDT

Comment on attachment 206393 [details] fix memory leak in FontCache::getLastResortFallbackFont Clearing flags on attachment: 206393 Committed r152563: <http://trac.webkit.org/changeset/152563>

WebKit Commit Bot

Comment 3 2013-07-11 03:29:42 PDT

All reviewed patches have been landed. Closing bug.

Alex

Comment 4 2013-07-23 05:53:33 PDT

It looks like after applying this patch any QtWebKit-based browser crashing on loading this site: http://www.kevs3d.co.uk/dev/canvasmark Tested on Ubuntu 12.04 64bit

Fabienne Semeria

Comment 5 2013-07-23 09:06:30 PDT

Indeed, it looks like the Qt version of FontCache::getLastResortFallbackFont may now return a null pointer in some cases. It may be safer to revert the patch until a fix for this is available.

WebKit Commit Bot

Comment 6 2013-08-13 04:44:28 PDT

Re-opened since this is blocked by bug 119740

Allan Sandfeld Jensen

Comment 7 2013-08-13 05:22:09 PDT

Created attachment 208619 [details] Patch

Fabienne Semeria

Comment 8 2013-08-13 05:37:06 PDT

(In reply to comment #7) > Created an attachment (id=208619) [details] > Patch We tested a patch similar to this one, and it works for us.

Allan Sandfeld Jensen

Comment 9 2013-08-13 05:41:18 PDT

(In reply to comment #8) > (In reply to comment #7) > > Created an attachment (id=208619) [details] [details] > > Patch > > We tested a patch similar to this one, and it works for us. Great. We also had a case where the old fix crashed Qt browser-demo on 32bit, and the new version doesn't. I never got Alex' example to crash though, can anyone confirm that also works?

Milian Wolff

Comment 10 2013-08-14 04:53:17 PDT

This patch works fine for me (see bug 119740 and bug 119088).

Jocelyn Turcotte

Comment 11 2013-08-15 06:37:09 PDT

Comment on attachment 208619 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=208619&action=review > Source/WebCore/ChangeLog:8 > + Allocate FontPlatformData on the stack instead on the heap. A note that getCachedFontData does a deep copy of FontPlatformData could be useful to explain why this change is necessary.

Allan Sandfeld Jensen

Comment 12 2013-08-15 09:22:55 PDT

Committed r154103: <http://trac.webkit.org/changeset/154103>

Note You need to log in before you can comment on or make changes to this bug.