Add a flag to print the handles held by a child process when it shuts down.

Add a flag to print the handles held by a child process when it shuts down.
This will allow us to make tests that audit for extra handles and permissions being added to sandboxed processes.

BUG=85212
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=88602

[cpu_(ooo_6.6-7.5), 2011-06-07 21:18:00]

http://codereview.chromium.org/7054080/diff/13002/content/browser/handle_enum...
File content/browser/handle_enumerator_win.h (right):

http://codereview.chromium.org/7054080/diff/13002/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:17: 
we should use either src/sandbox/tools/finder/ntundoc.h or <winternl.h>, feel
free to enhance ntundoc.h as needed.

[Cris Neckar, 2011-06-07 22:10:09]

http://codereview.chromium.org/7054080/diff/13002/content/browser/handle_enum...
File content/browser/handle_enumerator_win.h (right):

http://codereview.chromium.org/7054080/diff/13002/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:17: 
On 2011/06/07 21:18:00, cpu wrote:
> we should use either src/sandbox/tools/finder/ntundoc.h or <winternl.h>, feel
> free to enhance ntundoc.h as needed.

Done.

[cpu_(ooo_6.6-7.5), 2011-06-08 02:23:28]

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
File content/browser/handle_enumerator_win.cc (right):

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.cc:48: 
leaking handle_info ?

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.cc:162: string16
HandleEnumerator::ProcessTypeString(
it seems this not need to be a member function of the class

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.cc:233: ACCESS_MASK access) {
function does not seem to need be member of the class.

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
File content/browser/handle_enumerator_win.h (right):

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:5: #ifndef
BASE_HANDLE_ENUMERATION_WIN_H_
the header guards are not to chrome standard

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:14: #include "build/build_config.h"
some of these headers seem not necessary, like build config and the
render_process_host, etc. Find the minimal set and move the others to the cc
file.

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:19: const wchar_t kNtdllDllName[] =
L"ntdll.dll";
can this constant and the next move to the cc file?

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:22: typedef enum _HANDLE_TYPE {
c++ does not need the typedef thing and the _HANDLE_TYPE can be dropped.

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:54: ChildProcessInfo::ProcessType type()
{ return type_; }
this function and the next can be const

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:62: static HandleType
StringToHandleType(string16 type);
const string16& type ?

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:79: };
DISALLOW_COPY_AND_ASSIGN

[Cris Neckar, 2011-06-08 18:39:21]

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
File content/browser/handle_enumerator_win.cc (right):

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.cc:48: 
On 2011/06/08 02:23:28, cpu wrote:
> leaking handle_info ?

Done.

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.cc:162: string16
HandleEnumerator::ProcessTypeString(
On 2011/06/08 02:23:28, cpu wrote:
> it seems this not need to be a member function of the class

Done.

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.cc:233: ACCESS_MASK access) {
On 2011/06/08 02:23:28, cpu wrote:
> function does not seem to need be member of the class.

Done.

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
File content/browser/handle_enumerator_win.h (right):

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:5: #ifndef
BASE_HANDLE_ENUMERATION_WIN_H_
On 2011/06/08 02:23:28, cpu wrote:
> the header guards are not to chrome standard

Done.

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:14: #include "build/build_config.h"
On 2011/06/08 02:23:28, cpu wrote:
> some of these headers seem not necessary, like build config and the
> render_process_host, etc. Find the minimal set and move the others to the cc
> file. 

Done.

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:19: const wchar_t kNtdllDllName[] =
L"ntdll.dll";
On 2011/06/08 02:23:28, cpu wrote:
> can this constant and the next move to the cc file?

Done.

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:22: typedef enum _HANDLE_TYPE {
On 2011/06/08 02:23:28, cpu wrote:
> c++ does not need the typedef thing and the _HANDLE_TYPE can be dropped.

Done.

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:54: ChildProcessInfo::ProcessType type()
{ return type_; }
On 2011/06/08 02:23:28, cpu wrote:
> this function and the next can be const

Done.

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:62: static HandleType
StringToHandleType(string16 type);
On 2011/06/08 02:23:28, cpu wrote:
> const string16& type ?

Done.

http://codereview.chromium.org/7054080/diff/10003/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:79: };
On 2011/06/08 02:23:28, cpu wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

[Cris Neckar, 2011-06-08 19:21:54]

I split this into 2 flags to make it easier to see the important stuff. One will
print all handles (like the original) and the other will only print a select
group (by my judgement the bad stuff). Let me know if you think anything should
be added/removed from the minimized list.

[cpu_(ooo_6.6-7.5), 2011-06-08 21:16:02]

lgtm

http://codereview.chromium.org/7054080/diff/10006/sandbox/tools/finder/ntundoc.h
File sandbox/tools/finder/ntundoc.h (right):

http://codereview.chromium.org/7054080/diff/10006/sandbox/tools/finder/ntundo...
sandbox/tools/finder/ntundoc.h:2: // Use of this source code is governed by a
BSD-style license that can be
fix copyright year

[jam, 2011-06-08 22:06:38]

http://codereview.chromium.org/7054080/diff/10006/content/browser/child_proce...
File content/browser/child_process_launcher.cc (right):

http://codereview.chromium.org/7054080/diff/10006/content/browser/child_proce...
content/browser/child_process_launcher.cc:23: #include
"chrome/common/chrome_switches.h"
nit: don't add this to chrome_switches if it's used in content.  it should be in
content_switches. (content shouldn't include stuff from chrome, we're working on
reducing that to 0)

http://codereview.chromium.org/7054080/diff/10006/content/browser/handle_enum...
File content/browser/handle_enumerator_win.h (right):

http://codereview.chromium.org/7054080/diff/10006/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:14: enum HandleType {
please add put this whole file inside the content namespace (new code in content
should be in that namespace, sorry I know this is not well advertised)

[Cris Neckar, 2011-06-09 18:20:13]

http://codereview.chromium.org/7054080/diff/10006/content/browser/child_proce...
File content/browser/child_process_launcher.cc (right):

http://codereview.chromium.org/7054080/diff/10006/content/browser/child_proce...
content/browser/child_process_launcher.cc:23: #include
"chrome/common/chrome_switches.h"
On 2011/06/08 22:06:38, John Abd-El-Malek wrote:
> nit: don't add this to chrome_switches if it's used in content.  it should be
in
> content_switches. (content shouldn't include stuff from chrome, we're working
on
> reducing that to 0)

Done.

http://codereview.chromium.org/7054080/diff/10006/content/browser/handle_enum...
File content/browser/handle_enumerator_win.h (right):

http://codereview.chromium.org/7054080/diff/10006/content/browser/handle_enum...
content/browser/handle_enumerator_win.h:14: enum HandleType {
On 2011/06/08 22:06:38, John Abd-El-Malek wrote:
> please add put this whole file inside the content namespace (new code in
content
> should be in that namespace, sorry I know this is not well advertised)

Done.

http://codereview.chromium.org/7054080/diff/10006/sandbox/tools/finder/ntundoc.h
File sandbox/tools/finder/ntundoc.h (right):

http://codereview.chromium.org/7054080/diff/10006/sandbox/tools/finder/ntundo...
sandbox/tools/finder/ntundoc.h:2: // Use of this source code is governed by a
BSD-style license that can be
On 2011/06/08 21:16:02, cpu wrote:
> fix copyright year

Done.

[jam, 2011-06-09 21:44:29]

http://codereview.chromium.org/7054080/diff/17003/content/browser/child_proce...
File content/browser/child_process_launcher.cc (right):

http://codereview.chromium.org/7054080/diff/17003/content/browser/child_proce...
content/browser/child_process_launcher.cc:24: #include
"chrome/common/chrome_switches.h"
this isn't needed

http://codereview.chromium.org/7054080/diff/17003/content/common/content_swit...
File content/common/content_switches.cc (right):

http://codereview.chromium.org/7054080/diff/17003/content/common/content_swit...
content/common/content_switches.cc:7: #include "base/command_line.h"
ditto

http://codereview.chromium.org/7054080/diff/17003/content/common/content_swit...
File content/common/content_switches.h (right):

http://codereview.chromium.org/7054080/diff/17003/content/common/content_swit...
content/common/content_switches.h:11: #include "base/command_line.h"
this seems wrong.  you should only include command_line.h if you need it in this
header, which you don't.

[Cris Neckar, 2011-06-09 22:17:16]

http://codereview.chromium.org/7054080/diff/17003/content/browser/child_proce...
File content/browser/child_process_launcher.cc (right):

http://codereview.chromium.org/7054080/diff/17003/content/browser/child_proce...
content/browser/child_process_launcher.cc:24: #include
"chrome/common/chrome_switches.h"
On 2011/06/09 21:44:29, John Abd-El-Malek wrote:
> this isn't needed

Done.

http://codereview.chromium.org/7054080/diff/17003/content/common/content_swit...
File content/common/content_switches.cc (right):

http://codereview.chromium.org/7054080/diff/17003/content/common/content_swit...
content/common/content_switches.cc:7: #include "base/command_line.h"
On 2011/06/09 21:44:29, John Abd-El-Malek wrote:
> ditto

Done.

http://codereview.chromium.org/7054080/diff/17003/content/common/content_swit...
File content/common/content_switches.h (right):

http://codereview.chromium.org/7054080/diff/17003/content/common/content_swit...
content/common/content_switches.h:11: #include "base/command_line.h"
On 2011/06/09 21:44:29, John Abd-El-Malek wrote:
> this seems wrong.  you should only include command_line.h if you need it in
this
> header, which you don't.  

Done.

[jam, 2011-06-09 22:23:47]

lgtm