Wait on the network process at shutdown.

This gives the child process time to flush I/O. This can help avoid the OS restarting in the middle
of flushing data. It's also needed because swarming jobs fail when the browser quits with child
processes still alive.

An example flake is
https://chromium-swarm.appspot.com/task?id=3d8cd8e056a4b310&refresh=10&show_raw=1. Roughly half of
the layout test runs on Win Mojo are flaking as a result.

Bug: 820996
Change-Id: I9c3b13f83c716f020f555cbe2aa14e20c8825ed3
Reviewed-on: https://chromium-review.googlesource.com/1066622Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560368}

content/browser/browser_process_sub_thread.cc

@@ -6,13 +6,18 @@
 
 #include "base/compiler_specific.h"
 #include "base/debug/alias.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/trace_event/memory_dump_manager.h"
 #include "content/browser/browser_child_process_host_impl.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/gpu/browser_gpu_memory_buffer_manager.h"
 #include "content/browser/notification_service_impl.h"
+#include "content/browser/utility_process_host.h"
+#include "content/common/child_process_host_impl.h"
+#include "content/public/browser/browser_child_process_host_iterator.h"
 #include "content/public/browser/browser_thread_delegate.h"
+#include "content/public/common/process_type.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request.h"
 
@@ -173,6 +178,34 @@ void BrowserProcessSubThread::IOThreadCleanUp() {
   // IO thread only resources they are referencing.
   BrowserChildProcessHostImpl::TerminateAll();
 
+  for (BrowserChildProcessHostIterator it(PROCESS_TYPE_UTILITY); !it.Done();
+       ++it) {
+    UtilityProcessHost* utility_process =
+        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.
+      // https://crbug.com/841001
+      const int kMaxSecondsToWaitForNetworkProcess = 10;
+      ChildProcessHostImpl* child_process =
+          static_cast<ChildProcessHostImpl*>(it.GetHost());
+      const base::TimeTicks start_time = base::TimeTicks::Now();
+      child_process->peer_process().WaitForExitWithTimeout(
+          base::TimeDelta::FromSeconds(kMaxSecondsToWaitForNetworkProcess),
+          nullptr);
+      // Record time spent for the method call. Don't include failures.
+      UMA_HISTOGRAM_TIMES("NetworkService.ShutdownTime",
+                          base::TimeTicks::Now() - start_time);
+    }
+  }
+
   // Unregister GpuMemoryBuffer dump provider before IO thread is shut down.
   base::trace_event::MemoryDumpManager::GetInstance()->UnregisterDumpProvider(
       BrowserGpuMemoryBufferManager::current());

content/browser/utility_process_host.h

@@ -71,6 +71,8 @@ class CONTENT_EXPORT UtilityProcessHost
   // SANDBOX_TYPE_NO_SANDBOX is specified.
   void SetSandboxType(service_manager::SandboxType sandbox_type);
 
+  service_manager::SandboxType sandbox_type() const { return sandbox_type_; }
+
   // Returns information about the utility child process.
   const ChildProcessData& GetData();
 #if defined(OS_POSIX)

content/common/child_process_host_impl.h

@@ -71,6 +71,8 @@ class CONTENT_EXPORT ChildProcessHostImpl : public ChildProcessHost,
   void BindInterface(const std::string& interface_name,
                      mojo::ScopedMessagePipeHandle interface_pipe) override;
 
+  base::Process& peer_process() { return peer_process_; }
+
  private:
   friend class ChildProcessHost;
 

tools/metrics/histograms/histograms.xml

@@ -55342,6 +55342,13 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="NetworkService.ShutdownTime" units="ms">
+  <owner>jam@chromium.org</owner>
+  <summary>
+    How long the browser waits for the network process to exit at shutdown.
+  </summary>
+</histogram>
+
 <histogram name="NetworkTimeTracker.ClockDivergence.Negative" units="seconds">
   <owner>estark@chromium.org</owner>
   <owner>mab@chromium.org</owner>