Pass font prefs to WebKit even if they are the empty string.

Pass font prefs to WebKit even if they are the empty string.

This is required because for per-script fonts, the empty string means to
fallback to the font for the Default script.

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

[falken, 2012-10-24 09:31:26]

Tony, can you review this change? It's not under webkit/ but related to the
WebPreferences code.

I was hesitant to do this because it results in a lot of set*FontFamily calls to
WebKit settings even if just one font pref changes. But using
performance_ui_tests, it looks like there's no impact.

Jungshik: FYI

[tony, 2012-10-24 17:33:06]

LGTM.  I guess this will be tested by a layout test in WebKit?

[falken, 2012-10-24 22:41:57]

On 2012/10/24 17:33:06, tony wrote:
> LGTM.  I guess this will be tested by a layout test in WebKit?

Thanks for the review.

I'm not sure how to test this. There is already a WebKit layout test for font
fallback when a per-script font setting is the empty string
(font-fallback-to-common-script.html). But this test acts on WebKit settings
directly and we want to test that Chrome passes the empty string to WebKit
settings.

I guess one of the following would work:

1. An "end-to-end" layout test that starts with setting prefs within Chrome
instead of WebKit settings directly.
2. A test that when a Chrome font pref is set to the empty string, the
corresponding setting in WebKit is the empty string. But it seems our existing
code like WebSettings just allows setting of WebKit settings, not getting.

Is there a way to do one of those, or is there some other way to test this?

[tony, 2012-10-24 22:47:13]

Oh right, this code change is in src/chrome.

I guess you could write a browser_test that does the end-to-end testing, but I'm
not sure it's worth the effort for this bug fix.  No test seems fine.

[commit-bot: I haz the power, 2012-10-25 00:26:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/falken@chromium.org/11264006/1

[commit-bot: I haz the power, 2012-10-25 00:26:57]

Presubmit check for 11264006-1 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome

Presubmit checks took 1.2s to calculate.

[falken, 2012-10-25 03:37:32]

thakis, can you review as an OWNER?

[Nico, 2012-10-25 04:21:57]

On 2012/10/25 03:37:32, falken wrote:
> thakis, can you review as an OWNER?

Lgtm

[commit-bot: I haz the power, 2012-10-25 05:30:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/falken@chromium.org/11264006/1

[commit-bot: I haz the power, 2012-10-25 11:51:48]

Change committed as 164050