Commit 9e3df998 authored by nsylvain@chromium.org's avatar nsylvain@chromium.org

Revert 86517 - Don't terminate plugin processes from the browser during...

Revert 86517 - 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

TBR=ananta@chromium.org
Review URL: http://codereview.chromium.org/7065048

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86532 0039d316-1c4b-4281-b951-d872f2087c98
parent e7a811df
...@@ -109,11 +109,6 @@ void BrowserChildProcessHost::ForceShutdown() { ...@@ -109,11 +109,6 @@ void BrowserChildProcessHost::ForceShutdown() {
ChildProcessHost::ForceShutdown(); ChildProcessHost::ForceShutdown();
} }
void BrowserChildProcessHost::SetTerminateChildOnShutdown(
bool terminate_on_shutdown) {
child_process_->SetTerminateChildOnShutdown(terminate_on_shutdown);
}
void BrowserChildProcessHost::Notify(NotificationType type) { void BrowserChildProcessHost::Notify(NotificationType type) {
BrowserThread::PostTask( BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE, new ChildNotificationTask(type, this)); BrowserThread::UI, FROM_HERE, new ChildNotificationTask(type, this));
......
...@@ -92,10 +92,6 @@ class BrowserChildProcessHost : public ChildProcessHost, ...@@ -92,10 +92,6 @@ class BrowserChildProcessHost : public ChildProcessHost,
// the host list. Calls ChildProcessHost::ForceShutdown // the host list. Calls ChildProcessHost::ForceShutdown
virtual void 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: private:
// By using an internal class as the ChildProcessLauncher::Client, we can // By using an internal class as the ChildProcessLauncher::Client, we can
// intercept OnProcessLaunched and do our own processing before // intercept OnProcessLaunched and do our own processing before
......
...@@ -44,8 +44,7 @@ class ChildProcessLauncher::Context ...@@ -44,8 +44,7 @@ class ChildProcessLauncher::Context
Context() Context()
: client_(NULL), : client_(NULL),
client_thread_id_(BrowserThread::UI), client_thread_id_(BrowserThread::UI),
starting_(true), starting_(true)
terminate_child_on_shutdown_(true)
#if defined(OS_LINUX) #if defined(OS_LINUX)
, zygote_(false) , zygote_(false)
#endif #endif
...@@ -88,10 +87,6 @@ class ChildProcessLauncher::Context ...@@ -88,10 +87,6 @@ class ChildProcessLauncher::Context
client_ = NULL; client_ = NULL;
} }
void set_terminate_child_on_shutdown(bool terminate_on_shutdown) {
terminate_child_on_shutdown_ = terminate_on_shutdown;
}
private: private:
friend class base::RefCountedThreadSafe<ChildProcessLauncher::Context>; friend class base::RefCountedThreadSafe<ChildProcessLauncher::Context>;
friend class ChildProcessLauncher; friend class ChildProcessLauncher;
...@@ -215,9 +210,6 @@ class ChildProcessLauncher::Context ...@@ -215,9 +210,6 @@ class ChildProcessLauncher::Context
if (!process_.handle()) if (!process_.handle())
return; return;
if (!terminate_child_on_shutdown_)
return;
// On Posix, EnsureProcessTerminated can lead to 2 seconds of sleep! So // On Posix, EnsureProcessTerminated can lead to 2 seconds of sleep! So
// don't this on the UI/IO threads. // don't this on the UI/IO threads.
BrowserThread::PostTask( BrowserThread::PostTask(
...@@ -265,9 +257,6 @@ class ChildProcessLauncher::Context ...@@ -265,9 +257,6 @@ class ChildProcessLauncher::Context
BrowserThread::ID client_thread_id_; BrowserThread::ID client_thread_id_;
base::Process process_; base::Process process_;
bool starting_; 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) #if defined(OS_LINUX)
bool zygote_; bool zygote_;
...@@ -344,10 +333,3 @@ void ChildProcessLauncher::SetProcessBackgrounded(bool background) { ...@@ -344,10 +333,3 @@ void ChildProcessLauncher::SetProcessBackgrounded(bool background) {
&ChildProcessLauncher::Context::SetProcessBackgrounded, &ChildProcessLauncher::Context::SetProcessBackgrounded,
background)); background));
} }
void ChildProcessLauncher::SetTerminateChildOnShutdown(
bool terminate_on_shutdown) {
if (context_)
context_->set_terminate_child_on_shutdown(terminate_on_shutdown);
}
...@@ -60,10 +60,6 @@ class ChildProcessLauncher { ...@@ -60,10 +60,6 @@ class ChildProcessLauncher {
// this after the process has started. // this after the process has started.
void SetProcessBackgrounded(bool background); void SetProcessBackgrounded(bool background);
// Controls whether the child process should be terminated on browser
// shutdown.
void SetTerminateChildOnShutdown(bool terminate_on_shutdown);
private: private:
class Context; class Context;
......
...@@ -221,12 +221,6 @@ bool PluginProcessHost::Init(const webkit::npapi::WebPluginInfo& info, ...@@ -221,12 +221,6 @@ bool PluginProcessHost::Init(const webkit::npapi::WebPluginInfo& info,
#endif #endif
cmd_line); 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); content::GetContentClient()->browser()->PluginProcessHostCreated(this);
AddFilter(new ResolveProxyMsgHelper(NULL)); AddFilter(new ResolveProxyMsgHelper(NULL));
......
...@@ -32,19 +32,9 @@ class PluginReleaseTask : public Task { ...@@ -32,19 +32,9 @@ 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. // How long we wait before releasing the plugin process.
const int kPluginReleaseTimeMs = 5 * 60 * 1000; // 5 minutes const int kPluginReleaseTimeMs = 5 * 60 * 1000; // 5 minutes
// How long we wait before forcibly shutting down the process.
const int kPluginProcessTerminateTimeoutMs = 3000;
} // namespace } // namespace
// If a sync call to the renderer results in a modal dialog, we need to have a // If a sync call to the renderer results in a modal dialog, we need to have a
...@@ -98,15 +88,6 @@ class PluginChannel::MessageFilter : public IPC::ChannelProxy::MessageFilter { ...@@ -98,15 +88,6 @@ class PluginChannel::MessageFilter : public IPC::ChannelProxy::MessageFilter {
private: private:
void OnFilterAdded(IPC::Channel* channel) { channel_ = channel; } 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) { bool OnMessageReceived(const IPC::Message& message) {
IPC_BEGIN_MESSAGE_MAP(PluginChannel::MessageFilter, message) IPC_BEGIN_MESSAGE_MAP(PluginChannel::MessageFilter, message)
IPC_MESSAGE_HANDLER_DELAY_REPLY(PluginMsg_Init, OnInit) IPC_MESSAGE_HANDLER_DELAY_REPLY(PluginMsg_Init, OnInit)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment