Test Keeping NaCl plugins used in app background pages alive when active.

(Re-landing with fixes from https://codereview.chromium.org/112663007/ )

Activity in Native Client plugins results in IPC messages
sent to the BrowserPpapiHostImpl and routed to call
extensions::ProcessManager::KeepaliveImpulse.

Testing patch, builds on implementation:
https://codereview.chromium.org/61063003/

Design doc:
https://docs.google.com/a/chromium.org/document/d/1mI0lS1rfAf-BAGLmWAEcWy37Xq9dOvgfMx8OqeUMXts/edit#

BUG=298339
No change to mseaborn owned file since last LGTM:

R=yzshen@chromium.org
TBR=mseaborn@chromium.org

Review URL: https://codereview.chromium.org/125353006

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

chrome/browser/extensions/DEPS

@@ -12,4 +12,7 @@ include_rules = [
   # DO NOT ADD ANY MORE ITEMS TO THE LIST BELOW!
   "!chrome/browser/ui/views/extensions/extension_view_views.h",
   # DO NOT ADD ANY MORE ITEMS TO THE ABOVE LIST!
+
+  # For access to testing command line switches.
+  "+ppapi/shared_impl",
 ]

chrome/browser/extensions/app_background_page_apitest.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 "base/path_service.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/background/background_contents_service.h"
@@ -11,10 +12,13 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_test_message_listener.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/extensions/application_launch.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/test/test_notification_tracker.h"
@@ -23,6 +27,7 @@
 #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"
@@ -109,6 +114,88 @@ class AppBackgroundPageApiTest : public ExtensionApiTest {
   base::ScopedTempDir app_dir_;
 };
 
+namespace {
+
+// Fixture to assist in testing v2 app background pages containing
+// Native Client embeds.
+class AppBackgroundPageNaClTest : public AppBackgroundPageApiTest {
+ public:
+  AppBackgroundPageNaClTest()
+      : extension_(NULL) {
+    PathService::Get(chrome::DIR_GEN_TEST_DATA, &app_dir_);
+    app_dir_ = app_dir_.AppendASCII(
+        "ppapi/tests/extensions/background_keepalive/newlib");
+  }
+  virtual ~AppBackgroundPageNaClTest() {
+  }
+
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    AppBackgroundPageApiTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitchASCII(
+        switches::kPpapiKeepAliveThrottle, "50");
+    command_line->AppendSwitchASCII(
+        extensions::switches::kEventPageIdleTime, "1000");
+    command_line->AppendSwitchASCII(
+        extensions::switches::kEventPageSuspendingTime, "1000");
+  }
+
+  const Extension* extension() { return extension_; }
+
+ protected:
+  void LaunchTestingApp() {
+    extension_ = LoadExtension(app_dir_);
+    ASSERT_TRUE(extension_);
+  }
+
+ private:
+  base::FilePath app_dir_;
+  const Extension* extension_;
+};
+
+// Produces an extensions::ProcessManager::ImpulseCallbackForTesting callback
+// that will match a specified goal and can be waited on.
+class ImpulseCallbackCounter {
+ public:
+  explicit ImpulseCallbackCounter(const std::string& extension_id)
+      : observed_(0),
+        goal_(0),
+        extension_id_(extension_id) {
+  }
+
+  extensions::ProcessManager::ImpulseCallbackForTesting
+      SetGoalAndGetCallback(int goal) {
+    observed_ = 0;
+    goal_ = goal;
+    message_loop_runner_ = new content::MessageLoopRunner();
+    return base::Bind(&ImpulseCallbackCounter::ImpulseCallback,
+                      base::Unretained(this),
+                      message_loop_runner_->QuitClosure(),
+                      extension_id_);
+  }
+
+  void Wait() {
+    message_loop_runner_->Run();
+  }
+ private:
+  void ImpulseCallback(
+      const base::Closure& quit_callback,
+      const std::string& extension_id_from_test,
+      const std::string& extension_id_from_manager) {
+    if (extension_id_from_test == extension_id_from_manager) {
+      if (++observed_ >= goal_) {
+        quit_callback.Run();
+      }
+    }
+  }
+
+  int observed_;
+  int goal_;
+  const std::string extension_id_;
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+};
+
+}  // namespace
+
 // Disable on Mac only.  http://crbug.com/95139
 #if defined(OS_MACOSX)
 #define MAYBE_Basic DISABLED_Basic
@@ -503,3 +590,50 @@ IN_PROC_BROWSER_TEST_F(AppBackgroundPageApiTest, UnloadExtensionWhileHidden) {
   content::RunAllPendingInMessageLoop();
   ASSERT_TRUE(WaitForBackgroundMode(false));
 }
+
+// Verify active NaCl embeds cause many keepalive impulses to be sent.
+IN_PROC_BROWSER_TEST_F(AppBackgroundPageNaClTest, BackgroundKeepaliveActive) {
+  ExtensionTestMessageListener nacl_modules_loaded("nacl_modules_loaded", true);
+  LaunchTestingApp();
+  extensions::ProcessManager* manager =
+    extensions::ExtensionSystem::Get(browser()->profile())->process_manager();
+  ImpulseCallbackCounter active_impulse_counter(extension()->id());
+  EXPECT_TRUE(nacl_modules_loaded.WaitUntilSatisfied());
+
+  // Target .5 seconds: .5 seconds / 50ms throttle * 2 embeds == 20 impulses.
+  manager->SetKeepaliveImpulseCallbackForTesting(
+      active_impulse_counter.SetGoalAndGetCallback(20));
+  active_impulse_counter.Wait();
+
+  // Clear callback to free reference to message loop in ImpulseCallbackCounter.
+  manager->SetKeepaliveImpulseCallbackForTesting(
+      extensions::ProcessManager::ImpulseCallbackForTesting());
+}
+
+// Verify that nacl modules that go idle will not send keepalive impulses.
+// Disabled on windows due to Win XP failures:
+// DesktopRootWindowHostWin::HandleCreate not implemented. crbug.com/331954
+#if defined(OS_WIN)
+#define MAYBE_BackgroundKeepaliveIdle DISABLED_BackgroundKeepaliveIdle
+#else
+#define MAYBE_BackgroundKeepaliveIdle BackgroundKeepaliveIdle
+#endif
+IN_PROC_BROWSER_TEST_F(AppBackgroundPageNaClTest,
+                       MAYBE_BackgroundKeepaliveIdle) {
+  ExtensionTestMessageListener nacl_modules_loaded("nacl_modules_loaded", true);
+  LaunchTestingApp();
+  extensions::ProcessManager* manager =
+    extensions::ExtensionSystem::Get(browser()->profile())->process_manager();
+  ImpulseCallbackCounter idle_impulse_counter(extension()->id());
+  EXPECT_TRUE(nacl_modules_loaded.WaitUntilSatisfied());
+
+  manager->SetKeepaliveImpulseDecrementCallbackForTesting(
+      idle_impulse_counter.SetGoalAndGetCallback(1));
+  nacl_modules_loaded.Reply("be idle");
+  idle_impulse_counter.Wait();
+
+  // Clear callback to free reference to message loop in ImpulseCallbackCounter.
+  manager->SetKeepaliveImpulseDecrementCallbackForTesting(
+      extensions::ProcessManager::ImpulseCallbackForTesting());
+}
+

chrome/chrome_tests.gypi

@@ -1685,7 +1685,8 @@
                 'test/data/nacl/nacl_test_data.gyp:*',
                 '../ppapi/native_client/native_client.gyp:nacl_irt',
                 '../ppapi/ppapi_untrusted.gyp:ppapi_nacl_tests',
-                '../ppapi/tests/extensions/extensions.gyp:ppapi_tests_extensions_socket',
+                '../ppapi/tests/extensions/extensions.gyp:ppapi_tests_extensions_background_keepalive',
+                '../ppapi/tests/extensions/extensions.gyp:ppapi_tests_extensions_socket'
               ],
               'conditions': [
                 ['OS=="linux"', {

components/nacl/browser/nacl_process_host.cc

@@ -809,7 +809,11 @@ void NaClProcessHost::OnPpapiBrowserChannelCreated(
     args.permissions = permissions_;
     CommandLine* cmdline = CommandLine::ForCurrentProcess();
     DCHECK(cmdline);
-    std::string flag_whitelist[] = {switches::kV, switches::kVModule};
+    std::string flag_whitelist[] = {
+      switches::kPpapiKeepAliveThrottle,
+      switches::kV,
+      switches::kVModule,
+    };
     for (size_t i = 0; i < arraysize(flag_whitelist); ++i) {
       std::string value = cmdline->GetSwitchValueASCII(flag_whitelist[i]);
       if (!value.empty()) {

extensions/browser/process_manager.cc

@@ -442,6 +442,9 @@ void ProcessManager::KeepaliveImpulse(const Extension* extension) {
       IncrementLazyKeepaliveCount(extension);
     }
   }
+
+  if (!keepalive_impulse_callback_for_testing_.is_null())
+    keepalive_impulse_callback_for_testing_.Run(extension->id());
 }
 
 // DecrementLazyKeepaliveCount is called when no calls to KeepaliveImpulse
@@ -454,8 +457,11 @@ void ProcessManager::OnKeepaliveImpulseCheck() {
   for (BackgroundPageDataMap::iterator i = background_page_data_.begin();
        i != background_page_data_.end();
        ++i) {
-    if (i->second.previous_keepalive_impulse && !i->second.keepalive_impulse)
+    if (i->second.previous_keepalive_impulse && !i->second.keepalive_impulse) {
       DecrementLazyKeepaliveCount(i->first);
+      if (!keepalive_impulse_decrement_callback_for_testing_.is_null())
+        keepalive_impulse_decrement_callback_for_testing_.Run(i->first);
+    }
 
     i->second.previous_keepalive_impulse = i->second.keepalive_impulse;
     i->second.keepalive_impulse = false;
@@ -576,6 +582,16 @@ content::BrowserContext* ProcessManager::GetBrowserContext() const {
   return site_instance_->GetBrowserContext();
 }
 
+void ProcessManager::SetKeepaliveImpulseCallbackForTesting(
+    const ImpulseCallbackForTesting& callback) {
+  keepalive_impulse_callback_for_testing_ = callback;
+}
+
+void ProcessManager::SetKeepaliveImpulseDecrementCallbackForTesting(
+    const ImpulseCallbackForTesting& callback) {
+  keepalive_impulse_decrement_callback_for_testing_ = callback;
+}
+
 void ProcessManager::Observe(int type,
                              const content::NotificationSource& source,
                              const content::NotificationDetails& details) {

extensions/browser/process_manager.h

@@ -121,6 +121,14 @@ class ProcessManager : public content::NotificationObserver {
   // related SiteInstances.
   content::BrowserContext* GetBrowserContext() const;
 
+  // Sets callbacks for testing keepalive impulse behavior.
+  typedef base::Callback<void(const std::string& extension_id)>
+      ImpulseCallbackForTesting;
+  void SetKeepaliveImpulseCallbackForTesting(
+      const ImpulseCallbackForTesting& callback);
+  void SetKeepaliveImpulseDecrementCallbackForTesting(
+      const ImpulseCallbackForTesting& callback);
+
  protected:
   // If |context| is incognito pass the master context as |original_context|.
   // Otherwise pass the same context for both.
@@ -216,6 +224,9 @@ class ProcessManager : public content::NotificationObserver {
 
   base::Callback<void(content::DevToolsAgentHost*, bool)> devtools_callback_;
 
+  ImpulseCallbackForTesting keepalive_impulse_callback_for_testing_;
+  ImpulseCallbackForTesting keepalive_impulse_decrement_callback_for_testing_;
+
   base::WeakPtrFactory<ProcessManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ProcessManager);

ppapi/proxy/plugin_main_nacl.cc

@@ -13,6 +13,7 @@
 
 #include "base/command_line.h"
 #include "base/message_loop/message_loop.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/thread.h"
 #include "components/tracing/child_trace_message_filter.h"
@@ -29,6 +30,7 @@
 #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(IPC_MESSAGE_LOG_ENABLED)
@@ -94,6 +96,8 @@ class PpapiDispatcher : public ProxyChannel,
                               SerializedHandle handle);
   void OnPluginDispatcherMessageReceived(const IPC::Message& msg);
 
+  void SetPpapiKeepAliveThrottleFromCommandLine();
+
   std::set<PP_Instance> instances_;
   std::map<uint32, PluginDispatcher*> plugin_dispatchers_;
   uint32 next_plugin_dispatcher_id_;
@@ -206,6 +210,7 @@ void PpapiDispatcher::OnMsgCreateNaClChannel(
     logging::LoggingSettings settings;
     settings.logging_dest = logging::LOG_TO_SYSTEM_DEBUG_LOG;
     logging::InitLogging(settings);
+    SetPpapiKeepAliveThrottleFromCommandLine();
     command_line_and_logging_initialized = true;
   }
   // Tell the process-global GetInterface which interfaces it can return to the
@@ -248,6 +253,18 @@ 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 PpapiPluginRegisterThreadCreator(

ppapi/shared_impl/ppapi_switches.cc

@@ -9,4 +9,7 @@ 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

ppapi/shared_impl/ppapi_switches.h

@@ -10,6 +10,7 @@
 namespace switches {
 
 PPAPI_SHARED_EXPORT extern const char kEnablePepperTesting[];
+PPAPI_SHARED_EXPORT extern const char kPpapiKeepAliveThrottle[];
 
 }  // namespace switches
 

ppapi/tests/extensions/background_keepalive/background.cc

+// Copyright (c) 2013 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 <cstdio>
+#include <string>
+
+#include "ppapi/cpp/instance.h"
+#include "ppapi/cpp/message_loop.h"
+#include "ppapi/cpp/module.h"
+#include "ppapi/cpp/var.h"
+#include "ppapi/utility/completion_callback_factory.h"
+
+class Instance : public pp::Instance {
+ public:
+  explicit Instance(PP_Instance instance) :
+      pp::Instance(instance),
+      callback_factory_(this),
+      delay_milliseconds_(10),
+      active_(true) {
+    DoSomething(PP_OK);
+  }
+  virtual ~Instance() {}
+
+  virtual void HandleMessage(const pp::Var& message_var) {
+    std::string message_string = message_var.AsString();
+    if (message_string == "be idle") {
+      active_ = false;
+    } else {
+      PostMessage("Unhandled control message.");
+    }
+  }
+
+  void DoSomething(int32_t result) {
+    if (active_) {
+      pp::MessageLoop loop = pp::MessageLoop::GetCurrent();
+      pp::CompletionCallback c = callback_factory_.NewCallback(
+          &Instance::DoSomething);
+      loop.PostWork(c, delay_milliseconds_);
+    }
+  }
+
+  pp::CompletionCallbackFactory<Instance> callback_factory_;
+  int delay_milliseconds_;
+  bool active_;
+};
+
+class Module : public pp::Module {
+ public:
+  Module() : pp::Module() {}
+  virtual ~Module() {}
+
+  virtual pp::Instance* CreateInstance(PP_Instance instance) {
+    return new Instance(instance);
+  }
+};
+
+namespace pp {
+Module* CreateModule() {
+  return new ::Module();
+}
+}  // namespace pp
+

ppapi/tests/extensions/background_keepalive/background.js

+// Copyright (c) 2013 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.
+
+var NaClModulesExpected = 0;
+var NaClModulesLoaded = 0;
+
+// Indicate load success.
+function moduleDidLoad() {
+  NaClModulesLoaded++;
+  if (NaClModulesLoaded == NaClModulesExpected)
+    chrome.test.sendMessage("nacl_modules_loaded", handleChromeTestMessage);
+}
+
+var handleChromeTestMessage = function (message) {
+  NaClModules = document.querySelectorAll('embed');
+  for (var i = 0; i < NaClModules.length; i++) {
+    NaClModules[i].postMessage(message);
+  }
+}
+
+function handleNaclMessage(message_event) {
+  console.log("handleNaclMessage: " + message_event.data);
+}
+
+function createNaClEmbed() {
+  NaClModulesExpected++;
+
+  var listener = document.createElement("div");
+  listener.addEventListener("load", moduleDidLoad, true);
+  listener.addEventListener("message", handleNaclMessage, true);
+  listener.innerHTML = '<embed' +
+    ' src="ppapi_tests_extensions_background_keepalive.nmf"' +
+    ' type="application/x-nacl" />';
+  document.body.appendChild(listener);
+}
+
+// Create 2 embeds to verify that we can handle more than one.
+createNaClEmbed();
+createNaClEmbed();
+

ppapi/tests/extensions/background_keepalive/manifest.json

+{
+  "manifest_version": 2,
+  "name": "background_nacl",
+  "version": "0",
+  "description": "Tests keeping background page with NaCl alive when active.",
+  "app": {
+    "background": {
+      "scripts": ["background.js"]
+    }
+  }
+}

ppapi/tests/extensions/extensions.gyp

@@ -29,5 +29,24 @@
         ],
       },
     },
+    {
+      'target_name': 'ppapi_tests_extensions_background_keepalive',
+      'type': 'none',
+      'variables': {
+        'nexe_target': 'ppapi_tests_extensions_background_keepalive',
+        # Only newlib build is used in tests, no need to build others.
+        'build_newlib': 1,
+        'build_glibc': 0,
+        'build_pnacl_newlib': 0,
+        'nexe_destination_dir': 'test_data/ppapi/tests/extensions/background_keepalive',
+        'sources': [
+          'background_keepalive/background.cc',
+        ],
+        'test_files': [
+          'background_keepalive/background.js',
+          'background_keepalive/manifest.json',
+        ],
+      },
+    },
   ],
 }