Refactor the Windows MinidumpGenerator interface to get rid of the overloads when generating dumps.

Refactor the Windows MinidumpGenerator interface to get rid of the overloads
when generating dumps.
All required params are now passed to the constructor and the various options
are set through new methods.

BUG=N/A
TEST=Existing minidump generation tests
R=mark@chromium.org

Committed: https://code.google.com/p/google-breakpad/source/detail?r=1274

[Mark Mentovai]

LGTM

Are APIs that are used in Chrome changing? If so, could you please roll the
Breakpad DEPS in Chrome after this lands in Breakpad and adapt whatever needs to
be adapted?

https://breakpad.appspot.com/1074002/diff/1/client/windows/crash_generation/m...
File client/windows/crash_generation/minidump_generator.h (right):

https://breakpad.appspot.com/1074002/diff/1/client/windows/crash_generation/m...
client/windows/crash_generation/minidump_generator.h:47: // Creates an instance
with the given dump path.
This comment needs to be expanded now. What do the arguments mean? Which ones
are required and which can be NULL?

[Cris Neckar]

On 2014/01/17 21:58:31, Mark Mentovai wrote:
> LGTM
> 
> Are APIs that are used in Chrome changing? If so, could you please roll the
> Breakpad DEPS in Chrome after this lands in Breakpad and adapt whatever needs
to
> be adapted?
> 
>
https://breakpad.appspot.com/1074002/diff/1/client/windows/crash_generation/m...
> File client/windows/crash_generation/minidump_generator.h (right):
> 
>
https://breakpad.appspot.com/1074002/diff/1/client/windows/crash_generation/m...
> client/windows/crash_generation/minidump_generator.h:47: // Creates an
instance
> with the given dump path.
> This comment needs to be expanded now. What do the arguments mean? Which ones
> are required and which can be NULL?

It should only impact Omaha. Chrome just calls MiniDumpWriteDump directly when
it handles dumps in process. 

I have plans for Omaha so I'm handling the changes there.

[Mark Mentovai]

Excellent. LGTM, then.

I thought I’d have suggestions, but as I read through the code, I dropped them,
because the later code explained some of the decisions you made that I had read
earlier.

[Cris Neckar]

Committed patchset #2 manually as r1274 (presubmit successful).

[Cris Neckar]

https://breakpad.appspot.com/1074002/diff/1/client/windows/crash_generation/m...
File client/windows/crash_generation/minidump_generator.h (right):

https://breakpad.appspot.com/1074002/diff/1/client/windows/crash_generation/m...
client/windows/crash_generation/minidump_generator.h:47: // Creates an instance
with the given dump path.
On 2014/01/17 21:58:32, Mark Mentovai wrote:
> This comment needs to be expanded now. What do the arguments mean? Which ones
> are required and which can be NULL?

Done.

[Lei Zhang (chromium)]

https://breakpad.appspot.com/1074002/diff/60001/client/windows/crash_generati...
File client/windows/crash_generation/crash_generation_server.cc (right):

https://breakpad.appspot.com/1074002/diff/60001/client/windows/crash_generati...
client/windows/crash_generation/crash_generation_server.cc:124:
dump_path_(*dump_path),
|dump_path| can be NULL, as seen by the fact that the old code checks for it.

BreakpadWinDeathTest.TestAccessViolation in remoting_unittests crashes here
with:

Backtrace:
	std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t>
>::assign [0x00871CBE+14]
	google_breakpad::CrashGenerationServer::CrashGenerationServer [0x00F05A0B+203]
	remoting::BreakpadWinDeathTest::SetUp [0x0087401E+990]

I'll fix this...