Terminate plugin processes on a delayed task to allow the plugins to shutdown gracefully, i.e. NP...

content/browser/child_process_launcher.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/child_process_launcher.h"
 
 #include <utility>  // For std::pair.
 
 #include "base/command_line.h"
 #include "base/logging.h"
@@ skipping 87 matching lines @@
   void LaunchInternal(
 #if defined(OS_WIN)
       const FilePath& exposed_dir,
 #elif defined(OS_POSIX)
       bool use_zygote,
       const base::environment_vector& env,
       int ipcfd,
 #endif
       CommandLine* cmd_line) {
     scoped_ptr<CommandLine> cmd_line_deleter(cmd_line);
+    process_type_ = cmd_line->GetSwitchValueASCII(switches::kProcessType);
     base::ProcessHandle handle = base::kNullProcessHandle;
 #if defined(OS_WIN)
     handle = sandbox::StartProcessWithAccess(cmd_line, exposed_dir);
 #elif defined(OS_POSIX)
 
 #if defined(OS_LINUX)
     // On Linux, we need to add some extra file descriptors for crash handling.
     std::string process_type =
         cmd_line->GetSwitchValueASCII(switches::kProcessType);
     int crash_signal_fd =
@@ skipping 85 matching lines @@
       client_->OnProcessLaunched();
     } else {
       Terminate();
     }
   }
 
   void Terminate() {
     if (!process_.handle())
       return;
 
+    const int kPluginProcessTerminateTimeoutMs = 2000;
+
     // On Posix, EnsureProcessTerminated can lead to 2 seconds of sleep!  So
     // don't this on the UI/IO threads.
-    BrowserThread::PostTask(
-        BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
-        NewRunnableFunction(
-            &ChildProcessLauncher::Context::TerminateInternal,
-#if defined(OS_LINUX)
-            zygote_,
-#endif
-            process_.handle()));
+    // Allow plugins to shutdown gracefully before terminating the process.
+    if (process_type_ == switches::kPluginProcess) {

[jam, 2011/05/23 22:32:59]

ChildProcessLauncher is the base class that's used by all child processes.  It
seems out of place to add this logic to it, which is specific to plugins.

Either way, what happens if we post a task to kill the plugin, and the browser
shuts before the task runs?  Then we can end up with a zombie plugin process,
which would be very bad (i.e. wouldn't allow auto-update etc).

+      BrowserThread::PostDelayedTask(
+          BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
+          NewRunnableFunction(
+              &ChildProcessLauncher::Context::TerminateInternal,
+  #if defined(OS_LINUX)
+              zygote_,
+  #endif
+              process_.handle()), kPluginProcessTerminateTimeoutMs);
+    } else {
+      BrowserThread::PostTask(
+          BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
+          NewRunnableFunction(
+              &ChildProcessLauncher::Context::TerminateInternal,
+  #if defined(OS_LINUX)
+              zygote_,
+  #endif
+              process_.handle()));
+    }
     process_.set_handle(base::kNullProcessHandle);
   }
 
   void SetProcessBackgrounded(bool background) {
     DCHECK(!starting_);
     process_.SetProcessBackgrounded(background);
   }
 
   static void TerminateInternal(
 #if defined(OS_LINUX)
@@ skipping 17 matching lines @@
       ProcessWatcher::EnsureProcessTerminated(handle);
     }
 #endif  // OS_POSIX
     process.Close();
   }
 
   Client* client_;
   BrowserThread::ID client_thread_id_;
   base::Process process_;
   bool starting_;
+  std::string process_type_;
 
 #if defined(OS_LINUX)
   bool zygote_;
 #endif
 };
 
 
 ChildProcessLauncher::ChildProcessLauncher(
 #if defined(OS_WIN)
     const FilePath& exposed_dir,
@@ skipping 56 matching lines @@
 }
 
 void ChildProcessLauncher::SetProcessBackgrounded(bool background) {
   BrowserThread::PostTask(
       BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
       NewRunnableMethod(
           context_.get(),
           &ChildProcessLauncher::Context::SetProcessBackgrounded,
           background));
 }