Bug 69370 - WebKit not ensure font load before calling Skia to drawPosText during Chrome printing
Summary: WebKit not ensure font load before calling Skia to drawPosText during Chrome ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL: http://code.google.com/p/chromium/iss...
Keywords:
Depends on: 69684
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-04 13:46 PDT by Arthur Hsu
Modified: 2011-10-13 17:16 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.23 KB, patch)
2011-10-04 14:15 PDT, Arthur Hsu
no flags Details | Formatted Diff | Diff
Patch for landing (1.36 KB, patch)
2011-10-13 13:45 PDT, James Simonsen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Arthur Hsu 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.
Comment 1 Arthur Hsu 2011-10-04 14:15:44 PDT
Created attachment 109685 [details]
Patch
Comment 2 James Robinson 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?
Comment 3 Arthur Hsu 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.
Comment 4 James Robinson 2011-10-05 16:53:15 PDT
So this is just a fix for printing?  You should update the bug title to reflect that.
Comment 5 Arthur Hsu 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.
Comment 6 Adam Langley 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.
Comment 7 James Robinson 2011-10-06 11:39:24 PDT
Comment on attachment 109685 [details]
Patch

agl's LGTM is good enough for me.
Comment 8 WebKit Review Bot 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>
Comment 9 WebKit Review Bot 2011-10-06 12:26:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 James Simonsen 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.
Comment 11 James Simonsen 2011-10-13 13:45:10 PDT
Created attachment 110897 [details]
Patch for landing
Comment 12 James Simonsen 2011-10-13 17:16:34 PDT
Committed r97426: <http://trac.webkit.org/changeset/97426>