Linux: Do better font substitution for Chinese characters.

Linux: Do better font substitution for Chinese characters.

BUG=347044
TEST=Manual, see bug.
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=260354

[Lei Zhang, 2014-03-27 00:05:14]

For the test case in the bug, it does require having a zh-CN truetype font
installed for the substitution to actually succeed.

[jungshik at Google, 2014-03-27 11:06:11]

Is this only about PDF-plugin? If so, can you add that to the CL description?

https://codereview.chromium.org/213793003/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_sandbox_host_linux.cc (right):

https://codereview.chromium.org/213793003/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_sandbox_host_linux.cc:418: if (is_english)
{
Can we access Chrome's per-script font pref here? Then, fallback fonts can be
set to them per script/language. Perhaps, in the next round.

https://codereview.chromium.org/213793003/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_sandbox_host_linux.cc:571: bool is_english
= false;
Perhaps, it'd be better to change change |is_english| to |is_lgc|
(Latin-Greek-Cyrillic) and set it to true for Windows code pages covering LGC
script. 

Arial/TNR/Courier covers more than LGC, but most Linux distros have very old
versions of them and the coverage does not go beyond LGC.

[Lei Zhang, 2014-03-27 19:31:24]

This code is used by more than just the PDF plugin. I don't know the PPAPI/Flash
code paths that well, but they would likely benefit from better font selection.

https://codereview.chromium.org/213793003/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_sandbox_host_linux.cc (right):

https://codereview.chromium.org/213793003/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_sandbox_host_linux.cc:418: if (is_english)
{
On 2014/03/27 11:06:11, Jungshik Shin wrote:
> Can we access Chrome's per-script font pref here? Then, fallback fonts can be
> set to them per script/language. Perhaps, in the next round. 

I don't know. Added a TODO. Will investigate in the future.

https://codereview.chromium.org/213793003/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_sandbox_host_linux.cc:571: bool is_english
= false;
On 2014/03/27 11:06:11, Jungshik Shin wrote:
> Perhaps, it'd be better to change change |is_english| to |is_lgc|
> (Latin-Greek-Cyrillic) and set it to true for Windows code pages covering LGC
> script. 

Done.

> Arial/TNR/Courier covers more than LGC, but most Linux distros have very old
> versions of them and the coverage does not go beyond LGC. 

On my system, half the characters in the PDF rendered with the default font.
With the zh-cn language preference set, it selected a simplified Chinese font
which has much better coverage.

[jam, 2014-03-27 22:44:07]

btw I'm not really familiar with this code, so I defer to whatever you guys
agree on. rubberstamp lgtm

[jungshik at Google, 2014-03-28 23:03:01]

https://codereview.chromium.org/213793003/diff/20001/content/browser/renderer...
File content/browser/renderer_host/render_sandbox_host_linux.cc (right):

https://codereview.chromium.org/213793003/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_sandbox_host_linux.cc:589:
FcLangSetAdd(langset, reinterpret_cast<const FcChar8*>("lt"));
lgc=true

https://codereview.chromium.org/213793003/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_sandbox_host_linux.cc:603:
FcLangSetAdd(langset, reinterpret_cast<const FcChar8*>("hr"));
lgc=true

https://codereview.chromium.org/213793003/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_sandbox_host_linux.cc:606:
FcLangSetAdd(langset, reinterpret_cast<const FcChar8*>("el"));
ditto

https://codereview.chromium.org/213793003/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_sandbox_host_linux.cc:614:
FcLangSetAdd(langset, reinterpret_cast<const FcChar8*>("ru"));
ditto

https://codereview.chromium.org/213793003/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_sandbox_host_linux.cc:621:
FcLangSetAdd(langset, reinterpret_cast<const FcChar8*>("tr"));
ditto

https://codereview.chromium.org/213793003/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_sandbox_host_linux.cc:624:
FcLangSetAdd(langset, reinterpret_cast<const FcChar8*>("vi"));
ditto

[jungshik at Google, 2014-03-28 23:03:48]

With the above additional lgc=true, lgtm

[Lei Zhang, 2014-03-28 23:05:49]

Done.

[Lei Zhang, 2014-03-28 23:05:52]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2014-03-28 23:06:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thestig@chromium.org/213793003/40001

[commit-bot: I haz the power, 2014-03-29 00:38:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-29 00:38:16]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[Lei Zhang, 2014-03-29 00:57:24]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2014-03-29 00:58:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thestig@chromium.org/213793003/40001

[commit-bot: I haz the power, 2014-03-29 02:43:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-29 02:43:34]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[Lei Zhang, 2014-03-29 03:44:01]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2014-03-29 03:44:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thestig@chromium.org/213793003/40001

[commit-bot: I haz the power, 2014-03-29 03:46:19]

Change committed as 260354