win: Make reading CrashpadInfo work across bitness

win: Make reading CrashpadInfo work across bitness

R=mark@chromium.org
BUG=crashpad:50
Committed: https://chromium.googlesource.com/crashpad/crashpad/+/bd9bc07625444e5e38597c116c8f3f1459798ecd

[scottmg, 2015-09-21 21:44:47]

https://codereview.chromium.org/1355503005/diff/1/client/crashpad_info.h
File client/crashpad_info.h (right):

https://codereview.chromium.org/1355503005/diff/1/client/crashpad_info.h#newc...
client/crashpad_info.h:124: #if !defined(NDEBUG) && defined(OS_WIN)
This is necessary to make the new PEImageAnnotationsReader.*WOW64 tests fail
without the fixed code, because otherwise it (normally) just reads a 0 dword for
the top half of the pointer when reading from the x86 process, so everything
works fine.

But it's ugly, so up to you if you think it's valuable or just confusing to have
here.

[scottmg, 2015-09-21 22:06:24]

[Fixed TODO added in ps#1 in ps#2, in case you already started to look.]

[Mark Mentovai, 2015-09-22 13:17:29]

LGTM

https://codereview.chromium.org/1355503005/diff/40001/snapshot/win/crashpad_s...
File snapshot/win/crashpad_snapshot_test_simple_annotations.cc (right):

https://codereview.chromium.org/1355503005/diff/40001/snapshot/win/crashpad_s...
snapshot/win/crashpad_snapshot_test_simple_annotations.cc:54: 
Lose the blank lines at EOF.

https://codereview.chromium.org/1355503005/diff/40001/snapshot/win/pe_image_a...
File snapshot/win/pe_image_annotations_reader_test.cc (right):

https://codereview.chromium.org/1355503005/diff/40001/snapshot/win/pe_image_a...
snapshot/win/pe_image_annotations_reader_test.cc:70:
ProcessSuspensionState::kRunning));
Spotted a tab here.

https://codereview.chromium.org/1355503005/diff/40001/snapshot/win/pe_image_a...
snapshot/win/pe_image_annotations_reader_test.cc:105: ADD_FAILURE();
Should this be FAIL() instead to cause an immediate return (like ASSERT_*()?)

https://codereview.chromium.org/1355503005/diff/40001/snapshot/win/pe_image_a...
snapshot/win/pe_image_annotations_reader_test.cc:110: switch (type) {
Optional: instead of having two switches on the same thing, you could have just
the one above, also responsible for setting DWORD expected_exit_code.

https://codereview.chromium.org/1355503005/diff/40001/test/win/child_launcher.cc
File test/win/child_launcher.cc (right):

https://codereview.chromium.org/1355503005/diff/40001/test/win/child_launcher...
test/win/child_launcher.cc:42: // Create a pipe for the stdin/stdout of the
child.
Now this is plural “pipes.”

https://codereview.chromium.org/1355503005/diff/40001/test/win/child_launcher...
test/win/child_launcher.cc:93: DWORD exit_code;
Initialize this so that if GetExitCodeProcess() falis, you return something
deterministic.

[scottmg, 2015-09-22 17:26:00]

Thanks

https://codereview.chromium.org/1355503005/diff/40001/snapshot/win/crashpad_s...
File snapshot/win/crashpad_snapshot_test_simple_annotations.cc (right):

https://codereview.chromium.org/1355503005/diff/40001/snapshot/win/crashpad_s...
snapshot/win/crashpad_snapshot_test_simple_annotations.cc:54: 
On 2015/09/22 13:17:29, Mark Mentovai - August is over wrote:
> Lose the blank lines at EOF.

Done.

https://codereview.chromium.org/1355503005/diff/40001/snapshot/win/pe_image_a...
File snapshot/win/pe_image_annotations_reader_test.cc (right):

https://codereview.chromium.org/1355503005/diff/40001/snapshot/win/pe_image_a...
snapshot/win/pe_image_annotations_reader_test.cc:70:
ProcessSuspensionState::kRunning));
On 2015/09/22 13:17:29, Mark Mentovai - August is over wrote:
> Spotted a tab here.

Gah, new OS install and haven't reconfigured everything yet (apparently). Done.

https://codereview.chromium.org/1355503005/diff/40001/snapshot/win/pe_image_a...
snapshot/win/pe_image_annotations_reader_test.cc:105: ADD_FAILURE();
On 2015/09/22 13:17:29, Mark Mentovai - August is over wrote:
> Should this be FAIL() instead to cause an immediate return (like ASSERT_*()?)

Done.

https://codereview.chromium.org/1355503005/diff/40001/snapshot/win/pe_image_a...
snapshot/win/pe_image_annotations_reader_test.cc:110: switch (type) {
On 2015/09/22 13:17:29, Mark Mentovai - August is over wrote:
> Optional: instead of having two switches on the same thing, you could have
just
> the one above, also responsible for setting DWORD expected_exit_code.

Done.

https://codereview.chromium.org/1355503005/diff/40001/test/win/child_launcher.cc
File test/win/child_launcher.cc (right):

https://codereview.chromium.org/1355503005/diff/40001/test/win/child_launcher...
test/win/child_launcher.cc:42: // Create a pipe for the stdin/stdout of the
child.
On 2015/09/22 13:17:29, Mark Mentovai - August is over wrote:
> Now this is plural “pipes.”

Done.

https://codereview.chromium.org/1355503005/diff/40001/test/win/child_launcher...
test/win/child_launcher.cc:93: DWORD exit_code;
On 2015/09/22 13:17:29, Mark Mentovai - August is over wrote:
> Initialize this so that if GetExitCodeProcess() falis, you return something
> deterministic.

Done.

[scottmg, 2015-09-22 17:37:18]

Committed patchset #4 (id:60001) manually as
bd9bc07625444e5e38597c116c8f3f1459798ecd (presubmit successful).