Don’t log an error when creating a new crash report database

Don’t log an error when creating a new crash report database

Previously, any attempt to create a new crash report database would
result in this message being logged:

[p:t:yyyymmdd,hhmmss.uuuuuu:ERROR file_io.cc:30] read: expected 40,
observed 0

This would be the first thing that a developer embedding Crashpad into
their application would see after getting everything right. It doesn’t
exactly seem like everything’s right with that being logged. It would
also be the first thing that a user would see on stderr or in logs upon
launching a Crashpad-enabled application, which also seems kind of
dodgy.

The crash report database settings creation logic is restructured to
avoid logging this error when definitely creating a new database, while
retaining all other error logging.

BUG=crashpad:63
TEST=crashpad_database_util --database $new_db --show-client-id
     (should not show any errors)

R=rsesek@chromium.org, scottmg@chromium.org

Committed: https://chromium.googlesource.com/crashpad/crashpad/+/0884d4d3a8c283a0f9711494e39dd6c5c4a6c1b0

[Mark Mentovai, 2015-10-07 16:17:28]

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

[scottmg, 2015-10-07 17:28:33]

lgtm

https://codereview.chromium.org/1392953002/diff/1/client/settings.h
File client/settings.h (right):

https://codereview.chromium.org/1392953002/diff/1/client/settings.h#newcode146
client/settings.h:146: // file.
Maybe add a comment here that |handle| is expected to be locked, if valid (and
below).

[Robert Sesek, 2015-10-07 17:35:51]

LGTM

https://codereview.chromium.org/1392953002/diff/1/client/settings.cc
File client/settings.cc (right):

https://codereview.chromium.org/1392953002/diff/1/client/settings.cc#newcode166
client/settings.cc:166: DCHECK(mode != FileWriteMode::kTruncateOrCreate);
DCHECK_NE ?

[Mark Mentovai, 2015-10-07 17:50:09]

https://codereview.chromium.org/1392953002/diff/1/client/settings.cc
File client/settings.cc (right):

https://codereview.chromium.org/1392953002/diff/1/client/settings.cc#newcode166
client/settings.cc:166: DCHECK(mode != FileWriteMode::kTruncateOrCreate);
Robert Sesek wrote:
> DCHECK_NE ?

In file included from ../../client/settings.cc:19:
../../third_party/mini_chromium/mini_chromium/base/logging.h:66:23: error:
invalid operands to binary expression ('basic_ostream<char,
std::__1::char_traits<char> >' and 'crashpad::FileWriteMode')
  ss << names << " (" << v1 << " vs. " << v2 << ")";
  ~~~~~~~~~~~~~~~~~~~ ^  ~~

enum classes don’t automatically convert to anything that can be streamed. I
could use DCHECK_NE with a cast to int, but that didn’t seem prudent or
worthwhile.

https://codereview.chromium.org/1392953002/diff/1/client/settings.h
File client/settings.h (right):

https://codereview.chromium.org/1392953002/diff/1/client/settings.h#newcode146
client/settings.h:146: // file.
scottmg wrote:
> Maybe add a comment here that |handle| is expected to be locked, if valid (and
> below).

Done, sorta. I borrowed the verbiage from RecoverSettings().

[scottmg, 2015-10-07 17:54:27]

lgtm

https://codereview.chromium.org/1392953002/diff/20001/client/settings.h
File client/settings.h (right):

https://codereview.chromium.org/1392953002/diff/20001/client/settings.h#newco...
client/settings.h:143: // |handle| must the result of OpenForReading() or
OpenForReadingAndWriting().
must -> must be

[Mark Mentovai, 2015-10-07 18:04:27]

https://codereview.chromium.org/1392953002/diff/20001/client/settings.h
File client/settings.h (right):

https://codereview.chromium.org/1392953002/diff/20001/client/settings.h#newco...
client/settings.h:143: // |handle| must the result of OpenForReading() or
OpenForReadingAndWriting().
scottmg wrote:
> must -> must be

Thanks, fixed.

[Mark Mentovai, 2015-10-07 20:20:34]

Committed patchset #3 (id:40001) manually as
0884d4d3a8c283a0f9711494e39dd6c5c4a6c1b0 (presubmit successful).