Stop delay loading user32.dll from chrome.dll on x86/Windows.

Check in a custom-built import library for user32 exports up to Windows XP SP2/SP3, which is used by chrome.dll in preference to the Platform SDK's user32.lib import library.
Custom build an import library for Chrome's post-WinXP imports, that binds to a fictional "user32-delay.dll".
Implement and test a delay load hook that diverts dynamic loading of any dll "foo-delay.dll" to "foo.dll".

R=cpu@chromium.org
BUG=176040
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=191173

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=191418

[Sigurður Ásgeirsson, 2013-02-19 21:47:35]

Please take a look - I think this legitimately belongs in base, but I'm very
much open to other suggestions.

[cpu_(ooo_6.6-7.5), 2013-02-20 21:52:54]

could this code be in chrome/app ?

https://codereview.chromium.org/12295040/diff/1/base/win/delay_load_hook.cc
File base/win/delay_load_hook.cc (right):

https://codereview.chromium.org/12295040/diff/1/base/win/delay_load_hook.cc#n...
base/win/delay_load_hook.cc:57: }
not sure we want this here, the current handling via the exception seems more
robust, at least from the perspective of actually showing a dialog to the user.
I mean the dialog is shown by csrss in the current code.

I guess we also want a crash report which can be accommodated either here or
there.

At least a unification of this with hard_error_handler_win.cc needs to happen.

[cpu_(ooo_6.6-7.5), 2013-02-20 21:57:22]

I also see https://codereview.chromium.org/12210017/ had this in chrome/app

[Sigurður Ásgeirsson, 2013-02-26 18:56:19]

PTAL - I baked this into chrome/app. However it's not clear to me that
chrome/app code should link into chrome.dll?

https://codereview.chromium.org/12295040/diff/1/base/win/delay_load_hook.cc
File base/win/delay_load_hook.cc (right):

https://codereview.chromium.org/12295040/diff/1/base/win/delay_load_hook.cc#n...
base/win/delay_load_hook.cc:57: }
On 2013/02/20 21:52:55, cpu wrote:
> not sure we want this here, the current handling via the exception seems more
> robust, at least from the perspective of actually showing a dialog to the
user.
> I mean the dialog is shown by csrss in the current code.
> 
> I guess we also want a crash report which can be accommodated either here or
> there.
> 
> At least a unification of this with hard_error_handler_win.cc needs to happen.
> 

Moved this to chrome/app. However, the intent here is to link the hook into
chrome.dll to handle the redirection.
Maybe the error reporting from here is not the best idea?

[cpu_(ooo_6.6-7.5), 2013-02-26 20:51:11]

hmm I see both our exe and our dll have their own delayload machinery and hooks.

since chrome.exe depends now of so many things in src\ it would seem we might
need the magic in both.

But I assume this patch works which means that chrome dll is linking chrome\app
? which is fine by me.

[cpu_(ooo_6.6-7.5), 2013-02-26 20:53:28]

looks good, there is the other comment (of line 57) left to address.

[cpu_(ooo_6.6-7.5), 2013-02-26 20:55:48]

also along the same lines of comment of line 57 it seems dangerous to use LOG
which afaik takes locks from inside a loader callback. In other words it would
be easy to break us if somebody adds a windows call during logging that triggers
this path.

[Sigurður Ásgeirsson, 2013-02-26 21:19:40]

On Tue, Feb 26, 2013 at 3:55 PM, <cpu@chromium.org> wrote:

> also along the same lines of comment of line 57 it seems dangerous to use
> LOG
> which afaik takes locks from inside a loader callback. In other words it
> would
> be easy to break us if somebody adds a windows call during logging that
> triggers
> this path.
>

I think the right thing to do here is to remove the error reporting
altogether, and rely on the exception handler for that. The main reason is
because there is code in Chrome that probes for imports under an SEH, and
this code as-is will end up doing fatal logging.

A delay load hook doesn't run under the loader's lock - but it will grab it
if it invokes LoadLibrary or GetProcAddress.


>
>
>
>
https://codereview.chromium.**org/12295040/<https://codereview.chromium.org/1...
>

[cpu_(ooo_6.6-7.5), 2013-02-27 00:41:59]

sure, ping me when the change is ready.

[M-A Ruel, 2013-03-03 01:48:02]

Please commit user32.winxp.lib manually first.

https://codereview.chromium.org/12295040/diff/18001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/12295040/diff/18001/build/common.gypi#newcode...
build/common.gypi:4045: #     link against the Vista-level X64 import library.
I'd prefer to not break the 5 Win2k3 users once chrome x64 gets out.

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/crea...
File build/win/importlibs/create_importlib_win.py (right):

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:6: # A utility script to create
import libs from an import description file.
"""Creates import libs from an import description file."""

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:85: 
remove one line

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:87: """Write an assembly file
containing empty declarations.
Writes

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:122: 
Remove one line

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:179: dest='output_dir',
Remove 'dest' and 'default=False' everywhere, they are not necessary.

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/filt...
File build/win/importlibs/filter_export_list.py (right):

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:6: # A utility script to help with
maintaining DLL import lists.
docstring

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:55: dest='reverse',
Remove dest and default.

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:77: # Reverse the found_exports list.
This comment is confusing, to me it sounds like reverse(foo)

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:78: found_exports = [e for e in
(set(master_mapping.values()) -
found_exports = list(set(master_mapping.values()) - set(found_exports))

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:81: # Sort the found exports for tidy
output.
optional style nit: I would have written it like that:
print '\n'.join(sorted(found_exports))

then you wouldn't need the list() cast at line 78.

[cpu_(ooo_6.6-7.5), 2013-03-04 02:18:53]

Besides the following items I think the code looks good.

I still will like to see consensus on the best place to put the cpp code. I lean
slightly to have it in /build but I can be talked into \base

I understand it is problematic unit testing in /build so I am ok having the test
live in base.

https://codereview.chromium.org/12295040/diff/18001/base/win/delay_load_hook.cc
File base/win/delay_load_hook.cc (right):

https://codereview.chromium.org/12295040/diff/18001/base/win/delay_load_hook....
base/win/delay_load_hook.cc:35: const char kDelaySuffix[] = "-delay.dll";
any way to see if we get 8dot3 ~1 madness here?

And by 'see' I just want to hear "yeah its not going to be a problem".

https://codereview.chromium.org/12295040/diff/18001/base/win/delay_load_hook....
base/win/delay_load_hook.cc:65: // Nothing to do here.
How about this here?

// By returning NULL the delayload machinery will generate
// the structure VcppException that some code might want to
// handle.
return NULL;

[cpu_(ooo_6.6-7.5), 2013-03-04 02:21:04]

also note that I don't read python, so I go with whatever maruel and siggi come
up with.

[Sigurður Ásgeirsson, 2013-03-25 20:43:29]

Please take another look guys. I've reduced the scope of the patch to only
affect Chrome.dll in x86, as that's what we're shipping now.
Consequently I've moved the delay load hook back (again) to chrome\app. 
I've also re-written the CL description, it should reflect the actual truth at
this point.

Maruel: do I still need to commit the .lib file manually ahead of this CL?

https://codereview.chromium.org/12295040/diff/18001/base/win/delay_load_hook.cc
File base/win/delay_load_hook.cc (right):

https://codereview.chromium.org/12295040/diff/18001/base/win/delay_load_hook....
base/win/delay_load_hook.cc:35: const char kDelaySuffix[] = "-delay.dll";
On 2013/03/04 02:18:53, cpu wrote:
> any way to see if we get 8dot3 ~1 madness here?
> 
> And by 'see' I just want to hear "yeah its not going to be a problem".

You'll get whatever you put into the binary here, e.g. if your import library
binds to "somereallylongnameofanarchanecomponent.dll", then that's what you do.
The file system does not play to this point.

https://codereview.chromium.org/12295040/diff/18001/base/win/delay_load_hook....
base/win/delay_load_hook.cc:65: // Nothing to do here.
On 2013/03/04 02:18:53, cpu wrote:
> How about this here?
> 
> // By returning NULL the delayload machinery will generate
> // the structure VcppException that some code might want to
> // handle.
> return NULL;

Done.

https://codereview.chromium.org/12295040/diff/18001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/12295040/diff/18001/build/common.gypi#newcode...
build/common.gypi:4045: #     link against the Vista-level X64 import library.
On 2013/03/03 01:48:02, Marc-Antoine Ruel wrote:
> I'd prefer to not break the 5 Win2k3 users once chrome x64 gets out.

I've reduced the patch in scope to only affect chrome.dll on x86.

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/crea...
File build/win/importlibs/create_importlib_win.py (right):

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:6: # A utility script to create
import libs from an import description file.
On 2013/03/03 01:48:02, Marc-Antoine Ruel wrote:
> """Creates import libs from an import description file."""

Done.

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:85: 
On 2013/03/03 01:48:02, Marc-Antoine Ruel wrote:
> remove one line

Done.

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:87: """Write an assembly file
containing empty declarations.
On 2013/03/03 01:48:02, Marc-Antoine Ruel wrote:
> Writes

Done.

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:122: 
On 2013/03/03 01:48:02, Marc-Antoine Ruel wrote:
> Remove one line

Done.

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:179: dest='output_dir',
On 2013/03/03 01:48:02, Marc-Antoine Ruel wrote:
> Remove 'dest' and 'default=False' everywhere, they are not necessary.

Done.

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/filt...
File build/win/importlibs/filter_export_list.py (right):

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:6: # A utility script to help with
maintaining DLL import lists.
On 2013/03/03 01:48:02, Marc-Antoine Ruel wrote:
> docstring

Done.

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:55: dest='reverse',
On 2013/03/03 01:48:02, Marc-Antoine Ruel wrote:
> Remove dest and default.

Done.

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:77: # Reverse the found_exports list.
On 2013/03/03 01:48:02, Marc-Antoine Ruel wrote:
> This comment is confusing, to me it sounds like reverse(foo)

Done.

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:78: found_exports = [e for e in
(set(master_mapping.values()) -
On 2013/03/03 01:48:02, Marc-Antoine Ruel wrote:
> found_exports = list(set(master_mapping.values()) - set(found_exports))

Done.

https://codereview.chromium.org/12295040/diff/18001/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:81: # Sort the found exports for tidy
output.
On 2013/03/03 01:48:02, Marc-Antoine Ruel wrote:
> optional style nit: I would have written it like that:
> print '\n'.join(sorted(found_exports))
> 
> then you wouldn't need the list() cast at line 78.

Done.

[M-A Ruel, 2013-03-26 12:59:14]

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
File build/win/importlibs/create_import_lib.gypi (right):

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
build/win/importlibs/create_import_lib.gypi:47: '--output-file',
'<(lib_dir)/<(RULE_INPUT_ROOT).lib',
Or <@(_outputs), as you prefer.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
File build/win/importlibs/create_importlib_win.py (right):

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:29: ; This file is autogenerated,
do not edit.
; This file is autogenerated by create_importlib_win.py, do not edit.

Naming the script is useful when someone is tracking down what script generated
a file.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:40: ; This file is autogenerated,
do not edit.
same

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:85: 
remove one line

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:113: with
open(os.path.join(self._temp_dir, asm_name), 'w') as stubs_file:
We always use 'wb' for consistency. Unless you actually need CRLF.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:136: print "Obj file", obj_file #
DO NOT SUBMIT
I guess you won't submit this :)

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
File build/win/importlibs/filter_export_list.py (right):

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:6: """A utility script to help with
maintaining DLL import lists."""
"""Helps maintaining DLL import lists."""

but I'd expand "helps"

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:13: _EXPORT_RE = re.compile("""
It should be a r"""

otherwise you'll get caught by an \ escape eventually.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:42: # Slurp the master file.
with open(master_file) as f:
  master_exports = ast.literal_eval(f.read())
return dict((i.split('@', 1)[0], i) for i in master_exports)

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:76: found_exports =
list(set(master_mapping.values()) - set(found_exports))
not necessary to convert back into a list.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:79: '\n'.join(sorted(found_exports))
Did you intent to print it?

Then return 0

https://codereview.chromium.org/12295040/diff/27017/chrome/app/delay_load_hoo...
File chrome/app/delay_load_hook_win.cc (right):

https://codereview.chromium.org/12295040/diff/27017/chrome/app/delay_load_hoo...
chrome/app/delay_load_hook_win.cc:77: NOTREACHED() << "Impossible delay load
notification.";
It's tricky,  you could get into a loop here because NOTREACHED() could trigger
another delayed import.

[Sigurður Ásgeirsson, 2013-03-26 15:35:35]

Thanks! Please take another look.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
File build/win/importlibs/create_import_lib.gypi (right):

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
build/win/importlibs/create_import_lib.gypi:47: '--output-file',
'<(lib_dir)/<(RULE_INPUT_ROOT).lib',
On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> Or <@(_outputs), as you prefer.

Done.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
File build/win/importlibs/create_importlib_win.py (right):

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:29: ; This file is autogenerated,
do not edit.
On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> ; This file is autogenerated by create_importlib_win.py, do not edit.
> 
> Naming the script is useful when someone is tracking down what script
generated
> a file.

Done.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:40: ; This file is autogenerated,
do not edit.
On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> same

Done.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:85: 
On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> remove one line

Done.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:113: with
open(os.path.join(self._temp_dir, asm_name), 'w') as stubs_file:
On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> We always use 'wb' for consistency. Unless you actually need CRLF.

Done.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:136: print "Obj file", obj_file #
DO NOT SUBMIT
On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> I guess you won't submit this :)

Ooops - I'm used to the presubmit check (in Syzygy) slapping me when there's
such a comment. I guess chromium presubmit checks don't have this?

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
File build/win/importlibs/filter_export_list.py (right):

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:6: """A utility script to help with
maintaining DLL import lists."""
On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> """Helps maintaining DLL import lists."""
> 
> but I'd expand "helps"

The "how" is in the script's usage string.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:13: _EXPORT_RE = re.compile("""
On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> It should be a r"""
> 
> otherwise you'll get caught by an \ escape eventually.

Done.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:42: # Slurp the master file.
On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> with open(master_file) as f:
>   master_exports = ast.literal_eval(f.read())
> return dict((i.split('@', 1)[0], i) for i in master_exports)

I generally prefer readability to terseness, do you consider this more readable?
(Not being facetious - I don't write enough python to consider myself an
expert).

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:76: found_exports =
list(set(master_mapping.values()) - set(found_exports))
On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> not necessary to convert back into a list.

Done.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:79: '\n'.join(sorted(found_exports))
On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> Did you intent to print it?
> 
> Then return 0

Done.

https://codereview.chromium.org/12295040/diff/27017/chrome/app/delay_load_hoo...
File chrome/app/delay_load_hook_win.cc (right):

https://codereview.chromium.org/12295040/diff/27017/chrome/app/delay_load_hoo...
chrome/app/delay_load_hook_win.cc:77: NOTREACHED() << "Impossible delay load
notification.";
On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> It's tricky,  you could get into a loop here because NOTREACHED() could
trigger
> another delayed import.

That's true but unlikely now that we're not delay loading user32 (in particular)
any more - what do you suggest?
I'd consider it a bug if logging requires delay loading in any case :).

[M-A Ruel, 2013-03-26 15:44:37]

lgtm on my side.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
File build/win/importlibs/create_importlib_win.py (right):

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/crea...
build/win/importlibs/create_importlib_win.py:136: print "Obj file", obj_file #
DO NOT SUBMIT
On 2013/03/26 15:35:35, Sigurður Ásgeirsson wrote:
> On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> > I guess you won't submit this :)
> 
> Ooops - I'm used to the presubmit check (in Syzygy) slapping me when there's
> such a comment. I guess chromium presubmit checks don't have this?

Yes but only on commit.

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
File build/win/importlibs/filter_export_list.py (right):

https://codereview.chromium.org/12295040/diff/27017/build/win/importlibs/filt...
build/win/importlibs/filter_export_list.py:42: # Slurp the master file.
On 2013/03/26 15:35:35, Sigurður Ásgeirsson wrote:
> On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> > with open(master_file) as f:
> >   master_exports = ast.literal_eval(f.read())
> > return dict((i.split('@', 1)[0], i) for i in master_exports)
> 
> I generally prefer readability to terseness, do you consider this more
readable?
> (Not being facetious - I don't write enough python to consider myself an
> expert).

This is intermediate python but I don't really mind, it's more FYI. One day,
you'll embrace generators.

https://codereview.chromium.org/12295040/diff/27017/chrome/app/delay_load_hoo...
File chrome/app/delay_load_hook_win.cc (right):

https://codereview.chromium.org/12295040/diff/27017/chrome/app/delay_load_hoo...
chrome/app/delay_load_hook_win.cc:77: NOTREACHED() << "Impossible delay load
notification.";
On 2013/03/26 15:35:35, Sigurður Ásgeirsson wrote:
> On 2013/03/26 12:59:14, Marc-Antoine Ruel wrote:
> > It's tricky,  you could get into a loop here because NOTREACHED() could
> trigger
> > another delayed import.
> 
> That's true but unlikely now that we're not delay loading user32 (in
particular)
> any more - what do you suggest?
> I'd consider it a bug if logging requires delay loading in any case :).

Indeed, I just guess there's no way to ensure that, and it'll likely regress
throughout the years. Maybe just add a comment about that.

[cpu_(ooo_6.6-7.5), 2013-03-26 18:20:58]

lgtm

The notreached also makes me a bit antsy. How about a __debugbreak() ?

Also is worth pointing out that std::string will use the heap which uses
tcmalloc which uses win32 in a wantom way. I think its fine but I think is worth
thinking on a followup CL to make this trimming code work without mallocs.

Anyhow this is good to go, lets get this in.

[commit-bot: I haz the power, 2013-03-26 18:46:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/siggi@chromium.org/12295040/51002

[commit-bot: I haz the power, 2013-03-26 18:46:59]

Presubmit check for 12295040-51002 failed and returned exit status 1.

INFO:root:Found 11 file(s).
/b/commit-queue/workdir/chromium/build/win/importlibs/create_importlib_win.py:
Has shebang but not executable bit
/b/commit-queue/workdir/chromium/build/win/importlibs/filter_export_list.py: Has
shebang but not executable bit
Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    chrome/chrome.user32.delay.imports
    chrome/chrome.gyp
    chrome/chrome_dll.gypi

Presubmit checks took 1.3s to calculate.

[Sigurður Ásgeirsson, 2013-03-26 18:52:05]

Brett - I need owner's approval for this change, and you seem to qualify...

[brettw, 2013-03-26 20:25:33]

LGTM rubberstamp, I hope you guys know what you are doing.

[commit-bot: I haz the power, 2013-03-26 22:28:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/siggi@chromium.org/12295040/51002

[commit-bot: I haz the power, 2013-03-27 02:30:30]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-03-27 13:50:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/siggi@chromium.org/12295040/74001

[commit-bot: I haz the power, 2013-03-27 16:37:34]

Retried try job too often on win7_aura for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[Sigurður Ásgeirsson, 2013-03-28 00:16:43]

M-A, can I ask you to look at the last patch - I had to move the dependency
declaration to the custom import lib around. The dependency was creeping into
unit_tests.exe, which didn't run well with an import to the non-existent
user32-delay.dll, and no delay load hook.

[Sigurður Ásgeirsson, 2013-03-28 12:31:31]

Ugh, belay that. Looks like I need to shift some more config to get it out
of the way of more tests.


On Wed, Mar 27, 2013 at 8:16 PM, <siggi@chromium.org> wrote:

> M-A, can I ask you to look at the last patch - I had to move the dependency
> declaration to the custom import lib around. The dependency was creeping
> into
> unit_tests.exe, which didn't run well with an import to the non-existent
> user32-delay.dll, and no delay load hook.
>
>
https://codereview.chromium.**org/12295040/<https://codereview.chromium.org/1...
>

[M-A Ruel, 2013-03-28 12:53:28]

found small nit, still lgtm

https://codereview.chromium.org/12295040/diff/100001/build/win/importlibs/fil...
File build/win/importlibs/filter_export_list.py (right):

https://codereview.chromium.org/12295040/diff/100001/build/win/importlibs/fil...
build/win/importlibs/filter_export_list.py:64: return 1
Please remove, parser.error() calls sys.exit(2)

[Sigurður Ásgeirsson, 2013-03-28 14:25:08]

Thanks, committing.

https://codereview.chromium.org/12295040/diff/100001/build/win/importlibs/fil...
File build/win/importlibs/filter_export_list.py (right):

https://codereview.chromium.org/12295040/diff/100001/build/win/importlibs/fil...
build/win/importlibs/filter_export_list.py:64: return 1
On 2013/03/28 12:53:28, Marc-Antoine Ruel wrote:
> Please remove, parser.error() calls sys.exit(2)

Done.

[commit-bot: I haz the power, 2013-03-28 14:25:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/siggi@chromium.org/12295040/109001

[commit-bot: I haz the power, 2013-03-28 16:38:57]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-03-28 17:00:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/siggi@chromium.org/12295040/109001

[commit-bot: I haz the power, 2013-03-28 17:51:10]

Change committed as 191173

[commit-bot: I haz the power, 2013-03-29 16:30:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/siggi@chromium.org/12295040/140001

[commit-bot: I haz the power, 2013-03-29 19:17:59]

Change committed as 191418