MinidumpLocationDescriptorListWriter

minidump: Refactor MinidumpCrashpadInfoListWriter into a base class,
MinidumpLocationDescriptorListWriter. This allows easier construction of
minidump structures that are lists of MINIDUMP_LOCATION_DESCRIPTOR
structures in the future.

TEST=minidump_test
R=rsesek@chromium.org

Committed: https://chromium.googlesource.com/crashpad/crashpad/+/20032b5ddf8e58ab0a3a1d7c46dc30639c4b547b

[Mark Mentovai, 2014-11-05 17:29:02]

mark@chromium.org changed reviewers:
+ rsesek@chromium.org

[Mark Mentovai, 2014-11-05 17:29:02]

https://codereview.chromium.org/707543002/diff/1/minidump/minidump_location_d...
File minidump/minidump_location_descriptor_list_writer.h (right):

https://codereview.chromium.org/707543002/diff/1/minidump/minidump_location_d...
minidump/minidump_location_descriptor_list_writer.h:44: //! To provide
type-correctness, subclasses are expected to provide a public
Another alternative was templatizing this class, but that doesn’t seem to offer
much of a benefit.

[Mark Mentovai, 2014-11-06 19:23:46]

Updated to improve sharing in the tests.

[Robert Sesek, 2014-11-06 19:44:28]

https://codereview.chromium.org/707543002/diff/40001/minidump/minidump_locati...
File minidump/minidump_location_descriptor_list_writer.h (right):

https://codereview.chromium.org/707543002/diff/40001/minidump/minidump_locati...
minidump/minidump_location_descriptor_list_writer.h:53: const
PointerVector<MinidumpWritable>& children() const { return children_; }
Could you return this as a std::vector instead? Yes the PointerVector is by
reference, but that name usually confers ownership semantics, so it's a bit odd.

https://codereview.chromium.org/707543002/diff/60001/minidump/minidump_locati...
File minidump/minidump_location_descriptor_list_writer_test.cc (right):

https://codereview.chromium.org/707543002/diff/60001/minidump/minidump_locati...
minidump/minidump_location_descriptor_list_writer_test.cc:60: void
AddChild(scoped_ptr<TestMinidumpWritable> child) {
Nothing is done with the parameter after it is passed as an argument here, so
how about rename this to AddChildValue(uint32_t) and then that will create the
TestMinidumpWritable ?

[Mark Mentovai, 2014-11-06 20:37:05]

Updated.

https://codereview.chromium.org/707543002/diff/40001/minidump/minidump_locati...
File minidump/minidump_location_descriptor_list_writer.h (right):

https://codereview.chromium.org/707543002/diff/40001/minidump/minidump_locati...
minidump/minidump_location_descriptor_list_writer.h:53: const
PointerVector<MinidumpWritable>& children() const { return children_; }
Robert Sesek wrote:
> Could you return this as a std::vector instead? Yes the PointerVector is by
> reference, but that name usually confers ownership semantics, so it's a bit
odd.

I could return a const vector*, but not a const vector&.

I could also return a regular vector by copying everything into it, but that
sucks.

The only thing that callers ever do with this is test emptiness, so I’ll just
provide a higher-level empty().

[Robert Sesek, 2014-11-06 21:36:57]

LGTM

[Mark Mentovai, 2014-11-06 21:58:40]

Committed patchset #5 (id:80001) manually as
20032b5ddf8e58ab0a3a1d7c46dc30639c4b547b (presubmit successful).