Add ProcessWatcher::EnsureProcessGetsReaped() for zombie reaping. First target: xdg-open.

BUG=15342
TEST=Click show in folder for a download, close the file manager that opens up, make sure the xdg-open process eventually gets reaped.
Review URL: http://codereview.chromium.org/149057

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

3 files changed, 42 insertions, 7 deletions

diff --git a/chrome/common/platform_util_linux.cc b/chrome/common/platform_util_linux.cc
index 988de92..b13033c 100644
--- a/chrome/common/platform_util_linux.cc
+++ b/chrome/common/platform_util_linux.cc
@@ -6,10 +6,10 @@
 
 #include <gtk/gtk.h>
 
-#include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/process_util.h"
 #include "base/string_util.h"
+#include "chrome/common/process_watcher.h"
 
 namespace {
 
@@ -18,7 +18,9 @@ void XDGOpen(const FilePath& path) {
   argv.push_back("xdg-open");
   argv.push_back(path.value());
   base::file_handle_mapping_vector no_files;
-  base::LaunchApp(argv, no_files, false, NULL);
+  base::ProcessHandle handle;
+  if (base::LaunchApp(argv, no_files, false, &handle))
+    ProcessWatcher::EnsureProcessGetsReaped(handle);
 }
 
 }  // namespace
diff --git a/chrome/common/process_watcher.h b/chrome/common/process_watcher.h
index 3242984..a1f909c 100644
--- a/chrome/common/process_watcher.h
+++ b/chrome/common/process_watcher.h
@@ -5,6 +5,8 @@
 #ifndef CHROME_COMMON_PROCESS_WATCHER_H_
 #define CHROME_COMMON_PROCESS_WATCHER_H_
 
+#include "build/build_config.h"
+
 #include "base/basictypes.h"
 #include "base/process_util.h"
 
@@ -25,6 +27,12 @@ class ProcessWatcher {
   //
   static void EnsureProcessTerminated(base::ProcessHandle process_handle);
 
+#if defined(OS_POSIX)
+  // The nicer version of EnsureProcessTerminated() that is patient and will
+  // wait for |process_handle| to finish and then reap it.
+  static void EnsureProcessGetsReaped(base::ProcessHandle process_handle);
+#endif
+
  private:
   // Do not instantiate this class.
   ProcessWatcher();
diff --git a/chrome/common/process_watcher_posix.cc b/chrome/common/process_watcher_posix.cc
index f1ae4f4..0991f0cb 100644
--- a/chrome/common/process_watcher_posix.cc
+++ b/chrome/common/process_watcher_posix.cc
@@ -30,8 +30,9 @@ static bool IsChildDead(pid_t child) {
 // If the child doesn't exit within a couple of seconds, kill it.
 class BackgroundReaper : public PlatformThread::Delegate {
  public:
-  explicit BackgroundReaper(pid_t child)
-      : child_(child) {
+  explicit BackgroundReaper(pid_t child, unsigned timeout)
+      : child_(child),
+        timeout_(timeout) {
   }
 
   void ThreadMain() {
@@ -40,11 +41,21 @@ class BackgroundReaper : public PlatformThread::Delegate {
   }
 
   void WaitForChildToDie() {
+    // Wait forever case.
+    if (timeout_ == 0) {
+      pid_t r = waitpid(child_, NULL, 0);
+      if (r != child_) {
+        LOG(ERROR) << "While waiting for " << child_
+                   << " to terminate, we got the following result: " << r;
+      }
+      return;
+    }
+
     // There's no good way to wait for a specific child to exit in a timed
     // fashion. (No kqueue on Linux), so we just loop and sleep.
 
-    // Waits 0.5 * 4 = 2 seconds.
-    for (unsigned i = 0; i < 4; ++i) {
+    // Wait for 2 * timeout_ 500 milliseconds intervals.
+    for (unsigned i = 0; i < 2 * timeout_; ++i) {
       PlatformThread::Sleep(500);  // 0.5 seconds
       if (IsChildDead(child_))
         return;
@@ -62,6 +73,9 @@ class BackgroundReaper : public PlatformThread::Delegate {
 
  private:
   const pid_t child_;
+  // Number of seconds to wait, if 0 then wait forever and do not attempt to
+  // kill |child_|.
+  const unsigned timeout_;
 
   DISALLOW_COPY_AND_ASSIGN(BackgroundReaper);
 };
@@ -72,6 +86,17 @@ void ProcessWatcher::EnsureProcessTerminated(base::ProcessHandle process) {
   if (IsChildDead(process))
     return;
 
-  BackgroundReaper* reaper = new BackgroundReaper(process);
+  const unsigned timeout = 2;  // seconds
+  BackgroundReaper* reaper = new BackgroundReaper(process, timeout);
+  PlatformThread::CreateNonJoinable(0, reaper);
+}
+
+// static
+void ProcessWatcher::EnsureProcessGetsReaped(base::ProcessHandle process) {
+  // If the child is already dead, then there's nothing to do
+  if (IsChildDead(process))
+    return;
+
+  BackgroundReaper* reaper = new BackgroundReaper(process, 0);
   PlatformThread::CreateNonJoinable(0, reaper);
 }