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 Rendering

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

agl, jamesr, simonjam, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

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

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>