Add basic crash reporting to delegate_execute.exe.

Add basic crash reporting to delegate_execute.exe.

BUG=160658
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=169304

[grt (UTC plus 2), 2012-11-21 19:27:50]

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
File win8/delegate_execute/crash_server_init.cc (right):

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
win8/delegate_execute/crash_server_init.cc:14: 
#include "client/windows/handler/exception_handler.h"
(omit "breakpad/src" since it is in the list of include dirs thanks to
breakpad_handler dependency.)

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
win8/delegate_execute/crash_server_init.cc:49:
google_breakpad::CustomClientInfo* GetCustomInfo() {
move this into the unnamed namespace

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
File win8/delegate_execute/crash_server_init.h (right):

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
win8/delegate_execute/crash_server_init.h:1: // Copyright (c) 2011 The Chromium
Authors. All rights reserved.
2012

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
win8/delegate_execute/crash_server_init.h:8: #include
"breakpad/src/client/windows/handler/exception_handler.h"
forward decl instead of include:
namespace google_breakpad {
class ExceptionHandler;
}

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
win8/delegate_execute/crash_server_init.h:10: // Initializes breakpad crash
reporting and returns a pointer to a newly
put this in the delegate_execute namespace.

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/delegat...
File win8/delegate_execute/delegate_execute.gyp (right):

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/delegat...
win8/delegate_execute/delegate_execute.gyp:49: 'include_dirs': [
remove this and instead add a dependency to
../../breakpad/breakpad.gyp:breakpad_handler above?

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/delegat...
win8/delegate_execute/delegate_execute.gyp:55: 'crash_server_init.cc',
sort

[robertshield, 2012-11-21 21:09:15]

Thanks! PTAL

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
File win8/delegate_execute/crash_server_init.cc (right):

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
win8/delegate_execute/crash_server_init.cc:14: 
On 2012/11/21 19:27:50, grt wrote:
> #include "client/windows/handler/exception_handler.h"
> (omit "breakpad/src" since it is in the list of include dirs thanks to
> breakpad_handler dependency.)

Done.

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
win8/delegate_execute/crash_server_init.cc:49:
google_breakpad::CustomClientInfo* GetCustomInfo() {
On 2012/11/21 19:27:50, grt wrote:
> move this into the unnamed namespace

Done.

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
File win8/delegate_execute/crash_server_init.h (right):

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
win8/delegate_execute/crash_server_init.h:1: // Copyright (c) 2011 The Chromium
Authors. All rights reserved.
On 2012/11/21 19:27:50, grt wrote:
> 2012

Done.

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
win8/delegate_execute/crash_server_init.h:8: #include
"breakpad/src/client/windows/handler/exception_handler.h"
On 2012/11/21 19:27:50, grt wrote:
> forward decl instead of include:
> namespace google_breakpad {
> class ExceptionHandler;
> }

Done.

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/crash_s...
win8/delegate_execute/crash_server_init.h:10: // Initializes breakpad crash
reporting and returns a pointer to a newly
On 2012/11/21 19:27:50, grt wrote:
> put this in the delegate_execute namespace.

Done.

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/delegat...
File win8/delegate_execute/delegate_execute.gyp (right):

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/delegat...
win8/delegate_execute/delegate_execute.gyp:49: 'include_dirs': [
On 2012/11/21 19:27:50, grt wrote:
> remove this and instead add a dependency to
> ../../breakpad/breakpad.gyp:breakpad_handler above?

Done.

https://codereview.chromium.org/11421031/diff/7/win8/delegate_execute/delegat...
win8/delegate_execute/delegate_execute.gyp:55: 'crash_server_init.cc',
On 2012/11/21 19:27:50, grt wrote:
> sort

huh, you'd think after hours of http://www.youtube.com/watch?v=BELlZKpi1Zs, I'd
know that o comes before r.

[grt (UTC plus 2), 2012-11-22 02:32:35]

lgtm w/ one optional modernization change if you have the energy for it.

https://codereview.chromium.org/11421031/diff/12003/win8/delegate_execute/cra...
File win8/delegate_execute/crash_server_init.cc (right):

https://codereview.chromium.org/11421031/diff/12003/win8/delegate_execute/cra...
win8/delegate_execute/crash_server_init.cc:88:
google_breakpad::ExceptionHandler* breakpad =
come to think of it, this should use the new movable machinery of scoped_ptr.
please change this to:
  return scoped_ptr<google_breakpad::ExceptionHandler>(new
google_breakpad::ExceptionHandler(...));
and make the other corresponding changes so that this function looks like the
CreateFoo() example in scoped_ptr.h.  thanks.

[robertshield, 2012-11-22 15:58:51]

On 2012/11/22 02:32:35, grt wrote:
> lgtm w/ one optional modernization change if you have the energy for it.
> 
>
https://codereview.chromium.org/11421031/diff/12003/win8/delegate_execute/cra...
> File win8/delegate_execute/crash_server_init.cc (right):
> 
>
https://codereview.chromium.org/11421031/diff/12003/win8/delegate_execute/cra...
> win8/delegate_execute/crash_server_init.cc:88:
> google_breakpad::ExceptionHandler* breakpad =
> come to think of it, this should use the new movable machinery of scoped_ptr.
> please change this to:
>   return scoped_ptr<google_breakpad::ExceptionHandler>(new
> google_breakpad::ExceptionHandler(...));
> and make the other corresponding changes so that this function looks like the
> CreateFoo() example in scoped_ptr.h.  thanks.

Done, thanks. PTAL if you feel like it.

[grt (UTC plus 2), 2012-11-22 16:15:02]

win_rel has been failing due to the #includes in the new .cc file. i think my
suggestions below will resolve all iwyu issues with the file.

https://chromiumcodereview.appspot.com/11421031/diff/5005/win8/delegate_execu...
File win8/delegate_execute/crash_server_init.cc (right):

https://chromiumcodereview.appspot.com/11421031/diff/5005/win8/delegate_execu...
win8/delegate_execute/crash_server_init.cc:7: #include <sddl.h>
this is causing problems on win_rel. i don't see that it's being used here now
that base::win::GetUserSidString is being used, so just remove it?

https://chromiumcodereview.appspot.com/11421031/diff/5005/win8/delegate_execu...
win8/delegate_execute/crash_server_init.cc:9: #include <stdlib.h>
remove this

https://chromiumcodereview.appspot.com/11421031/diff/5005/win8/delegate_execu...
win8/delegate_execute/crash_server_init.cc:30:
GetModuleFileName(reinterpret_cast<HMODULE>(&__ImageBase),
#include <windows.h> for this

https://chromiumcodereview.appspot.com/11421031/diff/5005/win8/delegate_execu...
win8/delegate_execute/crash_server_init.cc:39: if (wcsstr(exe_path,
program_files_path) == exe_path) {
#include <cwchar> for this

https://chromiumcodereview.appspot.com/11421031/diff/5005/win8/delegate_execu...
win8/delegate_execute/crash_server_init.cc:70: DWORD path_len =
::GetTempPath(MAX_PATH, temp_path);
#include <windows.h> for this

[robertshield, 2012-11-22 18:18:28]

https://chromiumcodereview.appspot.com/11421031/diff/5005/win8/delegate_execu...
File win8/delegate_execute/crash_server_init.cc (right):

https://chromiumcodereview.appspot.com/11421031/diff/5005/win8/delegate_execu...
win8/delegate_execute/crash_server_init.cc:7: #include <sddl.h>
On 2012/11/22 16:15:02, grt wrote:
> this is causing problems on win_rel. i don't see that it's being used here now
> that base::win::GetUserSidString is being used, so just remove it?

Done.

https://chromiumcodereview.appspot.com/11421031/diff/5005/win8/delegate_execu...
win8/delegate_execute/crash_server_init.cc:9: #include <stdlib.h>
On 2012/11/22 16:15:02, grt wrote:
> remove this

Done.

https://chromiumcodereview.appspot.com/11421031/diff/5005/win8/delegate_execu...
win8/delegate_execute/crash_server_init.cc:30:
GetModuleFileName(reinterpret_cast<HMODULE>(&__ImageBase),
On 2012/11/22 16:15:02, grt wrote:
> #include <windows.h> for this

Done.

https://chromiumcodereview.appspot.com/11421031/diff/5005/win8/delegate_execu...
win8/delegate_execute/crash_server_init.cc:39: if (wcsstr(exe_path,
program_files_path) == exe_path) {
On 2012/11/22 16:15:02, grt wrote:
> #include <cwchar> for this

Done.

https://chromiumcodereview.appspot.com/11421031/diff/5005/win8/delegate_execu...
win8/delegate_execute/crash_server_init.cc:70: DWORD path_len =
::GetTempPath(MAX_PATH, temp_path);
On 2012/11/22 16:15:02, grt wrote:
> #include <windows.h> for this

Done.

[commit-bot: I haz the power, 2012-11-22 18:44:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robertshield@chromium.org/11421031/7007

[commit-bot: I haz the power, 2012-11-22 20:34:41]

Change committed as 169304