Add default per-script CJK fonts on Windows.

Add default per-script CJK fonts on Windows.

BUG=94217, 2685
TEST=view a page that uses CSS property --webkit-locale and verify that the font is the desired default


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98928

[falken, 2011-08-25 09:39:21]

jungshik: Can you check these defaults?  I took them from the
locale_settings_win_*.xtb files.  I was not sure about adding Thai.  It has a
serif and sans-serif font but not a standard font.  Is this intentional?

sky: chrome/browser/ui OWNER

[sky, 2011-08-25 14:52:05]

http://codereview.chromium.org/7748014/diff/3001/chrome/browser/ui/tab_conten...
File chrome/browser/ui/tab_contents/tab_contents_wrapper.cc (right):

http://codereview.chromium.org/7748014/diff/3001/chrome/browser/ui/tab_conten...
chrome/browser/ui/tab_contents/tab_contents_wrapper.cc:313: #if
defined(OS_CHROMEOS) || defined(OS_WIN)
These nested ifs are a bit unwieldy. How about an array at the top of the file
for each platform?

[falken, 2011-08-26 11:17:25]

I updated the patch.

Another option is to add a resource id to all platforms whenever any platform
has that resource id, but then it might be burdensome to add new resources.

http://codereview.chromium.org/7748014/diff/3001/chrome/browser/ui/tab_conten...
File chrome/browser/ui/tab_contents/tab_contents_wrapper.cc (right):

http://codereview.chromium.org/7748014/diff/3001/chrome/browser/ui/tab_conten...
chrome/browser/ui/tab_contents/tab_contents_wrapper.cc:313: #if
defined(OS_CHROMEOS) || defined(OS_WIN)
On 2011/08/25 14:52:05, sky wrote:
> These nested ifs are a bit unwieldy. How about an array at the top of the file
> for each platform?

Changed to use array.

[sky, 2011-08-26 15:33:51]

LGTM

[jungshik at Google, 2011-08-26 17:29:03]

Please, add Traditional Chinese font pref as well on Windows:

PMingLiU (for serif and standard) and MingLiU (for monospace).

With that, LGTM.  I'll add more entries for other scripts later.

[falken, 2011-08-29 10:38:26]

sky, Jungshik: Can you review this new patch?  I had to modify it to fix a Clang
error on Linux:

chrome/browser/ui/tab_contents/tab_contents_wrapper.cc:387:24: error: comparison
of unsigned expression < 0 is always false [-Werror,-Wtautological-compare]
  for (size_t i = 0; i < kPerScriptFontDefaultsLength; ++i) 

Now kPerScriptFontDefaultsLength is only defined and used when it is non-zero.

I think eventually we won't need this workaround since all platforms will have
at least one default.

[falken, 2011-08-30 11:26:46]

sky: ping. I have your previous LGTM but thought this not quite trivial change
to fix the Clang error should be reviewed.  (Sorry if this is overcautious.)

[falken, 2011-08-31 02:42:09]

On 2011/08/30 11:26:46, falken wrote:
> sky: ping. I have your previous LGTM but thought this not quite trivial change
> to fix the Clang error should be reviewed.  (Sorry if this is overcautious.)

I think I'll go ahead and commit. Once Linux/Mac defaults are added the Clang
workaround can be removed anyway. I'll commit later today unless there is an
objection.

[sky, 2011-08-31 04:29:04]

SLGTM
I some how missed your ping, sorry.

[commit-bot: I haz the power, 2011-08-31 06:19:30]

Change committed as 98928