Add MinidumpModuleWriter, MinidumpModuleListWriter, related classes, and their test

Add MinidumpModuleWriter, MinidumpModuleListWriter, related classes, and
their test.

TEST=minidump_test MinidumpModuleTest
R=rsesek@chromium.org

Committed: https://chromium.googlesource.com/crashpad/crashpad/+/0ba9d2b06546

[Robert Sesek, 2014-08-12 22:56:06]

https://codereview.chromium.org/460933004/diff/1/minidump/minidump_module_wri...
File minidump/minidump_module_writer.h (right):

https://codereview.chromium.org/460933004/diff/1/minidump/minidump_module_wri...
minidump/minidump_module_writer.h:109: //! \brief The writer for a
MinidumpModuleCodeViewRecordPDB20 object in a
MinidumpModuleCodeViewRecordPDB70

https://codereview.chromium.org/460933004/diff/1/minidump/minidump_module_wri...
File minidump/minidump_module_writer_test.cc (right):

https://codereview.chromium.org/460933004/diff/1/minidump/minidump_module_wri...
minidump/minidump_module_writer_test.cc:47: VerifyMinidumpHeader(header, 1, 0);
IWYU

https://codereview.chromium.org/460933004/diff/1/minidump/minidump_module_wri...
minidump/minidump_module_writer_test.cc:105: const char* expected_module_name,
Why const char* for the expected names instead of const std::string& ?

https://codereview.chromium.org/460933004/diff/1/minidump/minidump_module_wri...
minidump/minidump_module_writer_test.cc:160: if (expected_pdb_name) {
If this weren't in a test, this is where I'd tell you to split your function up…

https://codereview.chromium.org/460933004/diff/1/minidump/minidump_module_wri...
minidump/minidump_module_writer_test.cc:624: 
I assume you can mix and match modules with different CodeView types, but that's
not tested here. It's probably not significant to warrant a test.

Also, you don't test what happens when dwFileFlags contains unmasked flags, but
that's a DCHECK,s o also probably not necessary.

[Mark Mentovai, 2014-08-12 23:23:26]

https://codereview.chromium.org/460933004/diff/1/minidump/minidump_module_wri...
File minidump/minidump_module_writer_test.cc (right):

https://codereview.chromium.org/460933004/diff/1/minidump/minidump_module_wri...
minidump/minidump_module_writer_test.cc:105: const char* expected_module_name,
rsesek wrote:
> Why const char* for the expected names instead of const std::string& ?

So that they can be NULL to indicate that the MinidumpWritable should be absent.

expected_module_name is required, so I’ll make that a const std::string&, but
the others should remain pointers.

https://codereview.chromium.org/460933004/diff/1/minidump/minidump_module_wri...
minidump/minidump_module_writer_test.cc:160: if (expected_pdb_name) {
rsesek wrote:
> If this weren't in a test, this is where I'd tell you to split your function
up…

I split it up anyway.

https://codereview.chromium.org/460933004/diff/1/minidump/minidump_module_wri...
minidump/minidump_module_writer_test.cc:624: 
rsesek wrote:
> I assume you can mix and match modules with different CodeView types, but
that's
> not tested here. It's probably not significant to warrant a test.

I changed the ThreeModules test to do this.

> Also, you don't test what happens when dwFileFlags contains unmasked flags,
but
> that's a DCHECK,s o also probably not necessary.

Yeah, I’m not generally writing death tests for incidental DCHECKs, because the
DCHECK itself is what’s intended to catch bad code, not legitimate uses, and the
tests would need to be #if DCHECK_IS_ON. I’ll do death tests for CHECKs, though.

The exception is the review request I just sent you. Since its whole purpose is
to DCHECK, it needed death tests.

[Robert Sesek, 2014-08-13 19:57:52]

LGTM

[Mark Mentovai, 2014-08-13 22:26:25]

Committed patchset #3 manually as 0ba9d2b06546 (presubmit successful).