win: port multiprocess_exec_test_child.cc

win: port multiprocess_exec_test_child.cc

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

[scottmg, 2015-01-07 01:05:05]

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

[Mark Mentovai, 2015-01-08 17:20:20]

https://codereview.chromium.org/808493003/diff/20001/util/test/multiprocess_e...
File util/test/multiprocess_exec_test_child.cc (right):

https://codereview.chromium.org/808493003/diff/20001/util/test/multiprocess_e...
util/test/multiprocess_exec_test_child.cc:64: // Handles can theoretically go as
high as 2^32-4, but in practice they're
I thought Windows had an API to do this a little more gracefully than looping
over things and closing them (honestly, it’s kind of a hack on the POSIX side
too). Can’t we get the system to give us all of the handles that are available
in the process, along the lines of
http://nsylvain.blogspot.com/2007/09/how-list-all-open-handles.html?

https://codereview.chromium.org/808493003/diff/20001/util/test/multiprocess_e...
util/test/multiprocess_exec_test_child.cc:64: // Handles can theoretically go as
high as 2^32-4, but in practice they're
Blank line before the comment, to make it easier to spot that it’s a new
conceptual “thing.”

https://codereview.chromium.org/808493003/diff/20001/util/test/multiprocess_e...
util/test/multiprocess_exec_test_child.cc:70: continue;
{} me (the condition takes up more than one line, so the bracing helps make it
easier to follow through all that ragged indentation).

https://codereview.chromium.org/808493003/diff/20001/util/test/multiprocess_e...
util/test/multiprocess_exec_test_child.cc:71: if (CloseHandle(handle)) {
Can CloseHandle() fail for a reason other than an invalid handle?

https://codereview.chromium.org/808493003/diff/20001/util/util.gyp
File util/util.gyp (right):

https://codereview.chromium.org/808493003/diff/20001/util/util.gyp#newcode307
util/util.gyp:307:
'../third_party/mini_chromium/mini_chromium/base/base.gyp:base',
…but this executable isn’t using anything from base that I can see.

[scottmg, 2015-01-08 18:14:32]

Patchset #3 (id:40001) has been deleted

[scottmg, 2015-01-08 18:15:54]

https://codereview.chromium.org/808493003/diff/20001/util/test/multiprocess_e...
File util/test/multiprocess_exec_test_child.cc (right):

https://codereview.chromium.org/808493003/diff/20001/util/test/multiprocess_e...
util/test/multiprocess_exec_test_child.cc:64: // Handles can theoretically go as
high as 2^32-4, but in practice they're
On 2015/01/08 17:20:20, Mark Mentovai wrote:
> I thought Windows had an API to do this a little more gracefully than looping
> over things and closing them (honestly, it’s kind of a hack on the POSIX side
> too). Can’t we get the system to give us all of the handles that are available
> in the process, along the lines of
> http://nsylvain.blogspot.com/2007/09/how-list-all-open-handles.html

Sure, I actually used that at first based on
https://code.google.com/p/chromium/codesearch#chromium/src/sandbox/win/sandbo...,
but it seemed a bit heavy. Switched as it is a bit cleaner.

https://codereview.chromium.org/808493003/diff/20001/util/test/multiprocess_e...
util/test/multiprocess_exec_test_child.cc:70: continue;
On 2015/01/08 17:20:20, Mark Mentovai wrote:
> {} me (the condition takes up more than one line, so the bracing helps make it
> easier to follow through all that ragged indentation).

Removed.

https://codereview.chromium.org/808493003/diff/20001/util/test/multiprocess_e...
util/test/multiprocess_exec_test_child.cc:71: if (CloseHandle(handle)) {
On 2015/01/08 17:20:20, Mark Mentovai wrote:
> Can CloseHandle() fail for a reason other than an invalid handle?

I don't know of any reason, but there are so many types that I don't know for
sure. The other way should be more robust to that.

https://codereview.chromium.org/808493003/diff/20001/util/util.gyp
File util/util.gyp (right):

https://codereview.chromium.org/808493003/diff/20001/util/util.gyp#newcode307
util/util.gyp:307:
'../third_party/mini_chromium/mini_chromium/base/base.gyp:base',
On 2015/01/08 17:20:20, Mark Mentovai wrote:
> …but this executable isn’t using anything from base that I can see.

Was to get the include dir for build/build_config.h. Switched to low-level
predefined symbols in this target if you prefer that?

[Mark Mentovai, 2015-01-08 18:58:15]

LGTM

https://codereview.chromium.org/808493003/diff/60001/util/test/multiprocess_e...
File util/test/multiprocess_exec_test_child.cc (right):

https://codereview.chromium.org/808493003/diff/60001/util/test/multiprocess_e...
util/test/multiprocess_exec_test_child.cc:64: void EnsureOnlyStdioHandlesOpen()
{
Unnamed namespace for this.

https://codereview.chromium.org/808493003/diff/60001/util/test/multiprocess_e...
util/test/multiprocess_exec_test_child.cc:96: // the system between the size
query above, and the actual retrieval here.
Well, hopefully we’re single-threaded here.

[scottmg, 2015-01-08 19:07:43]

https://codereview.chromium.org/808493003/diff/60001/util/test/multiprocess_e...
File util/test/multiprocess_exec_test_child.cc (right):

https://codereview.chromium.org/808493003/diff/60001/util/test/multiprocess_e...
util/test/multiprocess_exec_test_child.cc:64: void EnsureOnlyStdioHandlesOpen()
{
On 2015/01/08 18:58:15, Mark Mentovai wrote:
> Unnamed namespace for this.

Done.

https://codereview.chromium.org/808493003/diff/60001/util/test/multiprocess_e...
util/test/multiprocess_exec_test_child.cc:96: // the system between the size
query above, and the actual retrieval here.
On 2015/01/08 18:58:15, Mark Mentovai wrote:
> Well, hopefully we’re single-threaded here.

Unfortunately, NtQuerySystemInformation has no way to filter by process at
retrieval time so we're retrieving handles for the entire system. If it's a
problem in practice, it might be sufficient to make the buffer arbitrarily
larger.

[scottmg, 2015-01-08 19:08:18]

Committed patchset #4 (id:80001) manually as
f9c487b1e1a365a8b2965367c3212b8f68d5b2f3 (presubmit successful).