Bug 89721

Summary: [Chromium] DumpRenderTree on Android needs to configure fonts for testing
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: dpranke, japhet, jnd, ojan, peter, tkent, webkit.review.bot, zhenghao
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 66687    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing
none
Patch for landing none

Description Adam Barth 2012-06-21 17:43:20 PDT
[Chromium] DumpRenderTree on Android needs to configure fonts for testing
Comment 1 Adam Barth 2012-06-21 17:46:18 PDT
Created attachment 148923 [details]
Patch
Comment 2 Adam Barth 2012-06-21 18:18:45 PDT
Comment on attachment 148923 [details]
Patch

Missed a couple.
Comment 3 Adam Barth 2012-06-21 18:21:19 PDT
Created attachment 148934 [details]
Patch
Comment 4 Peter Beverloo 2012-06-22 08:17:27 PDT
Comment on attachment 148934 [details]
Patch

LGTM for the WebKit side, but please do not commit this until the SkUseTestFontConfigFile API has landed in Skia and has rolled to Chromium, then to WebKit, none of which is the case as of yet.
Comment 5 Adam Barth 2012-06-22 10:08:37 PDT
Peter, is there a bug I can track to see when that happens?
Comment 6 Adam Barth 2012-06-22 11:05:25 PDT
I'm tempted to land this with the call to SkUseTestFontConfigFile commented out until the API is available.
Comment 7 Nate Chapin 2012-06-22 11:06:37 PDT
(In reply to comment #6)
> I'm tempted to land this with the call to SkUseTestFontConfigFile commented out until the API is available.

What sort of time scale are we looking at for SkUseTestFontConfigFile to become available?
Comment 8 Adam Barth 2012-06-22 11:08:39 PDT
Created attachment 149063 [details]
Patch for landing
Comment 9 Adam Barth 2012-06-22 11:15:35 PDT
> What sort of time scale are we looking at for SkUseTestFontConfigFile to become available?

Looks like the chromium-android branch has a 79 line diff to Skia, of which this change accounts for all but 7.

Maybe we should try contributing that patch to Skia ourselves?  It's all in Android-specific files.
Comment 10 Adam Barth 2012-06-22 16:54:47 PDT
Comment on attachment 149063 [details]
Patch for landing

I think it's ok to land this patch with the call to SkUseTestFontConfigFile commented out.  We're going to drive the Skia side of this change to completion, so we might as well have this code upstream.
Comment 11 Peter Beverloo 2012-06-22 18:43:26 PDT
Agreed, though I'd prefer to attach a bug to the FIXME so it's easier to keep track of. Not that I have any doubt about you following up, of course :).
Comment 12 Adam Barth 2012-06-22 18:52:29 PDT
I filed Bug 89801 about this topic.
Comment 13 Adam Barth 2012-06-22 19:00:47 PDT
Created attachment 149155 [details]
Patch for landing
Comment 14 WebKit Review Bot 2012-06-22 19:33:06 PDT
Comment on attachment 149155 [details]
Patch for landing

Clearing flags on attachment: 149155

Committed r121086: <http://trac.webkit.org/changeset/121086>
Comment 15 WebKit Review Bot 2012-06-22 19:33:12 PDT
All reviewed patches have been landed.  Closing bug.
Comment 16 Adam Barth 2012-06-24 01:33:15 PDT
*** Bug 87006 has been marked as a duplicate of this bug. ***