[Windows Sandbox Tests] Process Mitigations.

[Windows Sandbox Tests] Re-format process mitigation tests.

- Rename process_mitigations_test.cc -> process_mitigations_unittest.cc
- Pull large chunks out of process_mitigations_unittest.cc into unittest
files of their own.  The size was getting way out of hand.
- New files for existing tests: extensionpoints, imageload, win32k.
- NO new functionality tests in this CL - only re-organized things:
  - Note: the existing tests for every process mitigation that just check policy
	enabling have been centralized into one single test target function
	(SBOX_TESTS_COMMAND) called CheckPolicy.  Any new mitigation checks
	should be added here as well.
  - Note: The individual mitigation TESTs that trigger "CheckPolicy" have been
	adjusted to ensure we test both pre and post child-process startup wherever
	they are supported.

TEST= sbox_integration_tests.exe, ProcessMitigationsTest.*
BUG=733739
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
TBR=forshaw,wfh

Review-Url: https://codereview.chromium.org/2944493002
Cr-Commit-Position: refs/heads/master@{#485387}
Committed: https://chromium.googlesource.com/chromium/src/+/661bc439df8ba9d5205af4acb617f640acaf2f1f

[penny, 2017-06-15 18:29:27]

Description was changed from

==========
[Windows Sandbox Tests] Process Mitigations.

- Rename process_mitigations_test.cc -> process_mitigations_unittest.cc
- Pull large chunks out of process_mitigations_unittest.cc into unittest
files of their own.  The size was getting way out of hand.
- New files for existing tests: extensionpoints, imageload, win32k.
- NO new functionality tests in this CL - only re-organized things:
  - Note: the existing tests for every process mitigation that just check policy
	enabling have been centralized into one single test target function
	(SBOX_TESTS_COMMAND) called CheckPolicy.  Any new mitigation checks
	should be added here as well.
  - Note: The individual mitigation TESTs that trigger "CheckPolicy" have been
	adjusted to ensure we test both pre and post child-process startup wherever
	they are supported.

TEST= sbox_integration_tests.exe, ProcessMitigationsTest.*
BUG=733739
==========

to

==========
[Windows Sandbox Tests] Process Mitigations.

- Rename process_mitigations_test.cc -> process_mitigations_unittest.cc
- Pull large chunks out of process_mitigations_unittest.cc into unittest
files of their own.  The size was getting way out of hand.
- New files for existing tests: extensionpoints, imageload, win32k.
- NO new functionality tests in this CL - only re-organized things:
  - Note: the existing tests for every process mitigation that just check policy
	enabling have been centralized into one single test target function
	(SBOX_TESTS_COMMAND) called CheckPolicy.  Any new mitigation checks
	should be added here as well.
  - Note: The individual mitigation TESTs that trigger "CheckPolicy" have been
	adjusted to ensure we test both pre and post child-process startup wherever
	they are supported.

TEST= sbox_integration_tests.exe, ProcessMitigationsTest.*
BUG=733739
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[penny, 2017-06-15 18:31:57]

Description was changed from

==========
[Windows Sandbox Tests] Process Mitigations.

- Rename process_mitigations_test.cc -> process_mitigations_unittest.cc
- Pull large chunks out of process_mitigations_unittest.cc into unittest
files of their own.  The size was getting way out of hand.
- New files for existing tests: extensionpoints, imageload, win32k.
- NO new functionality tests in this CL - only re-organized things:
  - Note: the existing tests for every process mitigation that just check policy
	enabling have been centralized into one single test target function
	(SBOX_TESTS_COMMAND) called CheckPolicy.  Any new mitigation checks
	should be added here as well.
  - Note: The individual mitigation TESTs that trigger "CheckPolicy" have been
	adjusted to ensure we test both pre and post child-process startup wherever
	they are supported.

TEST= sbox_integration_tests.exe, ProcessMitigationsTest.*
BUG=733739
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
[Windows Sandbox Tests] Re-format process mitigation tests.

- Rename process_mitigations_test.cc -> process_mitigations_unittest.cc
- Pull large chunks out of process_mitigations_unittest.cc into unittest
files of their own.  The size was getting way out of hand.
- New files for existing tests: extensionpoints, imageload, win32k.
- NO new functionality tests in this CL - only re-organized things:
  - Note: the existing tests for every process mitigation that just check policy
	enabling have been centralized into one single test target function
	(SBOX_TESTS_COMMAND) called CheckPolicy.  Any new mitigation checks
	should be added here as well.
  - Note: The individual mitigation TESTs that trigger "CheckPolicy" have been
	adjusted to ensure we test both pre and post child-process startup wherever
	they are supported.

TEST= sbox_integration_tests.exe, ProcessMitigationsTest.*
BUG=733739
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[penny, 2017-06-15 18:32:12]

The CQ bit was checked by pennymac@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-15 18:32:43]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-15 19:47:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-15 19:47:42]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[penny, 2017-06-15 20:00:41]

pennymac@chromium.org changed reviewers:
+ wfh@chromium.org

[penny, 2017-06-15 20:03:10]

Hi Will,

I've split the original CL (https://codereview.chromium.org/2827123003/) into 2.

This is CL#1.  See description.

Thanks!

[penny, 2017-06-15 20:17:44]

Also, I've got CL#2 ready to go locally, but I'll wait until we land this one
(since it depends on this re-org).

[grt (UTC plus 2), 2017-06-15 20:58:19]

ridiculous drive-by comments below. feel free to disregard. ;-)

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/BUILD.gn
File sandbox/win/BUILD.gn (right):

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/BUILD.gn#newcod...
sandbox/win/BUILD.gn:186: "src/process_mitigations_unittest_extensionpoints.cc",
personal preference nit: make _unittest.cc the suffix (i.e.,
process_mitigations_extensionpoints_unittest.cc).

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/src/process_mit...
File sandbox/win/src/process_mitigations_unittest_extensionpoints.cc (right):

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/src/process_mit...
sandbox/win/src/process_mitigations_unittest_extensionpoints.cc:8: #include
<windows.h>
nit: <windows.h> should always precede other windows headers. put a blank line
between it and other "C" headers to thwart the presubmit hooks that will
complain about these not being sorted otherwise:

#include <windows.h>

#include <psapi.h>

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/src/process_mit...
sandbox/win/src/process_mitigations_unittest_extensionpoints.cc:349: const
wchar_t g_extension_point_test_mutex[] = L"ChromeExtensionTestMutex";
can these globals be in an unnamed namespace so that they have internal linkage?

[Will Harris, 2017-06-15 22:32:13]

still looking, more comments to follow.

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/BUILD.gn
File sandbox/win/BUILD.gn (right):

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/BUILD.gn#newcod...
sandbox/win/BUILD.gn:186: "src/process_mitigations_unittest_extensionpoints.cc",
On 2017/06/15 20:58:19, grt (UTC plus 2) wrote:
> personal preference nit: make _unittest.cc the suffix (i.e.,
> process_mitigations_extensionpoints_unittest.cc).

agree with this, I think _unittest.cc or _test.cc should be on the end of the
filename to match everything else...

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/src/process_mit...
File sandbox/win/src/process_mitigations_unittest_extensionpoints.cc (right):

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/src/process_mit...
sandbox/win/src/process_mitigations_unittest_extensionpoints.cc:349: const
wchar_t g_extension_point_test_mutex[] = L"ChromeExtensionTestMutex";
On 2017/06/15 20:58:19, grt (UTC plus 2) wrote:
> can these globals be in an unnamed namespace so that they have internal
linkage?

they only seem to be used in one place, so could just be declared const nearest
their use, I think, unless I'm missing something. oh, perhaps to save declaring
them multiple times? either way putting them in anon namespace makes sense, and
perhaps removing the extern declaration in the .h file.

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/tests/integrati...
File sandbox/win/tests/integration_tests/integration_tests_common.h (right):

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/tests/integrati...
sandbox/win/tests/integration_tests/integration_tests_common.h:44: extern const
char g_hook_handler_func[];
not sure why these have to be extern const, what else uses these?

[Will Harris, 2017-06-21 12:18:11]

can you address the variable declaration issues, perhaps by putting the common
variables in their own .cc file (e.g. see how chrome_switches.cc|h works) then
linking the file with both hooking_dll and your test binary? Once that's done
I'll take another look. thanks.

[penny, 2017-06-27 20:40:51]

The CQ bit was checked by pennymac@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-27 20:41:09]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[penny, 2017-06-27 20:43:50]

Hello Will (and Greg),

I think I've hit all your items in this update.  Let me know if you're happy
with where the defines are now.  I was struggling to not add extra .h files, but
I think everything is cleaner now.

Let me know what you think!

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/BUILD.gn
File sandbox/win/BUILD.gn (right):

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/BUILD.gn#newcod...
sandbox/win/BUILD.gn:186: "src/process_mitigations_unittest_extensionpoints.cc",
On 2017/06/15 22:32:13, Will Harris (UTC TZ) wrote:
> On 2017/06/15 20:58:19, grt (UTC plus 2) wrote:
> > personal preference nit: make _unittest.cc the suffix (i.e.,
> > process_mitigations_extensionpoints_unittest.cc).
> 
> agree with this, I think _unittest.cc or _test.cc should be on the end of the
> filename to match everything else...

DONE.

(I don't want to use _test anymore, since that's not our current chromium file
naming convention.  I would rather do more renames and file reorganization in a
follow-up CL to clean up the sandbox tests than just fall back to consistency. 
I touch these tests often enough that cleanup can continue.)

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/src/process_mit...
File sandbox/win/src/process_mitigations_unittest_extensionpoints.cc (right):

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/src/process_mit...
sandbox/win/src/process_mitigations_unittest_extensionpoints.cc:8: #include
<windows.h>
On 2017/06/15 20:58:19, grt (UTC plus 2) wrote:
> nit: <windows.h> should always precede other windows headers. put a blank line
> between it and other "C" headers to thwart the presubmit hooks that will
> complain about these not being sorted otherwise:
> 
> #include <windows.h>
> 
> #include <psapi.h>

DONE. Fixed in win32k unittest file as well.

This drives me nuts.  I had a discussion with Nico about this pre-submit check,
which not only warns of out-of-order includes - it also now forcibly re-orders
your includes for you on upload! (Yes, this has actually broken one of my CLs
before DURING upload.) In the end, he wasn't willing to change the pre-submit
behaviour just for this "rare" situation where the auto re-order actually breaks
the build due to windows.h not being first.

So instead we have to trick it by adding an extra ugly line.  This makes me sad.
 I agree with you it should always be first in the platform includes section.

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/tests/integrati...
File sandbox/win/tests/integration_tests/integration_tests_common.h (right):

https://codereview.chromium.org/2944493002/diff/1/sandbox/win/tests/integrati...
sandbox/win/tests/integration_tests/integration_tests_common.h:44: extern const
char g_hook_handler_func[];
On 2017/06/15 22:32:13, Will Harris (UTC TZ) wrote:
> not sure why these have to be extern const, what else uses these?

Some of the new tests coming up in CL2.  But I've changed
integration_tests_common again, and removed all the little defines that were
weirdly being shared between test DLLs and EXEs.

I wanted to try to keep it all in one .h file, but that just wasn't working
nicely.  So I've added .h files for hooking_dll and hooking_win_proc.

See what you think of it now.

[commit-bot: I haz the power, 2017-06-27 23:29:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-27 23:29:51]

Dry run: This issue passed the CQ dry run.

[grt (UTC plus 2), 2017-06-28 12:16:55]

i'll defer to wfh unless you really need an extra set of eyes. thanks.

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/src/process...
File sandbox/win/src/process_mitigations_extensionpoints_unittest.cc (right):

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/src/process...
sandbox/win/src/process_mitigations_extensionpoints_unittest.cc:31: const
wchar_t g_extension_point_test_mutex[] = L"ChromeExtensionTestMutex";
nit: use constexpr throughout where possible

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/src/process...
sandbox/win/src/process_mitigations_extensionpoints_unittest.cc:220: wchar_t*
non_const = const_cast<wchar_t*>(full_dll_path.value().c_str());
i don't think you need to cast away constness to call GetShortPathName

[Will Harris, 2017-06-28 12:27:55]

I think maybe it's a stylistic thing but the constants in the .h files still
confuses me a bit, since they are only used in one place. I'm not sure why they
are there, we don't do this anywhere else in Chromium. I suppose I don't feel
too strongly about this but normally we just use the string directly.

e.g. "ChromeMain" is exported from chrome.dll and chrome.exe just uses that
directly in MainDllLoader here ->
https://cs.chromium.org/chromium/src/chrome/app/main_dll_loader_win.cc?l=198

I think we should stick to using .h files for shared definitions between
compilation units rather than overloading their use for other things "defined"
by components... maybe grt@ has some views on this.

Otherwise, this is very close to an lg--tm.

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/src/process...
File sandbox/win/src/process_mitigations_unittest.cc (right):

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/src/process...
sandbox/win/src/process_mitigations_unittest.cc:453: std::wstring test_command =
L"CheckPolicy ";
style guide says do not use std::wstring except for direct interactions with
win32 apis that might need them.

use base::string16 here and elsewhere.

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
File sandbox/win/tests/integration_tests/hooking_dll.h (right):

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
sandbox/win/tests/integration_tests/hooking_dll.h:20: constexpr char
g_hook_handler_func[] = "HookProc";
same comment for g_hook_handler_func

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
sandbox/win/tests/integration_tests/hooking_dll.h:21: constexpr char
g_was_hook_called_func[] = "WasHookCalled";
same comment for g_was_hook_called_func

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
sandbox/win/tests/integration_tests/hooking_dll.h:22: constexpr char
g_set_hook_func[] = "SetHook";
same comment for g_set_hook_func

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
File sandbox/win/tests/integration_tests/hooking_win_proc.cc (right):

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
sandbox/win/tests/integration_tests/hooking_win_proc.cc:11: constexpr wchar_t
g_winproc_class_name[] = L"myWindowClass";
declare these closest to use, so that would be inside WinMain here.

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
File sandbox/win/tests/integration_tests/hooking_win_proc.h (right):

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
sandbox/win/tests/integration_tests/hooking_win_proc.h:12: constexpr wchar_t
g_winproc_file[] = L"sbox_integration_test_win_proc.exe ";
I still don't quite get the point of this g_winproc_file declaration, it seems
to only be used in one file so why not just declare it locally there...?

Is this because you feel their declaration is 'owned' by hooking_win_proc? I can
understand them being declared here if they are also used in hooking_win_proc.cc
but I don't think there is any precedent for not just declaring these locally
where they are used in other files if they are only used in one place... we make
same assumptions like this about target files and exported procedures elsewhere.

(this comment applies elsewhere, so refer to here)

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
File sandbox/win/tests/integration_tests/integration_tests_common.h (right):

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
sandbox/win/tests/integration_tests/integration_tests_common.h:27: DWORD
SboxTestEventTimeout();
this should be implemented in integration_tests_common.cc not in
process_mitigations_unittest.cc

[penny, 2017-06-28 22:32:12]

The CQ bit was checked by pennymac@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-28 22:32:38]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[penny, 2017-06-28 22:33:34]

Here's round 2 Will!

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/src/process...
File sandbox/win/src/process_mitigations_unittest.cc (right):

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/src/process...
sandbox/win/src/process_mitigations_unittest.cc:453: std::wstring test_command =
L"CheckPolicy ";
On 2017/06/28 12:27:54, Will Harris (UTC TZ) wrote:
> style guide says do not use std::wstring except for direct interactions with
> win32 apis that might need them.
> 
> use base::string16 here and elsewhere.

Done.  In each file I touched.  Thanks!

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
File sandbox/win/tests/integration_tests/hooking_win_proc.cc (right):

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
sandbox/win/tests/integration_tests/hooking_win_proc.cc:11: constexpr wchar_t
g_winproc_class_name[] = L"myWindowClass";
On 2017/06/28 12:27:55, Will Harris (UTC TZ) wrote:
> declare these closest to use, so that would be inside WinMain here.

Done.

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
File sandbox/win/tests/integration_tests/hooking_win_proc.h (right):

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
sandbox/win/tests/integration_tests/hooking_win_proc.h:12: constexpr wchar_t
g_winproc_file[] = L"sbox_integration_test_win_proc.exe ";
On 2017/06/28 12:27:55, Will Harris (UTC TZ) wrote:
> I still don't quite get the point of this g_winproc_file declaration, it seems
> to only be used in one file so why not just declare it locally there...?
> 
> Is this because you feel their declaration is 'owned' by hooking_win_proc? I
can
> understand them being declared here if they are also used in
hooking_win_proc.cc
> but I don't think there is any precedent for not just declaring these locally
> where they are used in other files if they are only used in one place... we
make
> same assumptions like this about target files and exported procedures
elsewhere.
> 
> (this comment applies elsewhere, so refer to here)

HookingWinProc (exe): 
  - g_winproc_event name is required in 2 places - the test and the exe. 
They're both accessing a named os resource during testing.
  - g_winproc_file name is indeed currently only used from inside the test
itself (to launch the exe).  however, since I need a .h file for the event name,
I figured it made sense to put the exe filename in here too.  exactly as you
mentioned, it kind of does "belong" to the test exe project itself.  if any
other projects want to use it - they should include this new .h file and refer
to the global instead of hard-coding the filename.

HookingDll (dll):
  - g_hook_event name is required in 2 places - the test and the exe.  They're
both accessing a named os resource during testing.
  - g_hook_dll_file same as above, except a perfect example of one that is used
by more than one test.  CL2 (coming right after this reorg CL) with the new
mitigation tests also use the hooking_dll, and access this global.
  - WRT the three exported function name strings
(SetHook/WasHookCalled/HookProc), I say two things:
1) I'm happy to move them into the extensionpoints .cc file as they're only used
here for now
2) I had put them in the .h file just because the names of these exports are
basically "owned" by the test DLL project, but I realize we don't tend to do
this now for dynamic function look-ups (though I would argue that's just because
we usually lookup from MS system dlls which don't provide the function name
strings as a service).  I would argue with you if I knew any other test files
would be doing the dynamic lookups - but they're not for now. 

Sorry - these defines were a bit confusing, and more so now because I split up
my mega CL.  In the next half, the new tests use some of these defines.

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
File sandbox/win/tests/integration_tests/integration_tests_common.h (right):

https://codereview.chromium.org/2944493002/diff/20001/sandbox/win/tests/integ...
sandbox/win/tests/integration_tests/integration_tests_common.h:27: DWORD
SboxTestEventTimeout();
On 2017/06/28 12:27:55, Will Harris (UTC TZ) wrote:
> this should be implemented in integration_tests_common.cc not in
> process_mitigations_unittest.cc

Good point, thanks!  There is no integration_test_common.cc file (yet)... I
dropped it into integration_tests.cc.  Let me know if you think this is the
right place for "common" functions... or if you think  I should actually create
a new cc file.

[commit-bot: I haz the power, 2017-06-29 00:09:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-29 00:09:48]

Dry run: This issue passed the CQ dry run.

[penny, 2017-07-10 19:37:13]

Description was changed from

==========
[Windows Sandbox Tests] Re-format process mitigation tests.

- Rename process_mitigations_test.cc -> process_mitigations_unittest.cc
- Pull large chunks out of process_mitigations_unittest.cc into unittest
files of their own.  The size was getting way out of hand.
- New files for existing tests: extensionpoints, imageload, win32k.
- NO new functionality tests in this CL - only re-organized things:
  - Note: the existing tests for every process mitigation that just check policy
	enabling have been centralized into one single test target function
	(SBOX_TESTS_COMMAND) called CheckPolicy.  Any new mitigation checks
	should be added here as well.
  - Note: The individual mitigation TESTs that trigger "CheckPolicy" have been
	adjusted to ensure we test both pre and post child-process startup wherever
	they are supported.

TEST= sbox_integration_tests.exe, ProcessMitigationsTest.*
BUG=733739
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
[Windows Sandbox Tests] Re-format process mitigation tests.

- Rename process_mitigations_test.cc -> process_mitigations_unittest.cc
- Pull large chunks out of process_mitigations_unittest.cc into unittest
files of their own.  The size was getting way out of hand.
- New files for existing tests: extensionpoints, imageload, win32k.
- NO new functionality tests in this CL - only re-organized things:
  - Note: the existing tests for every process mitigation that just check policy
	enabling have been centralized into one single test target function
	(SBOX_TESTS_COMMAND) called CheckPolicy.  Any new mitigation checks
	should be added here as well.
  - Note: The individual mitigation TESTs that trigger "CheckPolicy" have been
	adjusted to ensure we test both pre and post child-process startup wherever
	they are supported.

TEST= sbox_integration_tests.exe, ProcessMitigationsTest.*
BUG=733739
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
TBR=forshaw,wfh
==========

[penny, 2017-07-10 19:39:34]

Landing this CL now.  wfh is ooo for the next while.  The next half of this
commit (cl2) will be uploaded right after this one lands.

forshaw@, if you have comments/requests related to any of the last items in this
review, please just let me know and I can always change things in the next CL.

[penny, 2017-07-10 19:39:44]

The CQ bit was checked by pennymac@chromium.org

[commit-bot: I haz the power, 2017-07-10 19:40:16]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-10 21:09:21]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1499715584246450,
"parent_rev": "b8029247a53734bc872a4edf9f6608c6bd670fbf", "commit_rev":
"64b847ea765efdd6faefdda383c568016dc9a35b"}

[commit-bot: I haz the power, 2017-07-10 21:09:37]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1499715584246450,
"parent_rev": "4e969585d96f9181116cfd44ae53185a244b3c15", "commit_rev":
"b8a84046ca6c774b156b418ad5757e0a9c841d76"}

[commit-bot: I haz the power, 2017-07-10 21:09:53]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1499715584246450,
"parent_rev": "39242679a2202b4bb17246c361d01efc01cb4c1c", "commit_rev":
"661bc439df8ba9d5205af4acb617f640acaf2f1f"}

[commit-bot: I haz the power, 2017-07-10 21:10:05]

Description was changed from

==========
[Windows Sandbox Tests] Re-format process mitigation tests.

- Rename process_mitigations_test.cc -> process_mitigations_unittest.cc
- Pull large chunks out of process_mitigations_unittest.cc into unittest
files of their own.  The size was getting way out of hand.
- New files for existing tests: extensionpoints, imageload, win32k.
- NO new functionality tests in this CL - only re-organized things:
  - Note: the existing tests for every process mitigation that just check policy
	enabling have been centralized into one single test target function
	(SBOX_TESTS_COMMAND) called CheckPolicy.  Any new mitigation checks
	should be added here as well.
  - Note: The individual mitigation TESTs that trigger "CheckPolicy" have been
	adjusted to ensure we test both pre and post child-process startup wherever
	they are supported.

TEST= sbox_integration_tests.exe, ProcessMitigationsTest.*
BUG=733739
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
TBR=forshaw,wfh
==========

to

==========
[Windows Sandbox Tests] Re-format process mitigation tests.

- Rename process_mitigations_test.cc -> process_mitigations_unittest.cc
- Pull large chunks out of process_mitigations_unittest.cc into unittest
files of their own.  The size was getting way out of hand.
- New files for existing tests: extensionpoints, imageload, win32k.
- NO new functionality tests in this CL - only re-organized things:
  - Note: the existing tests for every process mitigation that just check policy
	enabling have been centralized into one single test target function
	(SBOX_TESTS_COMMAND) called CheckPolicy.  Any new mitigation checks
	should be added here as well.
  - Note: The individual mitigation TESTs that trigger "CheckPolicy" have been
	adjusted to ensure we test both pre and post child-process startup wherever
	they are supported.

TEST= sbox_integration_tests.exe, ProcessMitigationsTest.*
BUG=733739
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
TBR=forshaw,wfh

Review-Url: https://codereview.chromium.org/2944493002
Cr-Commit-Position: refs/heads/master@{#485387}
Committed:
https://chromium.googlesource.com/chromium/src/+/661bc439df8ba9d5205af4acb617...
==========

[commit-bot: I haz the power, 2017-07-10 21:10:07]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/661bc439df8ba9d5205af4acb617...

[penny, 2017-07-17 18:17:07]

FYI: CL2 at https://chromium-review.googlesource.com/c/566286.