[microdump] Add build fingerprint and product info metadata.

[microdump] Add build fingerprint and product info metadata.

This is to add build fingerprint and product name/version to
microdumps. Conversely to what happens in the case of minidumps
with MIME fields, due to the nature of minidumps, extra metadata
cannot be reliably injected after the dump is completed.
This CL adds the plumbing to inject two optional fields plus the
corresponding tests.

BUG=chromium:410294

[primiano]

Mark/Lei can I ask a review on this?
This cl is adding two strings to the microdump writer for the build/product info
(with all the joyful related plumbing)

I've realized that, due to the weird WebView release workflow, the module IDs
are not enough and we need the actual version numbers to to the proper routing
to the crash server.

I couldn't find a better plumbing other than using the MicrodumpDescriptor for
passing the two strings.

The complimentary code in chromium would be
https://codereview.chromium.org/1019663003/
that guarantees that the two strings will be stable (in global static storage).

Thanks

[Lei Zhang (chromium)]

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/handler/minidump...
File src/client/linux/handler/minidump_descriptor.h (right):

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/handler/minidump...
src/client/linux/handler/minidump_descriptor.h:110: void
SetMicrodumpBuildFingerprint(const char* build_fingerprint);
Do you see the need to do this separately from the ctor, or can the two set
methods be omitted if you just change the MicrodumpOnConsole version of the
ctor?

That would require a single CL to do the Breakpad roll + change on the Chromium
side, unless you temporarly have 2 MicrodumpOnConsole ctors.

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/microdump_writer...
File src/client/linux/microdump_writer/microdump_writer.h (right):

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/microdump_writer...
src/client/linux/microdump_writer/microdump_writer.h:49: //   build_fingerprint:
a (optional) C string which determines the product/OS
document |product_info| as well?

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/microdump_writer...
File src/client/linux/microdump_writer/microdump_writer_unittest.cc (right):

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/microdump_writer...
src/client/linux/microdump_writer/microdump_writer_unittest.cc:57: EXPECT_NE(-1,
pipe(fds));
Shouldn't we bail out with ASSERT_FOO like before on failures?

[primiano]

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/handler/minidump...
File src/client/linux/handler/minidump_descriptor.h (right):

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/handler/minidump...
src/client/linux/handler/minidump_descriptor.h:110: void
SetMicrodumpBuildFingerprint(const char* build_fingerprint);
On 2015/03/19 19:42:58, Lei Zhang (chromium) wrote:
> Do you see the need to do this separately from the ctor, or can the two set
> methods be omitted if you just change the MicrodumpOnConsole version of the
> ctor?
> 
> That would require a single CL to do the Breakpad roll + change on the
Chromium
> side, unless you temporarly have 2 MicrodumpOnConsole ctors.

Yeah the only reason for this was to make eventual reverts easy / not leave
breakpad in an unrollable state.
I'm happy with putting them in the ctor if that is you prefer.

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/microdump_writer...
File src/client/linux/microdump_writer/microdump_writer.h (right):

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/microdump_writer...
src/client/linux/microdump_writer/microdump_writer.h:49: //   build_fingerprint:
a (optional) C string which determines the product/OS
On 2015/03/19 19:42:58, Lei Zhang (chromium) wrote:
> document |product_info| as well?

Oh right.

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/microdump_writer...
File src/client/linux/microdump_writer/microdump_writer_unittest.cc (right):

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/microdump_writer...
src/client/linux/microdump_writer/microdump_writer_unittest.cc:57: EXPECT_NE(-1,
pipe(fds));
On 2015/03/19 19:42:58, Lei Zhang (chromium) wrote:
> Shouldn't we bail out with ASSERT_FOO like before on failures?

The problem is that once you are in a void helper function ASSERT generates code
that doesn't compile anymore (since it tries to return some gtest magic).
I don't know honestly a better alternative to keep the ASSERTS as such in a
helper fn. Or I can rewrite them as:
if (pipe() == -1)  { FAIL() << "pipe()"; return false}

and ASSERT_TRUE the call in the test fixture.

[Lei Zhang (chromium)]

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/handler/minidump...
File src/client/linux/handler/minidump_descriptor.h (right):

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/handler/minidump...
src/client/linux/handler/minidump_descriptor.h:110: void
SetMicrodumpBuildFingerprint(const char* build_fingerprint);
On 2015/03/19 20:14:39, primiano wrote:
> On 2015/03/19 19:42:58, Lei Zhang (chromium) wrote:
> > Do you see the need to do this separately from the ctor, or can the two set
> > methods be omitted if you just change the MicrodumpOnConsole version of the
> > ctor?
> > 
> > That would require a single CL to do the Breakpad roll + change on the
> Chromium
> > side, unless you temporarly have 2 MicrodumpOnConsole ctors.
> 
> Yeah the only reason for this was to make eventual reverts easy / not leave
> breakpad in an unrollable state.
> I'm happy with putting them in the ctor if that is you prefer.

Sure. We can certainly change this later once things stabilize.

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/microdump_writer...
File src/client/linux/microdump_writer/microdump_writer_unittest.cc (right):

https://breakpad.appspot.com/9794002/diff/1/src/client/linux/microdump_writer...
src/client/linux/microdump_writer/microdump_writer_unittest.cc:57: EXPECT_NE(-1,
pipe(fds));
On 2015/03/19 20:14:39, primiano wrote:
> On 2015/03/19 19:42:58, Lei Zhang (chromium) wrote:
> > Shouldn't we bail out with ASSERT_FOO like before on failures?
> 
> The problem is that once you are in a void helper function ASSERT generates
code
> that doesn't compile anymore (since it tries to return some gtest magic).
> I don't know honestly a better alternative to keep the ASSERTS as such in a
> helper fn. Or I can rewrite them as:
> if (pipe() == -1)  { FAIL() << "pipe()"; return false}
> 
> and ASSERT_TRUE the call in the test fixture.

It compiles fine here with ASSERTs with make
./src/client/linux/linux_client_unittest