diff options
author | jam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-03-27 20:45:59 +0000 |
---|---|---|
committer | jam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-03-27 20:45:59 +0000 |
commit | 51d70e07bd5d991420bc0f14ff05f1a9b72d02db (patch) | |
tree | 718c0b24680c1b14eb4570403cc15b8d09b4002d /chrome/plugin | |
parent | 70ac249f04337f0040ad63a72ced111d87fdc73a (diff) | |
download | chromium_src-51d70e07bd5d991420bc0f14ff05f1a9b72d02db.zip chromium_src-51d70e07bd5d991420bc0f14ff05f1a9b72d02db.tar.gz chromium_src-51d70e07bd5d991420bc0f14ff05f1a9b72d02db.tar.bz2 |
Refactor plugin process code which checks with the browser process before shutdown, to avoid races in which the browser process thinks the process is fine to use while it's shutting down. I also removed PluginProcess/WorkerProcess since they didn't have any code in them now.
I removed the plugin process code which waits 10 seconds before shutting itself down. That was a premature optimization, since testing with/without this didn't show any difference (see http://www/~jabdelmalek/chrome/test/plugins/processes.html). In both cases, the plugin on a page would get recreated in less than 100ms, even with reusing or starting a plugin process from scratch. We already spawn new renderer processes on back and forth if it's a different origin, and the plugin will be in the cache anyways.
Review URL: http://codereview.chromium.org/53091
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@12703 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/plugin')
-rw-r--r-- | chrome/plugin/chrome_plugin_host.cc | 6 | ||||
-rw-r--r-- | chrome/plugin/plugin.scons | 3 | ||||
-rw-r--r-- | chrome/plugin/plugin.vcproj | 8 | ||||
-rw-r--r-- | chrome/plugin/plugin_channel.cc | 8 | ||||
-rw-r--r-- | chrome/plugin/plugin_channel_base.cc | 4 | ||||
-rw-r--r-- | chrome/plugin/plugin_main.cc | 5 | ||||
-rw-r--r-- | chrome/plugin/plugin_process.cc | 59 | ||||
-rw-r--r-- | chrome/plugin/plugin_process.h | 38 | ||||
-rw-r--r-- | chrome/plugin/plugin_thread.cc | 17 | ||||
-rw-r--r-- | chrome/plugin/plugin_thread.h | 2 |
10 files changed, 15 insertions, 135 deletions
diff --git a/chrome/plugin/chrome_plugin_host.cc b/chrome/plugin/chrome_plugin_host.cc index 8ac50f2..48f975a 100644 --- a/chrome/plugin/chrome_plugin_host.cc +++ b/chrome/plugin/chrome_plugin_host.cc @@ -7,12 +7,12 @@ #include "base/command_line.h" #include "base/file_util.h" #include "base/message_loop.h" +#include "chrome/common/child_process.h" #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_plugin_lib.h" #include "chrome/common/chrome_plugin_util.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/plugin_messages.h" -#include "chrome/plugin/plugin_process.h" #include "chrome/plugin/plugin_thread.h" #include "chrome/plugin/webplugin_proxy.h" #include "net/base/data_url.h" @@ -255,9 +255,9 @@ void STDCALL CPB_SetKeepProcessAlive(CPID id, CPBool keep_alive) { if (desired_value != g_keep_process_alive) { g_keep_process_alive = desired_value; if (g_keep_process_alive) - PluginProcess::current()->AddRefProcess(); + ChildProcess::current()->AddRefProcess(); else - PluginProcess::current()->ReleaseProcess(); + ChildProcess::current()->ReleaseProcess(); } } diff --git a/chrome/plugin/plugin.scons b/chrome/plugin/plugin.scons index 3cf9e84..5621eeb 100644 --- a/chrome/plugin/plugin.scons +++ b/chrome/plugin/plugin.scons @@ -43,8 +43,6 @@ input_files = ChromeFileList([ 'plugin_channel_base.cc', 'plugin_channel_base.h', 'plugin_main.cc', - 'plugin_process.cc', - 'plugin_process.h', 'plugin_thread.cc', 'plugin_thread.h', '$CHROME_DIR/tools/build/win/precompiled$OBJSUFFIX', @@ -69,7 +67,6 @@ if env.Bit('posix'): 'plugin_channel.cc', 'plugin_channel_base.cc', 'plugin_main.cc', - 'plugin_process.cc', 'plugin_thread.cc', 'webplugin_delegate_stub.cc', 'webplugin_proxy.cc', diff --git a/chrome/plugin/plugin.vcproj b/chrome/plugin/plugin.vcproj index fadc40c..4fd5459 100644 --- a/chrome/plugin/plugin.vcproj +++ b/chrome/plugin/plugin.vcproj @@ -178,14 +178,6 @@ > </File> <File - RelativePath=".\plugin_process.cc" - > - </File> - <File - RelativePath=".\plugin_process.h" - > - </File> - <File RelativePath=".\plugin_thread.cc" > </File> diff --git a/chrome/plugin/plugin_channel.cc b/chrome/plugin/plugin_channel.cc index 125054a..066bda3 100644 --- a/chrome/plugin/plugin_channel.cc +++ b/chrome/plugin/plugin_channel.cc @@ -6,12 +6,12 @@ #include "chrome/plugin/plugin_channel.h" -#include "chrome/common/plugin_messages.h" #include "base/command_line.h" #include "base/process_util.h" #include "base/string_util.h" +#include "chrome/common/child_process.h" +#include "chrome/common/plugin_messages.h" #include "chrome/common/chrome_switches.h" -#include "chrome/plugin/plugin_process.h" #include "chrome/plugin/plugin_thread.h" PluginChannel* PluginChannel::GetPluginChannel(MessageLoop* ipc_message_loop) { @@ -29,13 +29,13 @@ PluginChannel* PluginChannel::GetPluginChannel(MessageLoop* ipc_message_loop) { PluginChannel::PluginChannel() : in_send_(0), off_the_record_(false) { SendUnblockingOnlyDuringDispatch(); - PluginProcess::current()->AddRefProcess(); + ChildProcess::current()->AddRefProcess(); const CommandLine* command_line = CommandLine::ForCurrentProcess(); log_messages_ = command_line->HasSwitch(switches::kLogPluginMessages); } PluginChannel::~PluginChannel() { - PluginProcess::current()->ReleaseProcess(); + ChildProcess::current()->ReleaseProcess(); } bool PluginChannel::Send(IPC::Message* msg) { diff --git a/chrome/plugin/plugin_channel_base.cc b/chrome/plugin/plugin_channel_base.cc index 61b56d5..0e26e86 100644 --- a/chrome/plugin/plugin_channel_base.cc +++ b/chrome/plugin/plugin_channel_base.cc @@ -7,8 +7,8 @@ #include "chrome/plugin/plugin_channel_base.h" #include "base/hash_tables.h" +#include "chrome/common/child_process.h" #include "chrome/common/ipc_sync_message.h" -#include "chrome/plugin/plugin_process.h" typedef base::hash_map<std::wstring, scoped_refptr<PluginChannelBase> > PluginChannelMap; @@ -78,7 +78,7 @@ bool PluginChannelBase::Init(MessageLoop* ipc_message_loop, bool create_pipe_now) { channel_.reset(new IPC::SyncChannel( channel_name_, mode_, this, NULL, ipc_message_loop, create_pipe_now, - PluginProcess::current()->GetShutDownEvent())); + ChildProcess::current()->GetShutDownEvent())); channel_valid_ = true; return true; } diff --git a/chrome/plugin/plugin_main.cc b/chrome/plugin/plugin_main.cc index f6808c1..fe71ea0 100644 --- a/chrome/plugin/plugin_main.cc +++ b/chrome/plugin/plugin_main.cc @@ -6,12 +6,13 @@ #include "base/message_loop.h" #include "base/string_util.h" #include "base/system_monitor.h" +#include "chrome/common/child_process.h" #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/logging_chrome.h" #include "chrome/common/main_function_params.h" #include "chrome/common/win_util.h" -#include "chrome/plugin/plugin_process.h" +#include "chrome/plugin/plugin_thread.h" #include "chrome/test/injection_test_dll.h" #include "sandbox/src/sandbox.h" @@ -54,7 +55,7 @@ int PluginMain(const MainFunctionParams& parameters) { } { - PluginProcess plugin_process; + ChildProcess plugin_process(new PluginThread()); if (!no_sandbox && target_services) { target_services->LowerToken(); } diff --git a/chrome/plugin/plugin_process.cc b/chrome/plugin/plugin_process.cc deleted file mode 100644 index 9d0f567..0000000 --- a/chrome/plugin/plugin_process.cc +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright (c) 2006-2008 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 <windows.h> -#include "chrome/plugin/plugin_process.h" - -#include "base/basictypes.h" -#include "base/scoped_handle.h" -#include "chrome/common/ipc_channel.h" -#include "chrome/common/ipc_message_utils.h" -#include "chrome/common/plugin_messages.h" -#include "chrome/common/render_messages.h" -#include "webkit/glue/webkit_glue.h" - -// How long to wait after there are no more plugin instances before killing the -// process. -static const int kProcessShutdownDelayMs = 10 * 1000; - -// No AddRef required when using PluginProcess with RunnableMethod. This is -// okay since the lifetime of the PluginProcess is always greater than the -// lifetime of PluginThread because it's a member variable. -template <> struct RunnableMethodTraits<PluginProcess> { - static void RetainCallee(PluginProcess*) {} - static void ReleaseCallee(PluginProcess*) {} -}; - -PluginProcess::PluginProcess() - : ChildProcess(new PluginThread()) { -} - -PluginProcess::~PluginProcess() { -} - -// Note: may be called on any thread -void PluginProcess::OnFinalRelease() { - // We override this to have the process linger for a few seconds to - // better accomdate back/forth navigation. This avoids shutting down and - // immediately starting a new plugin process. If a new channel is - // opened in the interim, the current process will not be shutdown. - child_thread()->owner_loop()->PostDelayedTask(FROM_HERE, NewRunnableMethod( - this, &PluginProcess::OnProcessShutdownTimeout), - kProcessShutdownDelayMs); -} - -void PluginProcess::OnProcessShutdownTimeout() { - if (ProcessRefCountIsZero()) { - // The plugin process shutdown sequence is a request response based - // mechanism, where we send out an initial feeler request to the plugin - // process host instance in the browser to verify if it is ok to shutdown - // the plugin process. The browser then sends back a response indicating - // whether it is ok to shutdown. - child_thread()->Send(new PluginProcessHostMsg_ShutdownRequest); - } -} - -void PluginProcess::Shutdown() { - ChildProcess::OnFinalRelease(); -} diff --git a/chrome/plugin/plugin_process.h b/chrome/plugin/plugin_process.h deleted file mode 100644 index 467b46d..0000000 --- a/chrome/plugin/plugin_process.h +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) 2006-2008 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. - -#ifndef CHROME_PLUGIN_PLUGIN_PROCESS_H_ -#define CHROME_PLUGIN_PLUGIN_PROCESS_H_ - -#include "base/file_path.h" -#include "chrome/common/child_process.h" -#include "chrome/plugin/plugin_thread.h" - -// Represents the plugin end of the renderer<->plugin connection. The -// opposite end is the PluginProcessHost. This is a singleton object for -// each plugin. -class PluginProcess : public ChildProcess { - public: - PluginProcess(); - virtual ~PluginProcess(); - - // Invoked when the browser is shutdown. This ensures that the plugin - // process does not hang around waiting for future invocations - // from the browser. - void Shutdown(); - - // Returns a pointer to the PluginProcess singleton instance. - static PluginProcess* current() { - return static_cast<PluginProcess*>(ChildProcess::current()); - } - - private: - - virtual void OnFinalRelease(); - void OnProcessShutdownTimeout(); - - DISALLOW_EVIL_CONSTRUCTORS(PluginProcess); -}; - -#endif // CHROME_PLUGIN_PLUGIN_PROCESS_H_ diff --git a/chrome/plugin/plugin_thread.cc b/chrome/plugin/plugin_thread.cc index 10172e7..9993c37 100644 --- a/chrome/plugin/plugin_thread.cc +++ b/chrome/plugin/plugin_thread.cc @@ -8,6 +8,7 @@ #include "chrome/plugin/plugin_thread.h" #include "base/command_line.h" +#include "chrome/common/child_process.h" #include "chrome/common/chrome_plugin_lib.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/notification_service.h" @@ -15,7 +16,6 @@ #include "chrome/common/render_messages.h" #include "chrome/plugin/chrome_plugin_host.h" #include "chrome/plugin/npobject_util.h" -#include "chrome/plugin/plugin_process.h" #include "chrome/renderer/render_thread.h" #include "net/base/net_errors.h" #include "webkit/glue/plugins/plugin_lib.h" @@ -40,9 +40,7 @@ PluginThread* PluginThread::current() { void PluginThread::OnControlMessageReceived(const IPC::Message& msg) { IPC_BEGIN_MESSAGE_MAP(PluginThread, msg) IPC_MESSAGE_HANDLER(PluginProcessMsg_CreateChannel, OnCreateChannel) - IPC_MESSAGE_HANDLER(PluginProcessMsg_ShutdownResponse, OnShutdownResponse) IPC_MESSAGE_HANDLER(PluginProcessMsg_PluginMessage, OnPluginMessage) - IPC_MESSAGE_HANDLER(PluginProcessMsg_BrowserShutdown, OnBrowserShutdown) IPC_END_MESSAGE_MAP() } @@ -100,27 +98,18 @@ void PluginThread::OnCreateChannel(bool off_the_record) { Send(new PluginProcessHostMsg_ChannelCreated(channel_name)); } -void PluginThread::OnShutdownResponse(bool ok_to_shutdown) { - if (ok_to_shutdown) - PluginProcess::current()->Shutdown(); -} - -void PluginThread::OnBrowserShutdown() { - PluginProcess::current()->Shutdown(); -} - void PluginThread::OnPluginMessage(const std::vector<unsigned char> &data) { // We Add/Release ref here to ensure that something will trigger the // shutdown mechanism for processes started in the absence of renderer's // opening a plugin channel. - PluginProcess::current()->AddRefProcess(); + ChildProcess::current()->AddRefProcess(); ChromePluginLib *chrome_plugin = ChromePluginLib::Find(plugin_path_); if (chrome_plugin) { void *data_ptr = const_cast<void*>(reinterpret_cast<const void*>(&data[0])); uint32 data_len = static_cast<uint32>(data.size()); chrome_plugin->functions().on_message(data_ptr, data_len); } - PluginProcess::current()->ReleaseProcess(); + ChildProcess::current()->ReleaseProcess(); } namespace webkit_glue { diff --git a/chrome/plugin/plugin_thread.h b/chrome/plugin/plugin_thread.h index 1550d5a..9b999e2 100644 --- a/chrome/plugin/plugin_thread.h +++ b/chrome/plugin/plugin_thread.h @@ -30,9 +30,7 @@ class PluginThread : public ChildThread { virtual void CleanUp(); void OnCreateChannel(bool off_the_record); - void OnShutdownResponse(bool ok_to_shutdown); void OnPluginMessage(const std::vector<uint8> &data); - void OnBrowserShutdown(); scoped_ptr<NotificationService> notification_service_; |