Launch unsandboxed network service in a job object to avoid the process outliving the browser.

Currently many layout test runs are flaking because swarming is not expecting child processes to outlive the browser. This is happening because TerminateProcess fails sometimes because there's pending I/O.

Bug: 820996
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I7595c131faccae03c268cde1139394c0ad48a7b4
Reviewed-on: https://chromium-review.googlesource.com/1071100
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Penny MacNeil <pennymac@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562058}

content/browser/browser_process_sub_thread.cc

@@ -197,14 +197,7 @@ void BrowserProcessSubThread::IOThreadCleanUp() {
         static_cast<UtilityProcessHost*>(it.GetDelegate());
     if (utility_process->sandbox_type() ==
         service_manager::SANDBOX_TYPE_NETWORK) {
-      // Even though the TerminateAll call above tries to kill all child
-      // processes, that will fail sometimes (e.g. on Windows if there's pending
-      // I/O). Once the network service is sandboxed this will be taken care of,
-      // since the sandbox ensures child processes are terminated. Until then,
-      // wait on the network process for a bit. This is done so that:
-      // 1) when Chrome quits, we ensure that cookies & cache are flushed
-      // 2) tests aren't killed by swarming because of child processes that
-      //    outlive the parent process.
+      // This ensures that cookies and cache are flushed to disk on shutdown.
       // https://crbug.com/841001
       const int kMaxSecondsToWaitForNetworkProcess = 10;
       ChildProcessHostImpl* child_process =

services/service_manager/sandbox/win/sandbox_win.cc

@@ -39,6 +39,7 @@
 #include "base/win/win_util.h"
 #include "base/win/windows_version.h"
 #include "sandbox/win/src/app_container_profile.h"
+#include "sandbox/win/src/job.h"
 #include "sandbox/win/src/process_mitigations.h"
 #include "sandbox/win/src/sandbox.h"
 #include "sandbox/win/src/sandbox_nt_util.h"
@@ -53,6 +54,8 @@ namespace {
 
 sandbox::BrokerServices* g_broker_services = NULL;
 
+HANDLE g_job_object_handle = NULL;
+
 // The DLLs listed here are known (or under strong suspicion) of causing crashes
 // when they are loaded in the renderer. Note: at runtime we generate short
 // versions of the dll name only if the dll has an extension.
@@ -827,6 +830,20 @@ sandbox::ResultCode SandboxWin::StartSandboxedProcess(
           service_manager::switches::kNoSandbox)) {
     base::LaunchOptions options;
     options.handles_to_inherit = handles_to_inherit;
+    if (sandbox_type == SANDBOX_TYPE_NETWORK) {
+      // Launch the process in a job to ensure that the network process doesn't
+      // outlive the browser. This could happen if there is a lot of I/O on
+      // process shutdown, in which case TerminateProcess would fail.
+      // https://crbug.com/820996
+      if (!g_job_object_handle) {
+        sandbox::Job job_obj;
+        DWORD result = job_obj.Init(sandbox::JOB_UNPROTECTED, nullptr, 0, 0);
+        if (result != ERROR_SUCCESS)
+          return sandbox::SBOX_ERROR_GENERIC;
+        g_job_object_handle = job_obj.Take().Take();
+      }
+      options.job_handle = g_job_object_handle;
+    }
     *process = base::LaunchProcess(*cmd_line, options);
     return sandbox::SBOX_ALL_OK;
   }