Bug 89721 - [Chromium] DumpRenderTree on Android needs to configure fonts for testing
Summary: [Chromium] DumpRenderTree on Android needs to configure fonts for testing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
: 87006 (view as bug list)
Depends on:
Blocks: 66687
  Show dependency tree / graph
 
Reported: 2012-06-21 17:43 PDT by Adam Barth
Modified: 2012-06-24 01:33 PDT (History)
8 users (show)

See Also:

Attachments
Patch (6.83 KB, patch)
2012-06-21 17:46 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
Patch (10.36 KB, patch)
2012-06-21 18:21 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
Patch for landing (10.46 KB, patch)
2012-06-22 11:08 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
Patch for landing (10.53 KB, patch)
2012-06-22 19:00 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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. ***