DirectWrite Font Cache browser side hookup

DirectWrite Font Cache browser side hookup
This part is mainly to hookup browser side for building and loading font cache.
In browser's postprofileinit if font cache already exists in user profile directory
then we load that and create a read only shared memory section. If cache is not there
then we spawn a Chrome utility process to build font cache, which will be used when browser restarts.

For details on how whole thing works please look at design document at https://goto.google.com/font-cache-design

BUG=406659
R=cpu, scottmg, ananta

Committed: https://crrev.com/5140a1f1ba5bd57d5c7d9af04ed3eb8ac83f487d
Cr-Commit-Position: refs/heads/master@{#304277}

Committed: https://crrev.com/4e44258a618f23923f3671edca12cc58d83f90f3
Cr-Commit-Position: refs/heads/master@{#304474}

[Shrikant Kelkar, 2014-11-13 02:17:29]

ptal.

[scottmg, 2014-11-13 07:35:30]

https://codereview.chromium.org/724633002/diff/1/chrome/browser/chrome_browse...
File chrome/browser/chrome_browser_main_win.h (right):

https://codereview.chromium.org/724633002/diff/1/chrome/browser/chrome_browse...
chrome/browser/chrome_browser_main_win.h:35: virtual void
ChromeBrowserMainPartsWin::PostProfileInit() override;
remove "ChromeBrowserMainPartsWin::"

https://codereview.chromium.org/724633002/diff/1/content/common/dwrite_font_p...
File content/common/dwrite_font_platform_win.cc (right):

https://codereview.chromium.org/724633002/diff/1/content/common/dwrite_font_p...
content/common/dwrite_font_platform_win.cc:1158: // on FILE thread.
not sure how important it is, but maybe

DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));

rather than a comment

https://codereview.chromium.org/724633002/diff/1/content/public/common/dwrite...
File content/public/common/dwrite_font_platform_win.h (right):

https://codereview.chromium.org/724633002/diff/1/content/public/common/dwrite...
content/public/common/dwrite_font_platform_win.h:27: // Build and load is to
accomplish both tasks of creating a font cache within
this comment seems out of date with the changed name

[scottmg, 2014-11-13 07:40:13]

content/common/dwrite_font_platform_win_unittest.cc likely needs an update too.

[Shrikant Kelkar, 2014-11-13 20:29:21]

https://codereview.chromium.org/724633002/diff/1/chrome/browser/chrome_browse...
File chrome/browser/chrome_browser_main_win.h (right):

https://codereview.chromium.org/724633002/diff/1/chrome/browser/chrome_browse...
chrome/browser/chrome_browser_main_win.h:35: virtual void
ChromeBrowserMainPartsWin::PostProfileInit() override;
On 2014/11/13 07:35:30, scottmg wrote:
> remove "ChromeBrowserMainPartsWin::"

:D
Done

https://codereview.chromium.org/724633002/diff/1/content/common/dwrite_font_p...
File content/common/dwrite_font_platform_win.cc (right):

https://codereview.chromium.org/724633002/diff/1/content/common/dwrite_font_p...
content/common/dwrite_font_platform_win.cc:1158: // on FILE thread.
On 2014/11/13 07:35:30, scottmg wrote:
> not sure how important it is, but maybe
> 
> DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
> 
> rather than a comment

Removed comment, didn't add DCHECK as all base::File call seem to be already
checking for base::ThreadRestrictions::AssertIOAllowed();

https://codereview.chromium.org/724633002/diff/1/content/public/common/dwrite...
File content/public/common/dwrite_font_platform_win.h (right):

https://codereview.chromium.org/724633002/diff/1/content/public/common/dwrite...
content/public/common/dwrite_font_platform_win.h:27: // Build and load is to
accomplish both tasks of creating a font cache within
On 2014/11/13 07:35:30, scottmg wrote:
> this comment seems out of date with the changed name

Done.

[ananta, 2014-11-13 23:24:47]

lgtm

https://codereview.chromium.org/724633002/diff/20001/chrome/utility/font_cach...
File chrome/utility/font_cache_handler_win.cc (right):

https://codereview.chromium.org/724633002/diff/20001/chrome/utility/font_cach...
chrome/utility/font_cache_handler_win.cc:11:
FontCacheHandler::FontCacheHandler() {}
Nuke and move to header.

https://codereview.chromium.org/724633002/diff/20001/chrome/utility/font_cach...
File chrome/utility/font_cache_handler_win.h (right):

https://codereview.chromium.org/724633002/diff/20001/chrome/utility/font_cach...
chrome/utility/font_cache_handler_win.h:16: // continue to run as it is.
as is

[Shrikant Kelkar, 2014-11-13 23:37:59]

https://codereview.chromium.org/724633002/diff/20001/chrome/utility/font_cach...
File chrome/utility/font_cache_handler_win.cc (right):

https://codereview.chromium.org/724633002/diff/20001/chrome/utility/font_cach...
chrome/utility/font_cache_handler_win.cc:11:
FontCacheHandler::FontCacheHandler() {}
On 2014/11/13 23:24:47, ananta wrote:
> Nuke and move to header.

Done.

https://codereview.chromium.org/724633002/diff/20001/chrome/utility/font_cach...
File chrome/utility/font_cache_handler_win.h (right):

https://codereview.chromium.org/724633002/diff/20001/chrome/utility/font_cach...
chrome/utility/font_cache_handler_win.h:16: // continue to run as it is.
On 2014/11/13 23:24:47, ananta wrote:
> as is

Done.

[jschuh, 2014-11-14 14:27:08]

ipc security lgtm for chrome_utility_messages.h. browser -> unsandboxed utility

[scottmg, 2014-11-14 16:55:33]

https://codereview.chromium.org/724633002/diff/40001/content/common/dwrite_fo...
File content/common/dwrite_font_platform_win_unittest.cc (right):

https://codereview.chromium.org/724633002/diff/40001/content/common/dwrite_fo...
content/common/dwrite_font_platform_win_unittest.cc:44:
EXPECT_TRUE(BuildAndLoadFontCache(cache_file_path_));
lgtm, but I'm pretty sure this still won't compile?

[Shrikant Kelkar, 2014-11-14 17:21:46]

On 2014/11/14 16:55:33, scottmg wrote:
>
https://codereview.chromium.org/724633002/diff/40001/content/common/dwrite_fo...
> File content/common/dwrite_font_platform_win_unittest.cc (right):
> 
>
https://codereview.chromium.org/724633002/diff/40001/content/common/dwrite_fo...
> content/common/dwrite_font_platform_win_unittest.cc:44:
> EXPECT_TRUE(BuildAndLoadFontCache(cache_file_path_));
> lgtm, but I'm pretty sure this still won't compile?

Fixed :)

[cpu_(ooo_6.6-7.5), 2014-11-14 18:13:52]

lgtm

https://codereview.chromium.org/724633002/diff/60001/chrome/utility/font_cach...
File chrome/utility/font_cache_handler_win.h (right):

https://codereview.chromium.org/724633002/diff/60001/chrome/utility/font_cach...
chrome/utility/font_cache_handler_win.h:11: // Handles requests to build static
direct write font cache. Must be invoked
..to build a static direct ..

https://codereview.chromium.org/724633002/diff/60001/chrome/utility/font_cach...
chrome/utility/font_cache_handler_win.h:13: // process as it is time consuming
as well as exception prone thing. We already
as well as crash prone.

[Shrikant Kelkar, 2014-11-14 18:51:53]

https://codereview.chromium.org/724633002/diff/60001/chrome/utility/font_cach...
File chrome/utility/font_cache_handler_win.h (right):

https://codereview.chromium.org/724633002/diff/60001/chrome/utility/font_cach...
chrome/utility/font_cache_handler_win.h:11: // Handles requests to build static
direct write font cache. Must be invoked
On 2014/11/14 18:13:52, cpu wrote:
> ..to build a static direct ..

Done.

https://codereview.chromium.org/724633002/diff/60001/chrome/utility/font_cach...
chrome/utility/font_cache_handler_win.h:13: // process as it is time consuming
as well as exception prone thing. We already
On 2014/11/14 18:13:52, cpu wrote:
> as well as crash prone.

Done.

[Shrikant Kelkar, 2014-11-14 21:22:59]

The CQ bit was checked by shrikant@chromium.org

[commit-bot: I haz the power, 2014-11-14 21:23:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/724633002/80001

[commit-bot: I haz the power, 2014-11-14 22:18:22]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2014-11-14 22:20:02]

Patchset 5 (id:??) landed as
https://crrev.com/5140a1f1ba5bd57d5c7d9af04ed3eb8ac83f487d
Cr-Commit-Position: refs/heads/master@{#304277}

[scottmg, 2014-11-15 04:03:19]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/729103002/ by scottmg@chromium.org.

The reason for reverting is: Seems to be flaking occasionally, see e.g.
http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%28...

@@@STEP_LOG_LINE@UnchangedCustom_1@PrefHashBrowserTestUnchangedCustomInstance/PrefHashBrowserTestUnchangedCustom.UnchangedCustom/1
(run #4):@@@
@@@STEP_LOG_LINE@UnchangedCustom_1@[ RUN      ]
PrefHashBrowserTestUnchangedCustomInstance/PrefHashBrowserTestUnchangedCustom.UnchangedCustom/1@@@
@@@STEP_LOG_LINE@UnchangedCustom_1@[3308:4824:1114/190930:WARNING:data_reduction_proxy_settings.cc(328)]
SPDY proxy OFF at startup@@@
@@@STEP_LOG_LINE@UnchangedCustom_1@[4200:5032:1114/190932:FATAL:dwrite_font_platform_win.cc(493)]
Check failed: false.@@@
@@@STEP_LOG_LINE@UnchangedCustom_1@Backtrace:@@@
@@@STEP_LOG_LINE@UnchangedCustom_1@	base::DiscardableMemoryShmemAllocator::~DiscardableMemoryShmemAllocator
[0x10099E01+538517]@@@
@@@STEP_LOG_LINE@UnchangedCustom_1@	base::DiscardableMemoryShmemAllocator::~DiscardableMemoryShmemAllocator
[0x10129E1F+1128371]@@@
@@@STEP_LOG_LINE@UnchangedCustom_1@	content::WebContentsImpl::WasShown
[0x1326A46D+21053350]@@@
@@@STEP_LOG_LINE@UnchangedCustom_1@	DWriteCreateFactory [0x6D494A06+363338]@@@
@@@STEP_LOG_LINE@UnchangedCustom_1@	DWriteCreateFactory [0x6D49402C+360816]@@@.

[Shrikant Kelkar, 2014-11-15 22:51:51]

To be more conservative in terms of working size, I have put an arbitrary limit
on file size. I did some analysis on how many files will be cached with this
conservative approach and seems like most of the files on a typical Win8 system.
I have updated document at goto/font-cache-design with an histogram which gives
more detail.

ptal.

The changes are very minor. If trybots for Win(Dbg)/(Rel) succeeds, I am
planning to submit these changes using commit queue, so that we can get some
latency numbers from the field before trying other approaches. Please let me
know if you think otherwise.

[Shrikant Kelkar, 2014-11-17 20:09:22]

The CQ bit was checked by shrikant@chromium.org

[commit-bot: I haz the power, 2014-11-17 20:10:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/724633002/80002

[commit-bot: I haz the power, 2014-11-17 21:00:12]

Committed patchset #6 (id:80002)

[commit-bot: I haz the power, 2014-11-17 21:02:08]

Patchset 6 (id:??) landed as
https://crrev.com/4e44258a618f23923f3671edca12cc58d83f90f3
Cr-Commit-Position: refs/heads/master@{#304474}