89133 2012-06-14 15:33:50 -0700 [Chromium-Android] Initialize font rendering in DumpRenderTree 2012-06-14 21:07:09 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 89014 89162 66687 1 wangxianzhu wangxianzhu abarth derat dpranke jamesr jnd tkent tony webkit.review.bot zhenghao oldest_to_newest 649547 0 wangxianzhu 2012-06-14 15:33:50 -0700 Chromium-android use chromium-linux as the baseline of layout test, so we need to configure it to let the layout test results match those of chromium-linux as much as possible. In downstream, we initialize font rendering for Android in WebCore/platform/graphics/harfbuzz/FontPlatformDataHarfBuzz.cpp for both DumpRenderTree and Chromium, which is not good. We should initialize font rendering in DumpRenderTree code through WebKit API. 649555 1 147664 wangxianzhu 2012-06-14 15:42:03 -0700 Created attachment 147664 patch 649560 2 147664 abarth 2012-06-14 15:45:07 -0700 Comment on attachment 147664 patch View in context: https://bugs.webkit.org/attachment.cgi?id=147664&action=review > Source/WebKit/chromium/src/linuxish/WebFontRendering.cpp:74 > +#if OS(LINUX) > + WebFontInfo::setSubpixelPositioning(useSubpixelPositioning); > +#endif Is there non-test code that needs to be updated here as well? 649569 3 147664 wangxianzhu 2012-06-14 15:49:04 -0700 Comment on attachment 147664 patch View in context: https://bugs.webkit.org/attachment.cgi?id=147664&action=review >> Source/WebKit/chromium/src/linuxish/WebFontRendering.cpp:74 >> +#endif > > Is there non-test code that needs to be updated here as well? No except Chromium. I'll update the patch in http://codereview.chromium.org/10544103/ and request review. 649615 4 147664 tony 2012-06-14 16:22:15 -0700 Comment on attachment 147664 patch FWIW, the name linuxish is ambiguous to me. Maybe we should make a fontconfig directory instead? 649631 5 wangxianzhu 2012-06-14 16:44:29 -0700 (In reply to comment #4) > (From update of attachment 147664 [details]) > FWIW, the name linuxish is ambiguous to me. Maybe we should make a fontconfig directory instead? In Chromium's build/filename_rules.gyp, 'linuxish' is used to indicate directories or files used when 'OS=="android" or OS=="linux" or OS=="openbsd" or OS=="freebsd"'. The implementation of WebFontRendering depends on HarfBuzz and/or Freetype port of Skia (don't know much details of it). In WebKit the harfbuzz directory is included when 'use_x11=1 or OS=="android"' so it seems to equal to 'linuxish' in Chromium's term. 649633 6 jamesr 2012-06-14 16:47:38 -0700 (In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 147664 [details] [details]) > > FWIW, the name linuxish is ambiguous to me. Maybe we should make a fontconfig directory instead? > > In Chromium's build/filename_rules.gyp, 'linuxish' is used to indicate directories or files used when 'OS=="android" or OS=="linux" or OS=="openbsd" or OS=="freebsd"'. The implementation of WebFontRendering depends on HarfBuzz and/or Freetype port of Skia (don't know much details of it). In WebKit the harfbuzz directory is included when 'use_x11=1 or OS=="android"' so it seems to equal to 'linuxish' in Chromium's term. Drop the "ish". Just "linux", it's simpler. 649641 7 wangxianzhu 2012-06-14 16:58:10 -0700 (In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > (From update of attachment 147664 [details] [details] [details]) > > > FWIW, the name linuxish is ambiguous to me. Maybe we should make a fontconfig directory instead? > > > > In Chromium's build/filename_rules.gyp, 'linuxish' is used to indicate directories or files used when 'OS=="android" or OS=="linux" or OS=="openbsd" or OS=="freebsd"'. The implementation of WebFontRendering depends on HarfBuzz and/or Freetype port of Skia (don't know much details of it). In WebKit the harfbuzz directory is included when 'use_x11=1 or OS=="android"' so it seems to equal to 'linuxish' in Chromium's term. > > Drop the "ish". Just "linux", it's simpler. No, in Chromium's build/filename_rules.gyp, files containing "_linux" and directories named "linux", and it's not possible to include them back in particular gyp with normal 'sources/' under conditions. Tricky 'target_conditions' must be used to override the rules. I changed the directory to "linuxish" to meet the Chromium's file name rule. 649643 8 jamesr 2012-06-14 16:59:21 -0700 There has to be a better solution than saying "linuxish". 649660 9 wangxianzhu 2012-06-14 17:25:35 -0700 cq- because of the concerns of naming. We should make a decision of it. I'll send a mail to chromium-dev for it. 649662 10 abarth 2012-06-14 17:26:47 -0700 > There has to be a better solution than saying "linuxish". IMHO, we should follow Chromium's conventions here for now. If we'd like to improve on linuxish, we should probably start by talking with the folks who picked that name. I suspect the choice might be more constrained that we might guess at first. Sharing this code between Linux and Android is valuable. We can move forward with this patch and debate project-wide naming issues in parallel. 649699 11 147664 wangxianzhu 2012-06-14 18:11:39 -0700 Comment on attachment 147664 patch cq+ now. I'd be glad to make change based on any new decision. 649800 12 147664 webkit.review.bot 2012-06-14 20:07:12 -0700 Comment on attachment 147664 patch Clearing flags on attachment: 147664 Committed r120394: <http://trac.webkit.org/changeset/120394> 649801 13 webkit.review.bot 2012-06-14 20:07:17 -0700 All reviewed patches have been landed. Closing bug. 649819 14 webkit.review.bot 2012-06-14 20:59:18 -0700 Re-opened since this is blocked by 89162 147664 2012-06-14 15:42:03 -0700 2012-06-14 20:07:11 -0700 patch 89133 text/plain 4723 wangxianzhu SW5kZXg6IFNvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCShyZXZpc2lvbiAxMjAzNjYpCisrKyBTb3Vy Y2UvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEz IEBACisyMDEyLTA2LTE0ICBYaWFuemh1IFdhbmcgIDx3YW5neGlhbnpodUBjaHJvbWl1bS5vcmc+ CisKKyAgICAgICAgW0Nocm9taXVtLUFuZHJvaWRdIEluaXRpYWxpemUgZm9udCByZW5kZXJpbmcg aW4gRHVtcFJlbmRlclRyZWUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTg5MTMzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgKiBzcmMvbGludXhpc2gvV2ViRm9udFJlbmRlcmluZy5jcHA6CisgICAgICAgIChX ZWJLaXQ6OldlYkZvbnRSZW5kZXJpbmc6OnNldFN1YnBpeGVsUG9zaXRpb25pbmcpOiBOb3cgY2Fs bHMgV2ViRm9udEluZm86OnNldFN1YnBpeGVsUG9zaXRpb25pbmcgdG8gYXZvaWQgQVBJIHVzZXJz IGZyb20gY2FsbGluZyBib3RoIGFuZCBzaW1wbGlmeSBwbGF0Zm9ybSBkaWZmZXJlbmNlcyBhdCBj YWxsIHNpdGVzLgorCiAyMDEyLTA2LTE0ICBEYW5pZWwgRXJhdCAgPGRlcmF0QGNocm9taXVtLm9y Zz4KIAogICAgICAgICBbY2hyb21pdW0vbGludXhdIERyb3AgZGVwcmVjYXRlZCBXZWJGb250UmVu ZGVyU3R5bGU6OnVzZVN1YnBpeGVsCkluZGV4OiBTb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9s aW51eGlzaC9XZWJGb250UmVuZGVyaW5nLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0 L2Nocm9taXVtL3NyYy9saW51eGlzaC9XZWJGb250UmVuZGVyaW5nLmNwcAkocmV2aXNpb24gMTIw MzQ2KQorKysgU291cmNlL1dlYktpdC9jaHJvbWl1bS9zcmMvbGludXhpc2gvV2ViRm9udFJlbmRl cmluZy5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTMzLDYgKzMzLDEwIEBACiAKICNpbmNsdWRlICJG b250UGxhdGZvcm1EYXRhLmgiCiAKKyNpZiBPUyhMSU5VWCkKKyNpbmNsdWRlICJXZWJGb250SW5m by5oIgorI2VuZGlmCisKIHVzaW5nIFdlYkNvcmU6OkZvbnRQbGF0Zm9ybURhdGE7CiAKIG5hbWVz cGFjZSBXZWJLaXQgewpAQCAtNjUsNiArNjksOSBAQCB2b2lkIFdlYkZvbnRSZW5kZXJpbmc6OnNl dFN1YnBpeGVsUmVuZGVyCiB2b2lkIFdlYkZvbnRSZW5kZXJpbmc6OnNldFN1YnBpeGVsUG9zaXRp b25pbmcoYm9vbCB1c2VTdWJwaXhlbFBvc2l0aW9uaW5nKQogewogICAgIEZvbnRQbGF0Zm9ybURh dGE6OnNldFN1YnBpeGVsUG9zaXRpb25pbmcodXNlU3VicGl4ZWxQb3NpdGlvbmluZyk7CisjaWYg T1MoTElOVVgpCisgICAgV2ViRm9udEluZm86OnNldFN1YnBpeGVsUG9zaXRpb25pbmcodXNlU3Vi cGl4ZWxQb3NpdGlvbmluZyk7CisjZW5kaWYKIH0KIAogLy8gc3RhdGljCkluZGV4OiBUb29scy9D aGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PQotLS0gVG9vbHMvQ2hhbmdlTG9nCShyZXZpc2lvbiAxMjAzNjYp CisrKyBUb29scy9DaGFuZ2VMb2cJKHdvcmtpbmcgY29weSkKQEAgLTEsMyArMSwxNiBAQAorMjAx Mi0wNi0xNCAgWGlhbnpodSBXYW5nICA8d2FuZ3hpYW56aHVAY2hyb21pdW0ub3JnPgorCisgICAg ICAgIFtDaHJvbWl1bS1BbmRyb2lkXSBJbml0aWFsaXplIGZvbnQgcmVuZGVyaW5nIGluIER1bXBS ZW5kZXJUcmVlCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD04OTEzMworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg ICogRHVtcFJlbmRlclRyZWUvY2hyb21pdW0vTGF5b3V0VGVzdENvbnRyb2xsZXIuY3BwOgorICAg ICAgICAoTGF5b3V0VGVzdENvbnRyb2xsZXI6OnJlc2V0KToKKyAgICAgICAgKExheW91dFRlc3RD b250cm9sbGVyOjpzZXRUZXh0U3VicGl4ZWxQb3NpdGlvbmluZyk6CisgICAgICAgICogRHVtcFJl bmRlclRyZWUvY2hyb21pdW0vVGVzdFNoZWxsQW5kcm9pZC5jcHA6CisgICAgICAgIChwbGF0Zm9y bUluaXQpOgorCiAyMDEyLTA2LTE0ICBJYW4gVm9sbGljayAgPHZvbGxpY2tAY2hyb21pdW0ub3Jn PgogCiAgICAgICAgIFtjaHJvbWl1bV0gQ2VydGFpbiBzZXR0aW5ncyBpbiBDQ1NldHRpbmdzIGNv dWxkIGJlIGdsb2JhbApJbmRleDogVG9vbHMvRHVtcFJlbmRlclRyZWUvY2hyb21pdW0vTGF5b3V0 VGVzdENvbnRyb2xsZXIuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFRvb2xzL0R1bXBSZW5kZXJUcmVlL2No cm9taXVtL0xheW91dFRlc3RDb250cm9sbGVyLmNwcAkocmV2aXNpb24gMTIwMzQ2KQorKysgVG9v bHMvRHVtcFJlbmRlclRyZWUvY2hyb21pdW0vTGF5b3V0VGVzdENvbnRyb2xsZXIuY3BwCSh3b3Jr aW5nIGNvcHkpCkBAIC03OCw5ICs3OCw4IEBACiAjaW5jbHVkZSA8d3RmL093bkFycmF5UHRyLmg+ CiAjZW5kaWYKIAotI2lmIE9TKExJTlVYKQotI2luY2x1ZGUgImxpbnV4L1dlYkZvbnRJbmZvLmgi Ci0jaW5jbHVkZSAibGludXgvV2ViRm9udFJlbmRlcmluZy5oIgorI2lmIE9TKExJTlVYKSB8fCBP UyhBTkRST0lEKQorI2luY2x1ZGUgImxpbnV4aXNoL1dlYkZvbnRSZW5kZXJpbmcuaCIKICNlbmRp ZgogCiB1c2luZyBuYW1lc3BhY2UgV2ViQ29yZTsKQEAgLTcwMiw4ICs3MDEsNyBAQCB2b2lkIExh eW91dFRlc3RDb250cm9sbGVyOjpyZXNldCgpCiAgICAgbV90YXNrTGlzdC5yZXZva2VBbGwoKTsK ICAgICBtX3Nob3VsZFN0YXlPblBhZ2VBZnRlckhhbmRsaW5nQmVmb3JlVW5sb2FkID0gZmFsc2U7 CiAgICAgbV9oYXNDdXN0b21GdWxsU2NyZWVuQmVoYXZpb3IgPSBmYWxzZTsKLSNpZiBPUyhMSU5V WCkKLSAgICBXZWJGb250SW5mbzo6c2V0U3VicGl4ZWxQb3NpdGlvbmluZyhmYWxzZSk7CisjaWYg T1MoTElOVVgpIHx8IE9TKEFORFJPSUQpCiAgICAgV2ViRm9udFJlbmRlcmluZzo6c2V0U3VicGl4 ZWxQb3NpdGlvbmluZyhmYWxzZSk7CiAjZW5kaWYKIH0KQEAgLTIxOTYsMTMgKzIxOTQsMTEgQEAg dm9pZCBMYXlvdXRUZXN0Q29udHJvbGxlcjo6ZGVsaXZlcldlYkludAogCiB2b2lkIExheW91dFRl c3RDb250cm9sbGVyOjpzZXRUZXh0U3VicGl4ZWxQb3NpdGlvbmluZyhjb25zdCBDcHBBcmd1bWVu dExpc3QmIGFyZ3VtZW50cywgQ3BwVmFyaWFudCogcmVzdWx0KQogewotI2lmIE9TKExJTlVYKQor I2lmIE9TKExJTlVYKSB8fCBPUyhBTkRST0lEKQogICAgIC8vIFNpbmNlIEZvbnRDb25maWcgZG9l c24ndCBwcm92aWRlIGEgdmFyaWFibGUgdG8gY29udHJvbCBzdWJwaXhlbCBwb3NpdGlvbmluZywg d2UnbGwgZmFsbCBiYWNrCiAgICAgLy8gdG8gc2V0dGluZyBpdCBnbG9iYWxseSBmb3IgYWxsIGZv bnRzLgotICAgIGlmIChhcmd1bWVudHMuc2l6ZSgpID4gMCAmJiBhcmd1bWVudHNbMF0uaXNCb29s KCkpIHsKLSAgICAgICAgV2ViRm9udEluZm86OnNldFN1YnBpeGVsUG9zaXRpb25pbmcoYXJndW1l bnRzWzBdLnZhbHVlLmJvb2xWYWx1ZSk7CisgICAgaWYgKGFyZ3VtZW50cy5zaXplKCkgPiAwICYm IGFyZ3VtZW50c1swXS5pc0Jvb2woKSkKICAgICAgICAgV2ViRm9udFJlbmRlcmluZzo6c2V0U3Vi cGl4ZWxQb3NpdGlvbmluZyhhcmd1bWVudHNbMF0udmFsdWUuYm9vbFZhbHVlKTsKLSAgICB9CiAj ZW5kaWYKICAgICByZXN1bHQtPnNldE51bGwoKTsKIH0KSW5kZXg6IFRvb2xzL0R1bXBSZW5kZXJU cmVlL2Nocm9taXVtL1Rlc3RTaGVsbEFuZHJvaWQuY3BwCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFRvb2xzL0R1 bXBSZW5kZXJUcmVlL2Nocm9taXVtL1Rlc3RTaGVsbEFuZHJvaWQuY3BwCShyZXZpc2lvbiAxMjAz NDYpCisrKyBUb29scy9EdW1wUmVuZGVyVHJlZS9jaHJvbWl1bS9UZXN0U2hlbGxBbmRyb2lkLmNw cAkod29ya2luZyBjb3B5KQpAQCAtMzEsNiArMzEsNyBAQAogI2luY2x1ZGUgImNvbmZpZy5oIgog I2luY2x1ZGUgIlRlc3RTaGVsbC5oIgogCisjaW5jbHVkZSAibGludXhpc2gvV2ViRm9udFJlbmRl cmluZy5oIgogI2luY2x1ZGUgPGFuZHJvaWQvbG9nLmg+CiAjaW5jbHVkZSA8ZXJybm8uaD4KICNp bmNsdWRlIDxmY250bC5oPgpAQCAtMTI3LDQgKzEyOCw4IEBAIHZvaWQgcGxhdGZvcm1Jbml0KGlu dCogYXJnYywgY2hhcioqKiBhcmcKICAgICAgICAgLy8gUmVkaXJlY3Qgc3RkZXJyIHRvIHN0ZG91 dC4KICAgICAgICAgZHVwMigxLCAyKTsKICAgICB9CisKKyAgICAvLyBEaXNhYmxlIGF1dG8gaGlu dCBhbmQgdXNlIG5vcm1hbCBoaW50aW5nIGluIGxheW91dCB0ZXN0IG1vZGUgdG8gcHJvZHVjZSB0 aGUgc2FtZSBmb250IG1ldHJpY3MgYXMgY2hyb21pdW0tbGludXguCisgICAgV2ViS2l0OjpXZWJG b250UmVuZGVyaW5nOjpzZXRBdXRvSGludChmYWxzZSk7CisgICAgV2ViS2l0OjpXZWJGb250UmVu ZGVyaW5nOjpzZXRIaW50aW5nKFNrUGFpbnQ6OmtOb3JtYWxfSGludGluZyk7CiB9Cg==