From 96282a0c6ef6ceabac242bca230b41a9a3b51188 Mon Sep 17 00:00:00 2001 From: "scheib@chromium.org" Date: Wed, 12 Mar 2014 05:08:50 +0000 Subject: [PATCH] Replace --ppapi-keep-alive-throttle command line switch with IPC parameter. Part of a broad effort to reduce the number of command line switches. BUG=350510 Review URL: https://codereview.chromium.org/191633002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@256436 0039d316-1c4b-4281-b951-d872f2087c98 --- .../extensions/app_background_page_apitest.cc | 5 ++-- components/nacl/browser/nacl_process_host.cc | 15 ++++++++++-- components/nacl/browser/nacl_process_host.h | 6 +++++ ppapi/ppapi_shared.gypi | 2 ++ ppapi/proxy/plugin_globals.cc | 24 +++++++------------ ppapi/proxy/plugin_globals.h | 5 ++-- ppapi/proxy/plugin_main_irt.cc | 20 ++++------------ ppapi/proxy/ppapi_messages.h | 1 + ppapi/shared_impl/ppapi_constants.cc | 11 +++++++++ ppapi/shared_impl/ppapi_constants.h | 20 ++++++++++++++++ ppapi/shared_impl/ppapi_nacl_plugin_args.cc | 6 ++++- ppapi/shared_impl/ppapi_nacl_plugin_args.h | 1 + ppapi/shared_impl/ppapi_switches.cc | 3 --- ppapi/shared_impl/ppapi_switches.h | 1 - 14 files changed, 75 insertions(+), 45 deletions(-) create mode 100644 ppapi/shared_impl/ppapi_constants.cc create mode 100644 ppapi/shared_impl/ppapi_constants.h diff --git a/chrome/browser/extensions/app_background_page_apitest.cc b/chrome/browser/extensions/app_background_page_apitest.cc index c423a8c7cd097f..b10fac403fad14 100644 --- a/chrome/browser/extensions/app_background_page_apitest.cc +++ b/chrome/browser/extensions/app_background_page_apitest.cc @@ -20,6 +20,7 @@ #include "chrome/browser/ui/extensions/application_launch.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" +#include "components/nacl/browser/nacl_process_host.h" #include "content/public/browser/notification_service.h" #include "content/public/test/test_notification_tracker.h" #include "content/public/test/test_utils.h" @@ -27,7 +28,6 @@ #include "extensions/common/switches.h" #include "net/dns/mock_host_resolver.h" #include "net/test/embedded_test_server/embedded_test_server.h" -#include "ppapi/shared_impl/ppapi_switches.h" #if defined(OS_MACOSX) #include "base/mac/scoped_nsautorelease_pool.h" @@ -131,8 +131,7 @@ class AppBackgroundPageNaClTest : public AppBackgroundPageApiTest { virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE { AppBackgroundPageApiTest::SetUpCommandLine(command_line); - command_line->AppendSwitchASCII( - switches::kPpapiKeepAliveThrottle, "50"); + nacl::NaClProcessHost::SetPpapiKeepAliveThrottleForTesting(50); command_line->AppendSwitchASCII( extensions::switches::kEventPageIdleTime, "1000"); command_line->AppendSwitchASCII( diff --git a/components/nacl/browser/nacl_process_host.cc b/components/nacl/browser/nacl_process_host.cc index b16b6d86c1fac2..b8bb6bde096ad3 100644 --- a/components/nacl/browser/nacl_process_host.cc +++ b/components/nacl/browser/nacl_process_host.cc @@ -48,10 +48,11 @@ #include "ppapi/host/host_factory.h" #include "ppapi/host/ppapi_host.h" #include "ppapi/proxy/ppapi_messages.h" +#include "ppapi/shared_impl/ppapi_constants.h" #include "ppapi/shared_impl/ppapi_nacl_plugin_args.h" -#include "ppapi/shared_impl/ppapi_switches.h" #if defined(OS_POSIX) + #include #include "ipc/ipc_channel_posix.h" @@ -233,6 +234,9 @@ struct NaClProcessHost::NaClInternal { // ----------------------------------------------------------------------------- +unsigned NaClProcessHost::keepalive_throttle_interval_milliseconds_ = + ppapi::kKeepaliveThrottleIntervalDefaultMilliseconds; + NaClProcessHost::NaClProcessHost(const GURL& manifest_url, int render_view_id, uint32 permission_bits, @@ -354,6 +358,12 @@ void NaClProcessHost::EarlyStartup() { NaClBrowser::GetDelegate()->SetDebugPatterns(nacl_debug_mask); } +// static +void NaClProcessHost::SetPpapiKeepAliveThrottleForTesting( + unsigned milliseconds) { + keepalive_throttle_interval_milliseconds_ = milliseconds; +} + void NaClProcessHost::Launch( NaClHostMessageFilter* nacl_host_message_filter, IPC::Message* reply_msg, @@ -858,10 +868,11 @@ void NaClProcessHost::OnPpapiChannelsCreated( ppapi::PpapiNaClPluginArgs args; args.off_the_record = nacl_host_message_filter_->off_the_record(); args.permissions = permissions_; + args.keepalive_throttle_interval_milliseconds = + keepalive_throttle_interval_milliseconds_; CommandLine* cmdline = CommandLine::ForCurrentProcess(); DCHECK(cmdline); std::string flag_whitelist[] = { - switches::kPpapiKeepAliveThrottle, switches::kV, switches::kVModule, }; diff --git a/components/nacl/browser/nacl_process_host.h b/components/nacl/browser/nacl_process_host.h index 9df7ade359b44d..eab94cde6b4471 100644 --- a/components/nacl/browser/nacl_process_host.h +++ b/components/nacl/browser/nacl_process_host.h @@ -78,6 +78,9 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate { // Do any minimal work that must be done at browser startup. static void EarlyStartup(); + // Specifies throttling time in milliseconds for PpapiHostMsg_Keepalive IPCs. + static void SetPpapiKeepAliveThrottleForTesting(unsigned milliseconds); + // Initialize the new NaCl process. Result is returned by sending ipc // message reply_msg. void Launch(NaClHostMessageFilter* nacl_host_message_filter, @@ -220,6 +223,9 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate { int render_view_id_; + // Throttling time in milliseconds for PpapiHostMsg_Keepalive IPCs. + static unsigned keepalive_throttle_interval_milliseconds_; + DISALLOW_COPY_AND_ASSIGN(NaClProcessHost); }; diff --git a/ppapi/ppapi_shared.gypi b/ppapi/ppapi_shared.gypi index 620a6ebab592cc..3b9574adaf76f3 100644 --- a/ppapi/ppapi_shared.gypi +++ b/ppapi/ppapi_shared.gypi @@ -46,6 +46,8 @@ 'shared_impl/media_stream_video_track_shared.cc', 'shared_impl/platform_file.cc', 'shared_impl/platform_file.h', + 'shared_impl/ppapi_constants.cc', + 'shared_impl/ppapi_constants.h', 'shared_impl/ppapi_globals.cc', 'shared_impl/ppapi_globals.h', 'shared_impl/ppapi_nacl_plugin_args.cc', diff --git a/ppapi/proxy/plugin_globals.cc b/ppapi/proxy/plugin_globals.cc index d34adf1e634798..d361ae1191950b 100644 --- a/ppapi/proxy/plugin_globals.cc +++ b/ppapi/proxy/plugin_globals.cc @@ -13,15 +13,10 @@ #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/proxy/ppb_message_loop_proxy.h" #include "ppapi/proxy/resource_reply_thread_registrar.h" +#include "ppapi/shared_impl/ppapi_constants.h" #include "ppapi/shared_impl/proxy_lock.h" #include "ppapi/thunk/enter.h" -namespace { - -const int kKeepaliveThrottleIntervalDefault = 5000; - -} // namespace - namespace ppapi { namespace proxy { @@ -64,7 +59,7 @@ PluginGlobals::PluginGlobals() new ResourceReplyThreadRegistrar(GetMainThreadMessageLoop())), plugin_recently_active_(false), keepalive_throttle_interval_milliseconds_( - kKeepaliveThrottleIntervalDefault), + ppapi::kKeepaliveThrottleIntervalDefaultMilliseconds), weak_factory_(this) { DCHECK(!plugin_globals_); plugin_globals_ = this; @@ -85,7 +80,7 @@ PluginGlobals::PluginGlobals(PerThreadForTest per_thread_for_test) new ResourceReplyThreadRegistrar(GetMainThreadMessageLoop())), plugin_recently_active_(false), keepalive_throttle_interval_milliseconds_( - kKeepaliveThrottleIntervalDefault), + kKeepaliveThrottleIntervalDefaultMilliseconds), weak_factory_(this) { DCHECK(!plugin_globals_); } @@ -186,12 +181,13 @@ void PluginGlobals::MarkPluginIsActive() { if (!GetBrowserSender() || !base::MessageLoop::current()) return; GetBrowserSender()->Send(new PpapiHostMsg_Keepalive()); - - GetMainThreadMessageLoop()->PostDelayedTask(FROM_HERE, + DCHECK(keepalive_throttle_interval_milliseconds_); + GetMainThreadMessageLoop()->PostDelayedTask( + FROM_HERE, RunWhileLocked(base::Bind(&PluginGlobals::OnReleaseKeepaliveThrottle, weak_factory_.GetWeakPtr())), base::TimeDelta::FromMilliseconds( - keepalive_throttle_interval_milliseconds())); + keepalive_throttle_interval_milliseconds_)); } } @@ -225,11 +221,7 @@ MessageLoopResource* PluginGlobals::loop_for_main_thread() { return loop_for_main_thread_.get(); } -int PluginGlobals::keepalive_throttle_interval_milliseconds() const { - return keepalive_throttle_interval_milliseconds_; -} - -void PluginGlobals::set_keepalive_throttle_interval_milliseconds(int i) { +void PluginGlobals::set_keepalive_throttle_interval_milliseconds(unsigned i) { keepalive_throttle_interval_milliseconds_ = i; } diff --git a/ppapi/proxy/plugin_globals.h b/ppapi/proxy/plugin_globals.h index dac33b613fe52c..5dc614d239bd72 100644 --- a/ppapi/proxy/plugin_globals.h +++ b/ppapi/proxy/plugin_globals.h @@ -137,8 +137,7 @@ class PPAPI_PROXY_EXPORT PluginGlobals : public PpapiGlobals { // Interval to limit how many IPC messages are sent indicating that the plugin // is active and should be kept alive. The value must be smaller than any // threshold used to kill inactive plugins by the embedder host. - int keepalive_throttle_interval_milliseconds() const; - void set_keepalive_throttle_interval_milliseconds(int i); + void set_keepalive_throttle_interval_milliseconds(unsigned i); private: class BrowserSender; @@ -183,7 +182,7 @@ class PPAPI_PROXY_EXPORT PluginGlobals : public PpapiGlobals { // all considered active. bool plugin_recently_active_; - int keepalive_throttle_interval_milliseconds_; + unsigned keepalive_throttle_interval_milliseconds_; // Member variables should appear before the WeakPtrFactory, see weak_ptr.h. base::WeakPtrFactory weak_factory_; diff --git a/ppapi/proxy/plugin_main_irt.cc b/ppapi/proxy/plugin_main_irt.cc index a3ea6c1835ca5f..980cc6ee8ab71d 100644 --- a/ppapi/proxy/plugin_main_irt.cc +++ b/ppapi/proxy/plugin_main_irt.cc @@ -32,7 +32,6 @@ #include "ppapi/proxy/plugin_message_filter.h" #include "ppapi/proxy/plugin_proxy_delegate.h" #include "ppapi/proxy/resource_reply_thread_registrar.h" -#include "ppapi/shared_impl/ppapi_switches.h" #include "ppapi/shared_impl/ppb_audio_shared.h" #if defined(__native_client__) @@ -116,8 +115,6 @@ class PpapiDispatcher : public PluginDispatcher::PluginDelegate, void OnMsgInitializeNaClDispatcher(const ppapi::PpapiNaClPluginArgs& args); void OnPluginDispatcherMessageReceived(const IPC::Message& msg); - void SetPpapiKeepAliveThrottleFromCommandLine(); - std::set instances_; std::map plugin_dispatchers_; uint32 next_plugin_dispatcher_id_; @@ -249,7 +246,10 @@ void PpapiDispatcher::OnMsgInitializeNaClDispatcher( logging::LoggingSettings settings; settings.logging_dest = logging::LOG_TO_SYSTEM_DEBUG_LOG; logging::InitLogging(settings); - SetPpapiKeepAliveThrottleFromCommandLine(); + + ppapi::proxy::PluginGlobals::Get() + ->set_keepalive_throttle_interval_milliseconds( + args.keepalive_throttle_interval_milliseconds); // Tell the process-global GetInterface which interfaces it can return to the // plugin. @@ -294,18 +294,6 @@ void PpapiDispatcher::OnPluginDispatcherMessageReceived( dispatcher->second->OnMessageReceived(msg); } -void PpapiDispatcher::SetPpapiKeepAliveThrottleFromCommandLine() { - unsigned keepalive_throttle_interval_milliseconds = 0; - if (base::StringToUint( - CommandLine::ForCurrentProcess()->GetSwitchValueASCII( - switches::kPpapiKeepAliveThrottle), - &keepalive_throttle_interval_milliseconds)) { - ppapi::proxy::PluginGlobals::Get()-> - set_keepalive_throttle_interval_milliseconds( - keepalive_throttle_interval_milliseconds); - } -} - } // namespace void SetIPCFileDescriptors(int ipc_browser_fd, int ipc_renderer_fd) { diff --git a/ppapi/proxy/ppapi_messages.h b/ppapi/proxy/ppapi_messages.h index 71f791e4fd23d5..b05b56b11d9390 100644 --- a/ppapi/proxy/ppapi_messages.h +++ b/ppapi/proxy/ppapi_messages.h @@ -364,6 +364,7 @@ IPC_STRUCT_TRAITS_END() IPC_STRUCT_TRAITS_BEGIN(ppapi::PpapiNaClPluginArgs) IPC_STRUCT_TRAITS_MEMBER(off_the_record) IPC_STRUCT_TRAITS_MEMBER(permissions) + IPC_STRUCT_TRAITS_MEMBER(keepalive_throttle_interval_milliseconds) IPC_STRUCT_TRAITS_MEMBER(switch_names) IPC_STRUCT_TRAITS_MEMBER(switch_values) IPC_STRUCT_TRAITS_END() diff --git a/ppapi/shared_impl/ppapi_constants.cc b/ppapi/shared_impl/ppapi_constants.cc new file mode 100644 index 00000000000000..6962513de0506f --- /dev/null +++ b/ppapi/shared_impl/ppapi_constants.cc @@ -0,0 +1,11 @@ +// Copyright 2014 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 "ppapi/shared_impl/ppapi_constants.h" + +namespace ppapi { + +const unsigned kKeepaliveThrottleIntervalDefaultMilliseconds = 5000; + +} // namespace ppapi diff --git a/ppapi/shared_impl/ppapi_constants.h b/ppapi/shared_impl/ppapi_constants.h new file mode 100644 index 00000000000000..9faf18d8457adf --- /dev/null +++ b/ppapi/shared_impl/ppapi_constants.h @@ -0,0 +1,20 @@ +// Copyright 2014 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 PPAPI_SHARED_IMPL_PPAPI_CONSTANTS_H_ +#define PPAPI_SHARED_IMPL_PPAPI_CONSTANTS_H_ + +#include "ppapi/shared_impl/ppapi_shared_export.h" + +namespace ppapi { + +// Default interval to space out IPC messages sent indicating that a plugin is +// active and should be kept alive. The value must be smaller than any threshold +// used to kill inactive plugins by the embedder host. +PPAPI_SHARED_EXPORT extern const unsigned + kKeepaliveThrottleIntervalDefaultMilliseconds; + +} // namespace ppapi + +#endif // PPAPI_SHARED_IMPL_PPAPI_CONSTANTS_H_ diff --git a/ppapi/shared_impl/ppapi_nacl_plugin_args.cc b/ppapi/shared_impl/ppapi_nacl_plugin_args.cc index 92df690ceb3a39..4dd511040c49eb 100644 --- a/ppapi/shared_impl/ppapi_nacl_plugin_args.cc +++ b/ppapi/shared_impl/ppapi_nacl_plugin_args.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "ppapi/shared_impl/ppapi_constants.h" #include "ppapi/shared_impl/ppapi_nacl_plugin_args.h" namespace ppapi { @@ -9,7 +10,10 @@ namespace ppapi { // We must provide explicit definitions of these functions for builds on // Windows. PpapiNaClPluginArgs::PpapiNaClPluginArgs() - : off_the_record(false), supports_dev_channel(false) {} + : off_the_record(false), + supports_dev_channel(false), + keepalive_throttle_interval_milliseconds( + kKeepaliveThrottleIntervalDefaultMilliseconds) {} PpapiNaClPluginArgs::~PpapiNaClPluginArgs() {} diff --git a/ppapi/shared_impl/ppapi_nacl_plugin_args.h b/ppapi/shared_impl/ppapi_nacl_plugin_args.h index 2f18c8e025dc6d..993131d3b09ed7 100644 --- a/ppapi/shared_impl/ppapi_nacl_plugin_args.h +++ b/ppapi/shared_impl/ppapi_nacl_plugin_args.h @@ -20,6 +20,7 @@ struct PPAPI_SHARED_EXPORT PpapiNaClPluginArgs { bool off_the_record; PpapiPermissions permissions; bool supports_dev_channel; + unsigned keepalive_throttle_interval_milliseconds; // Switches from the command-line. std::vector switch_names; diff --git a/ppapi/shared_impl/ppapi_switches.cc b/ppapi/shared_impl/ppapi_switches.cc index 2a3f1c28600874..84ba25034ecb4a 100644 --- a/ppapi/shared_impl/ppapi_switches.cc +++ b/ppapi/shared_impl/ppapi_switches.cc @@ -9,7 +9,4 @@ namespace switches { // Enables the testing interface for PPAPI. const char kEnablePepperTesting[] = "enable-pepper-testing"; -// Specifies throttling time in milliseconds for PpapiHostMsg_Keepalive IPCs. -const char kPpapiKeepAliveThrottle[] = "ppapi-keep-alive-throttle"; - } // namespace switches diff --git a/ppapi/shared_impl/ppapi_switches.h b/ppapi/shared_impl/ppapi_switches.h index 4fb9c3481981a5..a5c8e9d461d23f 100644 --- a/ppapi/shared_impl/ppapi_switches.h +++ b/ppapi/shared_impl/ppapi_switches.h @@ -10,7 +10,6 @@ namespace switches { PPAPI_SHARED_EXPORT extern const char kEnablePepperTesting[]; -PPAPI_SHARED_EXPORT extern const char kPpapiKeepAliveThrottle[]; } // namespace switches