Bug 69370

Summary: WebKit not ensure font load before calling Skia to drawPosText during Chrome printing
Product: WebKit Reporter: Arthur Hsu <arthurhsu>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: agl, jamesr, simonjam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
URL: http://code.google.com/p/chromium/issues/detail?id=94421
Bug Depends on: 69684    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch for landing none

Arthur Hsu
Reported 2011-10-04 13:46:18 PDT
In Chrome sandbox model, GDI calls such as GetOutlineTextMetrics() can fail if the font is not loaded in memory already. Chrome browser process implements a hack that will force GDI to load the desired font into memory, and in WebKit terms, it's called PlatformSupport::ensureFontLoaded(). This means that WebKit renderer must call PlatformSupport::ensureFontLoaded() right before calling Skia to draw glyphs because they are both in sandbox.
Attachments
Patch (1.23 KB, patch)
2011-10-04 14:15 PDT, Arthur Hsu 		no flags
DetailsFormatted DiffDiff
Patch for landing (1.36 KB, patch)
2011-10-13 13:45 PDT, James Simonsen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Arthur Hsu
Comment 1 2011-10-04 14:15:44 PDT
Created attachment 109685 [details] Patch
James Robinson
Comment 2 2011-10-04 14:19:37 PDT
Comment on attachment 109685 [details] Patch Is there any way to construct a test case for this? What is the failure mode if this doesn't happen?
Arthur Hsu
Comment 3 2011-10-04 14:24:56 PDT
(In reply to comment #2) > (From update of attachment 109685 [details]) > Is there any way to construct a test case for this? This currently causes problem only in Chrome printing code because Skia needs to query GDI outline text metrics for generating PDF data. I'm not sure how that sandbox/PDF generation can fit in WebKit testing. > What is the failure mode if this doesn't happen? In Chrome it will crash the renderer and the printing will fail.
James Robinson
Comment 4 2011-10-05 16:53:15 PDT
So this is just a fix for printing? You should update the bug title to reflect that.
Arthur Hsu
Comment 5 2011-10-05 18:09:04 PDT
(In reply to comment #4) > So this is just a fix for printing? You should update the bug title to reflect that. Done.
Adam Langley
Comment 6 2011-10-06 11:38:12 PDT
This LGTM, but I am not a WebKit reviewer. You need an r+ from a real WebKit reviewer before committing.
James Robinson
Comment 7 2011-10-06 11:39:24 PDT
Comment on attachment 109685 [details] Patch agl's LGTM is good enough for me.
WebKit Review Bot
Comment 8 2011-10-06 12:26:42 PDT
Comment on attachment 109685 [details] Patch Clearing flags on attachment: 109685 Committed r96847: <http://trac.webkit.org/changeset/96847>
WebKit Review Bot
Comment 9 2011-10-06 12:26:47 PDT
All reviewed patches have been landed. Closing bug.
James Simonsen
Comment 10 2011-10-07 18:56:58 PDT
There's a pretty significant memory regression on Chromium Windows. This is the only Windows change in the regression range, so we're going to try rolling it out and see if memory returns to normal. If it turns out this isn't causing it, I'll roll it back in.
James Simonsen
Comment 11 2011-10-13 13:45:10 PDT
Created attachment 110897 [details] Patch for landing
James Simonsen
Comment 12 2011-10-13 17:16:34 PDT
Committed r97426: <http://trac.webkit.org/changeset/97426>
Note You need to log in before you can comment on or make changes to this bug.