Keep NaCl plugins used in app background pages alive when active.

chrome/browser/nacl_host/nacl_browser_delegate_impl.cc

Index: chrome/browser/nacl_host/nacl_browser_delegate_impl.cc
diff --git a/chrome/browser/nacl_host/nacl_browser_delegate_impl.cc b/chrome/browser/nacl_host/nacl_browser_delegate_impl.cc
index 7f16017783667771f5795fef4969034f20f939ab..37637cc605b249d8d52fc4a8636a5b7b77eb0585 100644
--- a/chrome/browser/nacl_host/nacl_browser_delegate_impl.cc
+++ b/chrome/browser/nacl_host/nacl_browser_delegate_impl.cc
@@ -9,8 +9,10 @@
 #include "base/strings/string_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/component_updater/pnacl/pnacl_component_installer.h"
+#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/nacl_host/nacl_infobar_delegate.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_host/pepper/chrome_browser_pepper_host_factory.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_paths_internal.h"
@@ -19,13 +21,104 @@
 #include "chrome/common/extensions/manifest_handlers/shared_module_info.h"
 #include "chrome/common/logging_chrome.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/site_instance.h"
 #include "extensions/browser/info_map.h"
+#include "extensions/browser/process_manager.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/url_pattern.h"
 #include "ppapi/c/private/ppb_nacl_private.h"
 
 using extensions::SharedModuleInfo;
 
+namespace {
+
+// Handles an extension's NaCl process transitioning in or out of idle state by
+// relaying the state to the extension's process manager.
+//
+// A NaCl instance monitors its activity and sends an IPC to the browser
+// process BrowserPpapiHost upon transitioning in our out of idleness. The

[yzshen1, 2013/11/21 18:01:50]

our -> or

[scheib, 2013/12/11 21:35:40]

Done.

+// content::BrowserPpapiHost passes context information up to the chrome level
+// NaClProcessHost where we use the instance's context to find the associated
+// extension process manager.
+//
+// There is 1:many relationship for extension:nacl-embeds, but only a
+// 1:1 relationship for NaClProcessHost:PP_Instance. The content layer doesn't
+// rely on this knowledge because it routes messages for ppapi non-nacl
+// instances as well, though they won't have callbacks set. Here the 1:1
+// assumption is made and DCHECKed.
+void OnIdleStateChangeOnUIThread(
+    content::BrowserPpapiHost::OnIdleChangeInstanceData intance_data,

[yzshen1, 2013/11/21 18:01:50]

This whole vector is copied quite a few times:

when passed into OnIdleStateChange();
when bind() in OnIdleStateChange();
when OnIdleStateChangeOnUIThread is called.

Can we avoid this?

[scheib, 2013/12/11 21:35:40]

Done. Parameters changed to all const references. Also, I learned today that
base::Bind makes copies of const references for you. I'll update it's doc to be
clearer: https://codereview.chromium.org/81713003/

[yzshen1, 2013/12/13 21:23:15]

Right.
Thanks for the nice document!

+    const base::FilePath profile_data_directory,

[yzshen1, 2013/11/21 18:01:50]

const &, please.

[scheib, 2013/12/11 21:35:40]

Done.

[scheib, 2013/12/11 21:35:40]

Done.

+    bool idle) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  // Only one instance will exist for NaCl embeds, even when more than one
+  // embed of the same plugin exists on the same page.
+  DCHECK(intance_data.size() == 1);
+  if (intance_data.size() < 1)
+    return;
+
+  content::RenderViewHost* render_view_host = content::RenderViewHost::FromID(
+      intance_data[0].render_process_id, intance_data[0].render_view_id);
+  DCHECK(render_view_host);
+  if (!render_view_host)
+    return;
+
+  content::SiteInstance* site_instance = render_view_host->GetSiteInstance();
+  DCHECK(site_instance);
+  if (!site_instance)
+    return;
+
+  extensions::ExtensionSystem* extension_system =
+      extensions::ExtensionSystem::GetForBrowserContext(
+          site_instance->GetBrowserContext());
+  DCHECK(extension_system);
+  if (!extension_system)
+    return;
+
+  const ExtensionService* extension_service =
+      extension_system->extension_service();
+  DCHECK(extension_service);
+  if (!extension_service)
+    return;
+
+  const extensions::Extension* extension = extension_service->GetExtensionById(
+      intance_data[0].document_url.host(), false);
+  if (!extension)
+    return;
+
+  extensions::ProcessManager* pm = extension_system->process_manager();
+  DCHECK(pm);
+  if (!pm)
+    return;
+
+  if (idle) {
+    fprintf(stderr, "%s:%s:%d DecrementLazyKeepaliveCount\n",

[yzshen1, 2013/11/21 18:01:50]

Is this intended to be in the production code?

[scheib, 2013/12/11 21:35:40]

No, this patch was still Work In Progress soliciting thoughts on testing.

+            __FILE__, __FUNCTION__, __LINE__);
+    pm->DecrementLazyKeepaliveCount(extension);

[yzshen1, 2013/11/21 18:01:50]

If the plugin is destroyed while the last call is
pm->IncrementLazyKeepaliveCount(), is it true that the extension will be
considered active forever?

[scheib, 2013/12/11 21:35:40]

No longer an issue as the design has changed. Now if KeepaliveImpulse isn't
continuously called the extension will time out to being considered idle.

+  } else {
+    fprintf(stderr, "%s:%s:%d IncrementLazyKeepaliveCount\n",

[yzshen1, 2013/11/21 18:01:50]

Is this debugging code that should be removed?

[scheib, 2013/12/11 21:35:40]

Done.

+            __FILE__, __FUNCTION__, __LINE__);
+    pm->IncrementLazyKeepaliveCount(extension);
+  }
+}
+
+// Calls OnIdleStateChangeOnUIThread on UI thread.
+void OnIdleStateChange(
+    content::BrowserPpapiHost::OnIdleChangeInstanceData intance_data,
+    const base::FilePath profile_data_directory,
+    bool idle) {
+  DCHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
+                                   base::Bind(&OnIdleStateChangeOnUIThread,
+                                              intance_data,
+                                              profile_data_directory,
+                                              idle));
+}
+
+}  // namespace
+
 NaClBrowserDelegateImpl::NaClBrowserDelegateImpl(
     extensions::InfoMap* extension_info_map)
     : extension_info_map_(extension_info_map), inverse_debug_patterns_(false) {}
@@ -181,3 +274,8 @@ bool NaClBrowserDelegateImpl::MapUrlToLocalFilePath(
   *file_path = resource_file_path;
   return true;
 }
+
+const content::BrowserPpapiHost::OnIdleChangeCallback
+NaClBrowserDelegateImpl::GetOnIdleChangeCallback() {
+  return base::Bind(&OnIdleStateChange);
+}