Added restricted font fallback to custom font collection loader.

Added restricted font fallback to custom font collection loader.
After few experiments/crashes it seems that some users have enormous number of fonts in the registry. On suspect that we have for failing to load collection is that direct write cache is unable to load/handle those many fonts. In this CL we are trying to see if we can put arbitrary limit on number of fonts that could be loaded. We also added a fallback mechanism where in if loading from registry fails, we will try to load only default fonts. Definition of 'default' is taken from file prefs_tab_helper.cc ~ kDefaultFonts.

R=ananta,scottmg,cpu
BUG=399233
Committed: https://crrev.com/d4a0b1d5102a0fe01ea29f14c90a28afc87da95c
Cr-Commit-Position: refs/heads/master@{#292573}

[Shrikant Kelkar, 2014-08-28 23:58:54]

ptal.

[Shrikant Kelkar, 2014-08-29 00:43:22]

ptal.

[cpu_(ooo_6.6-7.5), 2014-08-29 01:14:31]

please beef up the CL description.

https://codereview.chromium.org/521483002/diff/20001/content/renderer/rendere...
File content/renderer/renderer_font_platform_win.cc (right):

https://codereview.chromium.org/521483002/diff/20001/content/renderer/rendere...
content/renderer/renderer_font_platform_win.cc:318: L"mingliu.ttc",
1) add a comment for each font that indicates which IDS_ this comes from, for
example cursive or monospace, etc.

2) please check if this is inclusive for win7.

https://codereview.chromium.org/521483002/diff/20001/content/renderer/rendere...
content/renderer/renderer_font_platform_win.cc:344: const UINT32
kMaxFontThreashold = 1000;
misspell --> threshold

[cpu_(ooo_6.6-7.5), 2014-08-29 01:17:54]

https://codereview.chromium.org/521483002/diff/20001/content/renderer/rendere...
File content/renderer/renderer_font_platform_win.cc (right):

https://codereview.chromium.org/521483002/diff/20001/content/renderer/rendere...
content/renderer/renderer_font_platform_win.cc:350: 
I rather have an "else" than rely on hr being e_fail.

Do we need the 351-355 block still? if so then move it to the end and add an
debug::alias bool that records the fallback.

[Shrikant Kelkar, 2014-08-29 01:41:28]

ptal.

https://codereview.chromium.org/521483002/diff/20001/content/renderer/rendere...
File content/renderer/renderer_font_platform_win.cc (right):

https://codereview.chromium.org/521483002/diff/20001/content/renderer/rendere...
content/renderer/renderer_font_platform_win.cc:318: L"mingliu.ttc",
On 2014/08/29 01:14:31, cpu wrote:
> 1) add a comment for each font that indicates which IDS_ this comes from, for
> example cursive or monospace, etc.
> 
> 2) please check if this is inclusive for win7.
> 

Done.

https://codereview.chromium.org/521483002/diff/20001/content/renderer/rendere...
content/renderer/renderer_font_platform_win.cc:344: const UINT32
kMaxFontThreashold = 1000;
On 2014/08/29 01:14:31, cpu wrote:
> misspell --> threshold

Done.

https://codereview.chromium.org/521483002/diff/20001/content/renderer/rendere...
content/renderer/renderer_font_platform_win.cc:350: 
On 2014/08/29 01:17:53, cpu wrote:
> I rather have an "else" than rely on hr being e_fail.
Idea is to execute it in both cases, try registry first if fonts less than 1000
if that fails too, then load restricted set. If fonts more than 1000, directly
go for restricted set.

> 
> Do we need the 351-355 block still? if so then move it to the end and add an
> debug::alias bool that records the fallback.
Done.

[cpu_(ooo_6.6-7.5), 2014-08-29 01:48:51]

lgtm

[Shrikant Kelkar, 2014-08-29 01:49:22]

The CQ bit was checked by shrikant@chromium.org

[commit-bot: I haz the power, 2014-08-29 01:50:55]

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

[scottmg, 2014-08-29 02:51:05]

I think it would be better to force load the standard set first (including other
standard ones like Verdana, Tahoma, etc.) and then load other fonts up to the
limit following that.

But this lgtm to hopefully improve the Canary crash rate anyway.

https://codereview.chromium.org/521483002/diff/40001/content/renderer/rendere...
File content/renderer/renderer_font_platform_win.cc (right):

https://codereview.chromium.org/521483002/diff/40001/content/renderer/rendere...
content/renderer/renderer_font_platform_win.cc:325: return true;
no need for return value here

https://codereview.chromium.org/521483002/diff/40001/content/renderer/rendere...
content/renderer/renderer_font_platform_win.cc:351: bool loadingRestricted =
false;
nit; loading_restricted

https://codereview.chromium.org/521483002/diff/40001/content/renderer/rendere...
content/renderer/renderer_font_platform_win.cc:356: g_font_loader.Get(), NULL,
0, g_font_collection.GetAddressOf());
does this succeed if any one of those fonts don't exist?

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

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/47950)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[commit-bot: I haz the power, 2014-08-29 03:28:01]

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

[commit-bot: I haz the power, 2014-08-29 03:28:02]

Try jobs failed on following builders:
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)

[Shrikant Kelkar, 2014-08-29 04:13:42]

The CQ bit was checked by shrikant@chromium.org

[commit-bot: I haz the power, 2014-08-29 04:14:02]

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

[commit-bot: I haz the power, 2014-08-29 04:48:21]

Committed patchset #3 (id:40001) as 7b8c1ba05a85a3eab2e51d82d37f0915f49f7214

[Shrikant Kelkar, 2014-08-29 23:26:28]

Sorry, I read your comments little late and had already scheduled on commit
queue. Will do those changes in next CL.

https://codereview.chromium.org/521483002/diff/40001/content/renderer/rendere...
File content/renderer/renderer_font_platform_win.cc (right):

https://codereview.chromium.org/521483002/diff/40001/content/renderer/rendere...
content/renderer/renderer_font_platform_win.cc:356: g_font_loader.Get(), NULL,
0, g_font_collection.GetAddressOf());
On 2014/08/29 02:51:05, scottmg wrote:
> does this succeed if any one of those fonts don't exist?

IIRC, it will not load that particular font/font family, but will continue
loading other fonts.

[commit-bot: I haz the power, 2014-09-10 03:06:24]

Patchset 3 (id:??) landed as
https://crrev.com/d4a0b1d5102a0fe01ea29f14c90a28afc87da95c
Cr-Commit-Position: refs/heads/master@{#292573}