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.
Created attachment 147664 [details] patch
Comment on attachment 147664 [details] 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?
Comment on attachment 147664 [details] 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.
Comment on attachment 147664 [details] patch FWIW, the name linuxish is ambiguous to me. Maybe we should make a fontconfig directory instead?
(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.
(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.
(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.
There has to be a better solution than saying "linuxish".
cq- because of the concerns of naming. We should make a decision of it. I'll send a mail to chromium-dev for it.
> 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.
Comment on attachment 147664 [details] patch cq+ now. I'd be glad to make change based on any new decision.
Comment on attachment 147664 [details] patch Clearing flags on attachment: 147664 Committed r120394: <http://trac.webkit.org/changeset/120394>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by 89162