Disallow breakaway from job for unit tests

BUG=328592, 371406
R=sky@chromium.org

Review URL: https://codereview.chromium.org/441333002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288305 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 32 insertions, 17 deletions

diff --git a/base/test/launcher/test_launcher.cc b/base/test/launcher/test_launcher.cc
index 6005db2..e6c1507 100644
--- a/base/test/launcher/test_launcher.cc
+++ b/base/test/launcher/test_launcher.cc
@@ -48,7 +48,7 @@ namespace base {
 // Returns exit code of the process.
 int LaunchChildTestProcessWithOptions(const CommandLine& command_line,
                                       const LaunchOptions& options,
-                                      bool use_job_objects,
+                                      int flags,
                                       base::TimeDelta timeout,
                                       bool* was_timeout);
 
@@ -223,7 +223,7 @@ void RunCallback(
 void DoLaunchChildTestProcess(
     const CommandLine& command_line,
     base::TimeDelta timeout,
-    bool use_job_objects,
+    int flags,
     bool redirect_stdio,
     scoped_refptr<MessageLoopProxy> message_loop_proxy,
     const TestLauncher::LaunchChildGTestProcessCallback& callback) {
@@ -275,7 +275,7 @@ void DoLaunchChildTestProcess(
 
   bool was_timeout = false;
   int exit_code = LaunchChildTestProcessWithOptions(
-      command_line, options, use_job_objects, timeout, &was_timeout);
+      command_line, options, flags, timeout, &was_timeout);
 
   if (redirect_stdio) {
 #if defined(OS_WIN)
@@ -409,7 +409,7 @@ void TestLauncher::LaunchChildGTestProcess(
     const CommandLine& command_line,
     const std::string& wrapper,
     base::TimeDelta timeout,
-    bool use_job_objects,
+    int flags,
     const LaunchChildGTestProcessCallback& callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
@@ -427,7 +427,7 @@ void TestLauncher::LaunchChildGTestProcess(
       Bind(&DoLaunchChildTestProcess,
            new_command_line,
            timeout,
-           use_job_objects,
+           flags,
            redirect_stdio,
            MessageLoopProxy::current(),
            Bind(&TestLauncher::OnLaunchTestProcessFinished,
@@ -1009,7 +1009,7 @@ CommandLine PrepareCommandLineForGTest(const CommandLine& command_line,
 // TODO(phajdan.jr): Move to anonymous namespace.
 int LaunchChildTestProcessWithOptions(const CommandLine& command_line,
                                       const LaunchOptions& options,
-                                      bool use_job_objects,
+                                      int flags,
                                       base::TimeDelta timeout,
                                       bool* was_timeout) {
 #if defined(OS_POSIX)
@@ -1023,19 +1023,22 @@ int LaunchChildTestProcessWithOptions(const CommandLine& command_line,
   DCHECK(!new_options.job_handle);
 
   win::ScopedHandle job_handle;
-  if (use_job_objects) {
+  if (flags & TestLauncher::USE_JOB_OBJECTS) {
     job_handle.Set(CreateJobObject(NULL, NULL));
     if (!job_handle.IsValid()) {
       LOG(ERROR) << "Could not create JobObject.";
       return -1;
     }
 
+    DWORD job_flags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
+
     // Allow break-away from job since sandbox and few other places rely on it
     // on Windows versions prior to Windows 8 (which supports nested jobs).
     // TODO(phajdan.jr): Do not allow break-away on Windows 8.
-    if (!SetJobObjectLimitFlags(job_handle.Get(),
-                                JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE |
-                                JOB_OBJECT_LIMIT_BREAKAWAY_OK)) {
+    if (flags & TestLauncher::ALLOW_BREAKAWAY_FROM_JOB)
+      job_flags |= JOB_OBJECT_LIMIT_BREAKAWAY_OK;
+
+    if (!SetJobObjectLimitFlags(job_handle.Get(), job_flags)) {
       LOG(ERROR) << "Could not SetJobObjectLimitFlags.";
       return -1;
     }
diff --git a/base/test/launcher/test_launcher.h b/base/test/launcher/test_launcher.h
index d50bf2f..1837aad 100644
--- a/base/test/launcher/test_launcher.h
+++ b/base/test/launcher/test_launcher.h
@@ -69,6 +69,18 @@ class TestLauncherDelegate {
 // Launches tests using a TestLauncherDelegate.
 class TestLauncher {
  public:
+  // Flags controlling behavior of LaunchChildGTestProcess.
+  enum LaunchChildGTestProcessFlags {
+    // Allows usage of job objects on Windows. Helps properly clean up child
+    // processes.
+    USE_JOB_OBJECTS = (1 << 0),
+
+    // Allows breakaway from job on Windows. May result in some child processes
+    // not being properly terminated after launcher dies if these processes
+    // fail to cooperate.
+    ALLOW_BREAKAWAY_FROM_JOB = (1 << 1),
+  };
+
   // Constructor. |parallel_jobs| is the limit of simultaneous parallel test
   // jobs.
   TestLauncher(TestLauncherDelegate* launcher_delegate, size_t parallel_jobs);
@@ -87,13 +99,12 @@ class TestLauncher {
   // Launches a child process (assumed to be gtest-based binary) using
   // |command_line|. If |wrapper| is not empty, it is prepended to the final
   // command line. If the child process is still running after |timeout|, it
-  // is terminated. |use_job_objects| determines whether job objects are used
-  // on Windows (if unsure pass true). After the child process finishes
-  // |callback| is called on the same thread this method was called.
+  // is terminated. After the child process finishes |callback| is called
+  // on the same thread this method was called.
   void LaunchChildGTestProcess(const CommandLine& command_line,
                                const std::string& wrapper,
                                base::TimeDelta timeout,
-                               bool use_job_objects,
+                               int flags,
                                const LaunchChildGTestProcessCallback& callback);
 
   // Called when a test has finished running.
diff --git a/base/test/launcher/unit_test_launcher.cc b/base/test/launcher/unit_test_launcher.cc
index 87784d9..841d5bee 100644
--- a/base/test/launcher/unit_test_launcher.cc
+++ b/base/test/launcher/unit_test_launcher.cc
@@ -187,7 +187,7 @@ class UnitTestLauncherDelegate : public TestLauncherDelegate {
         cmd_line,
         std::string(),
         TestTimeouts::test_launcher_timeout(),
-        use_job_objects_,
+        use_job_objects_ ? TestLauncher::USE_JOB_OBJECTS : 0,
         Bind(&UnitTestLauncherDelegate::SerialGTestCallback,
              Unretained(this),
              callback_state,
@@ -229,7 +229,7 @@ class UnitTestLauncherDelegate : public TestLauncherDelegate {
         cmd_line,
         std::string(),
         timeout,
-        use_job_objects_,
+        use_job_objects_ ? TestLauncher::USE_JOB_OBJECTS : 0,
         Bind(&UnitTestLauncherDelegate::GTestCallback,
              Unretained(this),
              callback_state));
diff --git a/content/public/test/test_launcher.cc b/content/public/test/test_launcher.cc
index 1914fd1..2222487 100644
--- a/content/public/test/test_launcher.cc
+++ b/content/public/test/test_launcher.cc
@@ -345,7 +345,8 @@ void WrapperTestLauncherDelegate::DoRunTest(base::TestLauncher* test_launcher,
       new_cmd_line,
       browser_wrapper ? browser_wrapper : std::string(),
       TestTimeouts::action_max_timeout(),
-      true,
+      base::TestLauncher::USE_JOB_OBJECTS |
+          base::TestLauncher::ALLOW_BREAKAWAY_FROM_JOB,
       base::Bind(&WrapperTestLauncherDelegate::GTestCallback,
                  base::Unretained(this),
                  test_launcher,