Replace --ppapi-keep-alive-throttle command line switch with IPC para…

…meter. 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
marcosholgado · Mar 12, 2014 · 96282a0 · 96282a0
1 parent e4e522d
commit 96282a0
Show file tree

Hide file tree

Showing 14 changed files with 75 additions and 45 deletions.
diff --git a/chrome/browser/extensions/app_background_page_apitest.cc b/chrome/browser/extensions/app_background_page_apitest.cc
@@ -20,14 +20,14 @@
 #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"
 #include "extensions/common/extension.h"
 #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
@@ -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 <fcntl.h>
 
 #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
@@ -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
@@ -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
@@ -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
@@ -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<PluginGlobals> weak_factory_;

diff --git 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<PP_Instance> instances_;
   std::map<uint32, PluginDispatcher*> 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
@@ -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
@@ -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
@@ -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
@@ -2,14 +2,18 @@
 // 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 {
 
 // 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
@@ -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<std::string> switch_names;

diff --git 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
@@ -10,7 +10,6 @@
 namespace switches {
 
 PPAPI_SHARED_EXPORT extern const char kEnablePepperTesting[];
-PPAPI_SHARED_EXPORT extern const char kPpapiKeepAliveThrottle[];
 
 }  // namespace switches