Break crashpad_handler into lib and exe for Windows

Break crashpad_handler into lib and exe for Windows

I've heard/lived enough horror stories about AV, outbound-blocking
firewalls, etc. on Windows, that I think the best approach is to have
chrome.exe embed the majority of crashpad_handler and jump to it as
early as possible when running in that mode.

So, move most of crashpad_handler into a static_library with just main()
in the executable target.

R=mark@chromium.org
BUG=chromium:546288, crashpad:27
Committed: https://chromium.googlesource.com/crashpad/crashpad/+/20869d94682cb72b3e4544b47cc27059aa0af3ad

[scottmg, 2015-11-06 01:14:30]

(The Chrome side is here for reference
https://codereview.chromium.org/1416133003/ .)

[Mark Mentovai, 2015-11-06 15:12:43]

LGTM with this change.

You can add BUG=crashpad:27 because this sort of addresses a little bit of that,
kind of.

We considered this option for Mac but decided against it. Loading another copy
of Chrome and all of the libraries it depends on bloats the crashpad_handler
process, and all of those library dependencies have module initializers that
aren’t always reliable. On the other hand, putting this into Chrome does mean
that you get to share with Chrome the parts of base that the handler uses, and
the parts of crashpad_util that the handler and client both use. And I certainly
appreciate the antivirus and application firewall problems.

Does this mean that chunks of Crashpad will be pulled into both chrome.exe and
chrome.dll?

https://codereview.chromium.org/1416873016/diff/20001/handler/entry_point.cc
File handler/entry_point.cc (right):

https://codereview.chromium.org/1416873016/diff/20001/handler/entry_point.cc#...
handler/entry_point.cc:1: // Copyright 2015 The Crashpad Authors. All rights
reserved.
The file naming is confusing, I think of “main.cc” as “the thing that provides
main()”, which is kind of a synonym for “entry_point.cc”. I’d do:

main.{cc,h} → handler_main.{cc,h}
entry_point.cc → main.cc

https://codereview.chromium.org/1416873016/diff/20001/handler/main.h
File handler/main.h (right):

https://codereview.chromium.org/1416873016/diff/20001/handler/main.h#newcode20
handler/main.h:20: //! \brief The main() of the crashpad_handler binary.
`main()`, `crashpad_handler`

https://codereview.chromium.org/1416873016/diff/20001/handler/main.h#newcode23
handler/main.h:23: //! binary, but called and used in the same way.
in the same way as…?

[scottmg, 2015-11-06 18:13:21]

Description was changed from

==========
Break crashpad_handler into lib and exe for Windows

I've heard/lived enough horror stories about AV, outbound-blocking
firewalls, etc. on Windows, that I think the best approach is to have
chrome.exe embed the majority of crashpad_handler and jump to it as
early as possible when running in that mode.

So, move most of crashpad_handler into a static_library with just main()
in the executable target.

R=mark@chromium.org
BUG=chromium:546288
==========

to

==========
Break crashpad_handler into lib and exe for Windows

I've heard/lived enough horror stories about AV, outbound-blocking
firewalls, etc. on Windows, that I think the best approach is to have
chrome.exe embed the majority of crashpad_handler and jump to it as
early as possible when running in that mode.

So, move most of crashpad_handler into a static_library with just main()
in the executable target.

R=mark@chromium.org
BUG=chromium:546288, crashpad:27
==========

[scottmg, 2015-11-06 18:22:23]

On 2015/11/06 15:12:43, Mark Mentovai wrote:
> LGTM with this change.
> 
> You can add BUG=crashpad:27 because this sort of addresses a little bit of
that,
> kind of.

Done.

> We considered this option for Mac but decided against it. Loading another copy
> of Chrome and all of the libraries it depends on bloats the crashpad_handler
> process, and all of those library dependencies have module initializers that
> aren’t always reliable. On the other hand, putting this into Chrome does mean
> that you get to share with Chrome the parts of base that the handler uses, and
> the parts of crashpad_util that the handler and client both use. And I
certainly
> appreciate the antivirus and application firewall problems.

Agreed. It's a little better on Windows because of the exe/dll separation (i.e.
there's less junk that's also getting loaded into the handler, because when
acting as crashpad_handler, chrome.exe doesn't load chrome.dll). I think the
firewall/AV considerations are a bigger concern though.

> 
> Does this mean that chunks of Crashpad will be pulled into both chrome.exe and
> chrome.dll?

Yes, in particular 'util' is duplicated between exe and dll because both handler
and client use it. "Most" of crashpad will only be in chrome.exe though.

I'm not sure if we could refactor to remove (or split and remove most of) util
from client. It's largely using util for file IO for the database and settings.

> 
> https://codereview.chromium.org/1416873016/diff/20001/handler/entry_point.cc
> File handler/entry_point.cc (right):
> 
>
https://codereview.chromium.org/1416873016/diff/20001/handler/entry_point.cc#...
> handler/entry_point.cc:1: // Copyright 2015 The Crashpad Authors. All rights
> reserved.
> The file naming is confusing, I think of “main.cc” as “the thing that
provides
> main()”, which is kind of a synonym for “entry_point.cc”. I’d do:
> 
> main.{cc,h} → handler_main.{cc,h}
> entry_point.cc → main.cc
> 
> https://codereview.chromium.org/1416873016/diff/20001/handler/main.h
> File handler/main.h (right):
> 
> https://codereview.chromium.org/1416873016/diff/20001/handler/main.h#newcode20
> handler/main.h:20: //! \brief The main() of the crashpad_handler binary.
> `main()`, `crashpad_handler`
> 
> https://codereview.chromium.org/1416873016/diff/20001/handler/main.h#newcode23
> handler/main.h:23: //! binary, but called and used in the same way.
> in the same way as…?

[scottmg, 2015-11-06 18:27:18]

https://codereview.chromium.org/1416873016/diff/20001/handler/entry_point.cc
File handler/entry_point.cc (right):

https://codereview.chromium.org/1416873016/diff/20001/handler/entry_point.cc#...
handler/entry_point.cc:1: // Copyright 2015 The Crashpad Authors. All rights
reserved.
On 2015/11/06 15:12:42, Mark Mentovai wrote:
> The file naming is confusing, I think of “main.cc” as “the thing that
provides
> main()”, which is kind of a synonym for “entry_point.cc”. I’d do:
> 
> main.{cc,h} → handler_main.{cc,h}
> entry_point.cc → main.cc

Done.

https://codereview.chromium.org/1416873016/diff/20001/handler/main.h
File handler/main.h (right):

https://codereview.chromium.org/1416873016/diff/20001/handler/main.h#newcode20
handler/main.h:20: //! \brief The main() of the crashpad_handler binary.
On 2015/11/06 15:12:42, Mark Mentovai wrote:
> `main()`, `crashpad_handler`

Done.

https://codereview.chromium.org/1416873016/diff/20001/handler/main.h#newcode23
handler/main.h:23: //! binary, but called and used in the same way.
On 2015/11/06 15:12:42, Mark Mentovai wrote:
> in the same way as…?

Better?

[Mark Mentovai, 2015-11-06 18:42:13]

LGTM

The opposite of --no-find-copies would have been good for #3, but I don’t need
to see that because HandlerMain() was good in #2.

https://codereview.chromium.org/1416873016/diff/40001/handler/handler_main.h
File handler/handler_main.h (right):

https://codereview.chromium.org/1416873016/diff/40001/handler/handler_main.h#...
handler/handler_main.h:23: //! binary, but called and used as if it were a
standalone binary.
standalone executable

[scottmg, 2015-11-06 18:49:44]

On 2015/11/06 18:42:13, Mark Mentovai wrote:
> LGTM
> 
> The opposite of --no-find-copies would have been good for #3, but I don’t need
> to see that because HandlerMain() was good in #2.

Oh, sorry. I added that to my local helper script because there's a problem
post-Blink merge where the find-copies option sometimes takes an extra ~30s when
uploading in the Chrome repo.

> 
> https://codereview.chromium.org/1416873016/diff/40001/handler/handler_main.h
> File handler/handler_main.h (right):
> 
>
https://codereview.chromium.org/1416873016/diff/40001/handler/handler_main.h#...
> handler/handler_main.h:23: //! binary, but called and used as if it were a
> standalone binary.
> standalone executable

[scottmg, 2015-11-06 18:49:48]

https://codereview.chromium.org/1416873016/diff/40001/handler/handler_main.h
File handler/handler_main.h (right):

https://codereview.chromium.org/1416873016/diff/40001/handler/handler_main.h#...
handler/handler_main.h:23: //! binary, but called and used as if it were a
standalone binary.
On 2015/11/06 18:42:13, Mark Mentovai wrote:
> standalone executable

Done.

[scottmg, 2015-11-06 18:52:15]

Committed patchset #4 (id:60001) manually as
20869d94682cb72b3e4544b47cc27059aa0af3ad (presubmit successful).