Transform ImagePreReader into PreReadFile (v2).

Transform ImagePreReader into PreReadFile (v2).

We can reduce startup time by pre-reading different non-PE files
(e.g. ChromeDWriteFontCache, GPU cache, resources).

This CL transforms ImagePreReader into PreReadFile, which can
pre-read any file type on Vista+. On XP, PreReadFile can only
pre-read PE files.

A similar patch was already submitted
(https://codereview.chromium.org/1412673006/). It was reverted because
of an XP-only perf regression (crbug.com/551488). This new version
loads files in memory using ::LoadLibraryExW instead of
base::MemoryMappedFile on XP. This was the existing behavior of
ImagePreReader, so it should no longer cause a perf regression.
Unfortunately, it means that on XP, only PE files can be pre-read.

BUG=547794
Committed: https://crrev.com/ab55daf96d88dc362951dcdead6617ab0379b569
Cr-Commit-Position: refs/heads/master@{#358917}

[fdoray, 2015-11-10 20:25:51]

fdoray@chromium.org changed reviewers:
+ grt@chromium.org, jochen@chromium.org

[fdoray, 2015-11-10 20:25:52]

grt@: Could you take another look?

Here are my answers to your comments on the previous CL:

https://codereview.chromium.org/1412673006/diff/180001/chrome/app/file_pre_re...
File chrome/app/file_pre_reader_win.cc (right):

https://codereview.chromium.org/1412673006/diff/180001/chrome/app/file_pre_re...
chrome/app/file_pre_reader_win.cc:75: UINT previous_error_mode =
::SetErrorMode(SEM_FAILCRITICALERRORS);
On 2015/11/10 19:29:10, grt wrote:
> does the pre-read happen before ChromeMainDelegate::SandboxInitialized runs? i
> see that it adds in FAILCRITICALERRORS.

Yes, PreReadFile is called before ChromeMainDelegate::SandboxInitialized.

MainDllLoader::Launch first calls MainDllLoader::Load, which calls PreReadFile
internally. Then, it calls ChromeMain which calls
ChromeMainDelegate::SandboxInitialized.

https://codereview.chromium.org/1412673006/diff/180001/chrome/app/file_pre_re...
chrome/app/file_pre_reader_win.cc:78: HMODULE dll_module = ::LoadLibraryExW(
On 2015/11/10 19:29:10, grt wrote:
> use
>     ScopedNativeLibrary dll_module(::LoadLibraryExW(...));
> to avoid the leak on line 90 (and remove the call to FreeLibrary on line 100).

Done.

https://codereview.chromium.org/1412673006/diff/180001/chrome/app/file_pre_re...
chrome/app/file_pre_reader_win.cc:95: const size_t dll_module_length =
On 2015/11/10 19:29:10, grt wrote:
> SizeOfImage is a 32-bit unsigned int for both 32-bit and 64-bit images. Can
you
> get rid of this cast if you change TouchPagesInRange so that it takes the
length
> as uint32_t?

Done.

[grt (UTC plus 2), 2015-11-10 20:29:22]

lgtm. good luck!

https://codereview.chromium.org/1432973004/diff/20001/chrome/app/file_pre_rea...
File chrome/app/file_pre_reader_win.cc (right):

https://codereview.chromium.org/1432973004/diff/20001/chrome/app/file_pre_rea...
chrome/app/file_pre_reader_win.cc:23: DCHECK_GT(length, static_cast<size_t>(0));
nit: size_t -> uint32_t

[jochen (gone - plz use gerrit), 2015-11-10 20:31:02]

lgtm

[fdoray, 2015-11-10 21:02:58]

The CQ bit was checked by fdoray@chromium.org

[fdoray, 2015-11-10 21:02:58]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
grt@chromium.org
Link to the patchset: https://codereview.chromium.org/1432973004/#ps40001
(title: "grt comment #3  size_t -> uint32_t")

[commit-bot: I haz the power, 2015-11-10 21:04:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1432973004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1432973004/40001

[commit-bot: I haz the power, 2015-11-10 21:54:24]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-11-10 21:55:05]

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