Don't terminate plugin processes from the browser during browser shutdown. This is to allow the plugins to

shutdown gracefully, i.e. NP_Shutdown gets called. To ensure that we handle the case of a hung plugin, we handle
the OnChannelError notification in the IPC message filter implementation in the plugin process and post a delayed 
task to kill the process.

Fixes bug http://code.google.com/p/chromium/issues/detail?id=48178

BUG=48178
Review URL: http://codereview.chromium.org/6992006

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

6 files changed, 57 insertions, 1 deletions

diff --git a/content/browser/browser_child_process_host.cc b/content/browser/browser_child_process_host.cc
index ad378c8..ad7bafd 100644
--- a/content/browser/browser_child_process_host.cc
+++ b/content/browser/browser_child_process_host.cc
@@ -109,6 +109,11 @@ void BrowserChildProcessHost::ForceShutdown() {
   ChildProcessHost::ForceShutdown();
 }
 
+void BrowserChildProcessHost::SetTerminateChildOnShutdown(
+  bool terminate_on_shutdown) {
+  child_process_->SetTerminateChildOnShutdown(terminate_on_shutdown);
+}
+
 void BrowserChildProcessHost::Notify(NotificationType type) {
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE, new ChildNotificationTask(type, this));
diff --git a/content/browser/browser_child_process_host.h b/content/browser/browser_child_process_host.h
index 07bc44d..61e7981 100644
--- a/content/browser/browser_child_process_host.h
+++ b/content/browser/browser_child_process_host.h
@@ -92,6 +92,10 @@ class BrowserChildProcessHost : public ChildProcessHost,
   // the host list. Calls ChildProcessHost::ForceShutdown
   virtual void ForceShutdown();
 
+  // Controls whether the child process should be terminated on browser
+  // shutdown. Default is to always terminate.
+  void SetTerminateChildOnShutdown(bool terminate_on_shutdown);
+
  private:
   // By using an internal class as the ChildProcessLauncher::Client, we can
   // intercept OnProcessLaunched and do our own processing before
diff --git a/content/browser/child_process_launcher.cc b/content/browser/child_process_launcher.cc
index 785d8a1..5d3888e 100644
--- a/content/browser/child_process_launcher.cc
+++ b/content/browser/child_process_launcher.cc
@@ -44,7 +44,8 @@ class ChildProcessLauncher::Context
   Context()
       : client_(NULL),
         client_thread_id_(BrowserThread::UI),
-        starting_(true)
+        starting_(true),
+        terminate_child_on_shutdown_(true)
 #if defined(OS_LINUX)
         , zygote_(false)
 #endif
@@ -87,6 +88,10 @@ class ChildProcessLauncher::Context
     client_ = NULL;
   }
 
+  void set_terminate_child_on_shutdown(bool terminate_on_shutdown) {
+    terminate_child_on_shutdown_ = terminate_on_shutdown;
+  }
+
  private:
   friend class base::RefCountedThreadSafe<ChildProcessLauncher::Context>;
   friend class ChildProcessLauncher;
@@ -210,6 +215,9 @@ class ChildProcessLauncher::Context
     if (!process_.handle())
       return;
 
+    if (!terminate_child_on_shutdown_)
+      return;
+
     // On Posix, EnsureProcessTerminated can lead to 2 seconds of sleep!  So
     // don't this on the UI/IO threads.
     BrowserThread::PostTask(
@@ -257,6 +265,9 @@ class ChildProcessLauncher::Context
   BrowserThread::ID client_thread_id_;
   base::Process process_;
   bool starting_;
+  // Controls whether the child process should be terminated on browser
+  // shutdown. Default behavior is to terminate the child.
+  bool terminate_child_on_shutdown_;
 
 #if defined(OS_LINUX)
   bool zygote_;
@@ -333,3 +344,10 @@ void ChildProcessLauncher::SetProcessBackgrounded(bool background) {
           &ChildProcessLauncher::Context::SetProcessBackgrounded,
           background));
 }
+
+void ChildProcessLauncher::SetTerminateChildOnShutdown(
+  bool terminate_on_shutdown) {
+  if (context_)
+    context_->set_terminate_child_on_shutdown(terminate_on_shutdown);
+}
+
diff --git a/content/browser/child_process_launcher.h b/content/browser/child_process_launcher.h
index d38c81e..f1f6369 100644
--- a/content/browser/child_process_launcher.h
+++ b/content/browser/child_process_launcher.h
@@ -60,6 +60,10 @@ class ChildProcessLauncher {
   // this after the process has started.
   void SetProcessBackgrounded(bool background);
 
+  // Controls whether the child process should be terminated on browser
+  // shutdown.
+  void SetTerminateChildOnShutdown(bool terminate_on_shutdown);
+
  private:
   class Context;
 
diff --git a/content/browser/plugin_process_host.cc b/content/browser/plugin_process_host.cc
index 2e72f31..9d2e7d4 100644
--- a/content/browser/plugin_process_host.cc
+++ b/content/browser/plugin_process_host.cc
@@ -221,6 +221,12 @@ bool PluginProcessHost::Init(const webkit::npapi::WebPluginInfo& info,
 #endif
       cmd_line);
 
+  // The plugin needs to be shutdown gracefully, i.e. NP_Shutdown needs to be
+  // called on the plugin. The plugin process exits when it receives the
+  // OnChannelError notification indicating that the browser plugin channel has
+  // been destroyed.
+  SetTerminateChildOnShutdown(false);
+
   content::GetContentClient()->browser()->PluginProcessHostCreated(this);
   AddFilter(new ResolveProxyMsgHelper(NULL));
 
diff --git a/content/plugin/plugin_channel.cc b/content/plugin/plugin_channel.cc
index cd2e99f..dd00a75 100644
--- a/content/plugin/plugin_channel.cc
+++ b/content/plugin/plugin_channel.cc
@@ -32,9 +32,19 @@ class PluginReleaseTask : public Task {
   }
 };
 
+class PluginProcessExitTask : public Task {
+ public:
+  void Run() {
+    base::KillProcess(base::GetCurrentProcessHandle(), 0, false);
+  }
+};
+
 // How long we wait before releasing the plugin process.
 const int kPluginReleaseTimeMs = 5 * 60 * 1000;  // 5 minutes
 
+// How long we wait before forcibly shutting down the process.
+const int kPluginProcessTerminateTimeoutMs = 3000;
+
 }  // namespace
 
 // If a sync call to the renderer results in a modal dialog, we need to have a
@@ -88,6 +98,15 @@ class PluginChannel::MessageFilter : public IPC::ChannelProxy::MessageFilter {
  private:
   void OnFilterAdded(IPC::Channel* channel) { channel_ = channel; }
 
+  virtual void OnChannelError() {
+    // Ensure that we don't wait indefinitely for the plugin to shutdown.
+    // as the browser does not terminate plugin processes on shutdown.
+    // We achieve this by posting an exit process task on the IO thread.
+    MessageLoop::current()->PostDelayedTask(FROM_HERE,
+                                            new PluginProcessExitTask(),
+                                            kPluginProcessTerminateTimeoutMs);
+  }
+
   bool OnMessageReceived(const IPC::Message& message) {
     IPC_BEGIN_MESSAGE_MAP(PluginChannel::MessageFilter, message)
       IPC_MESSAGE_HANDLER_DELAY_REPLY(PluginMsg_Init, OnInit)