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

ppapi/proxy/plugin_main_nacl.cc

Index: ppapi/proxy/plugin_main_nacl.cc
diff --git a/ppapi/proxy/plugin_main_nacl.cc b/ppapi/proxy/plugin_main_nacl.cc
index f050778227cad4eef35c4f1c26f59fe1a6ccbcbe..368f709bf6800c49f878e2fcbbcc48e8a7119601 100644
--- a/ppapi/proxy/plugin_main_nacl.cc
+++ b/ppapi/proxy/plugin_main_nacl.cc
@@ -12,6 +12,7 @@
 // ViewMsgLog et al. functions.
 
 #include "base/command_line.h"
+#include "base/debug/trace_event.h"
 #include "base/message_loop/message_loop.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/thread.h"
@@ -266,6 +267,23 @@ void PpapiPluginRegisterThreadCreator(
   ppapi::PPB_Audio_Shared::SetThreadFunctions(thread_functions);
 }
 
+const int checkIntervalInSeconds = 1;

[dmichael (off chromium), 2013/11/06 22:39:21]

nit: kCheckIntervalInSeconds

Some rationale about why 1 second is appropriate might be good (it's not obvious
to me)

[scheib, 2013/11/15 01:22:17]

Done.

+bool last_was_active = false;

[dmichael (off chromium), 2013/11/06 22:39:21]

Could this be a parameter to CheckActivity instead of a global?

Also...  kCheckIntervalInSeconds and g_last_was_active (if left global) should
probably move to the unnamed namespace.

[scheib, 2013/11/15 01:22:17]

Done.

+
+void CheckActivity(base::MessageLoop* loop, PpapiDispatcher* dispatcher) {

[dmichael (off chromium), 2013/11/06 22:39:21]

You're not using dispatcher, and it will become invalid at process shutdown. I
think you want to just remove the argument.

[scheib, 2013/11/15 01:22:17]

Done.

+  bool was_active = PluginGlobals::Get()->plugin_has_been_active();

[dmichael (off chromium), 2013/11/06 22:39:21]

At shutdown, PluginGlobals()::Get() will be NULL. Maybe you should just break
early in that case.

[scheib, 2013/11/15 01:22:17]

Done.

+  PluginGlobals::Get()->set_plugin_has_been_active(false);
+ 
+  if (last_was_active != was_active) {
+    PluginGlobals::Get()->GetBrowserSender()->Send(
+        new PpapiHostMsg_IdleState(!was_active));

[dmichael (off chromium), 2013/11/06 22:39:21]

So we're only sending a message if the state changes? If the plugin says it's
active (not idle), and then doesn't send a message for a long time, does the
browser then assume that it's active? Is that what we want?

[scheib, 2013/11/07 01:06:42]

I think it is what we want. Browser side we are tracking activity by
incrementing / decrementing a counter. There's no value to sending more IPC
traffic I think. If the plugin goes dark while we think it's active that's an
orthogonal problem I believe is already solved by a watchdog that will notice a
frozen plugin process.

[dmichael (off chromium), 2013/11/07 17:11:55]

Sounds good. I wrote this comment before I'd read the rest of the CL.

+  }
+  last_was_active = was_active;
+
+  loop->PostDelayedTask(FROM_HERE, base::Bind(&CheckActivity, loop, dispatcher),

[dmichael (off chromium), 2013/11/06 22:39:21]

Why not use MessageLoop::current()? I'm not used to seeing a bare MessageLoop
pointer passed around; if a parameter was necessary,
scoped_refptr<MessageLoopProxy> would seem better. But it looks like
MessageLoop::current() will do the trick here...

[scheib, 2013/11/15 01:22:17]

Done.

+                        base::TimeDelta::FromSeconds(checkIntervalInSeconds));
+}
+
 int PpapiPluginMain() {
   // Though it isn't referenced here, we must instantiate an AtExitManager.
   base::AtExitManager exit_manager;
@@ -289,6 +307,11 @@ int PpapiPluginMain() {
   PpapiDispatcher ppapi_dispatcher(io_thread.message_loop_proxy());
   plugin_globals.set_plugin_proxy_delegate(&ppapi_dispatcher);
 
+  io_thread.message_loop()->PostDelayedTask(FROM_HERE,
+                                            base::Bind(&CheckActivity,

[dmichael (off chromium), 2013/11/06 22:39:21]

CheckActivity needs to have the global proxy lock, since it uses shared stuff
like PluginGlobals. You can use "RunWhileLocked" from
ppapi/shared_impl/proxy_lock.h here to make that happen cleanly.
(https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/shared_impl/...)

Example usage:
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/proxy/ppb_va...

[scheib, 2013/11/15 01:22:17]

Done via: ppapi::ProxyAutoLock lock;

+                                                       io_thread.message_loop(),
+                                                       &ppapi_dispatcher),
+                       base::TimeDelta::FromSeconds(checkIntervalInSeconds));

[dmichael (off chromium), 2013/11/06 22:39:21]

(nit: all the arguments should just be indented by 4 so this one can fit)

[scheib, 2013/11/15 01:22:17]

Done.

   loop.Run();
 
   return 0;