Tracking push events for lucid sleep

extensions/browser/event_router.cc

Index: extensions/browser/event_router.cc
diff --git a/extensions/browser/event_router.cc b/extensions/browser/event_router.cc
index cddffdea45f1a981b2453e51cf60e282207069b4..b20b40701adcb4b3ea224fa3bc92ec34b299ebb0 100644
--- a/extensions/browser/event_router.cc
+++ b/extensions/browser/event_router.cc
@@ -8,8 +8,10 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/lazy_instance.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
+#include "base/synchronization/lock.h"
 #include "base/values.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
@@ -48,22 +50,20 @@ void DoNothing(ExtensionHost* host) {}
 // registered from its lazy background page.
 const char kFilteredEvents[] = "filtered_events";
 
-// Sends a notification about an event to the API activity monitor on the
-// UI thread. Can be called from any thread.
-void NotifyApiEventDispatched(void* browser_context_id,
-                              const std::string& extension_id,
-                              const std::string& event_name,
-                              scoped_ptr<ListValue> args) {
+// Sends a notification about an event to the API activity monitor and the
+// ExtensionHost for |extension_id| on the UI thread. Can be called from any
+// thread.
+void NotifyEventDispatched(void* browser_context_id,
+                           const std::string& extension_id,
+                           const std::string& event_name,
+                           int message_id,
+                           scoped_ptr<ListValue> args) {
   // The ApiActivityMonitor can only be accessed from the UI thread.
   if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
     BrowserThread::PostTask(
-        BrowserThread::UI,
-        FROM_HERE,
-        base::Bind(&NotifyApiEventDispatched,
-                   browser_context_id,
-                   extension_id,
-                   event_name,
-                   base::Passed(&args)));
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&NotifyEventDispatched, browser_context_id, extension_id,
+                   event_name, message_id, base::Passed(&args)));
     return;
   }
 
@@ -75,8 +75,21 @@ void NotifyApiEventDispatched(void* browser_context_id,
       ExtensionsBrowserClient::Get()->GetApiActivityMonitor(context);
   if (monitor)
     monitor->OnApiEventDispatched(extension_id, event_name, args.Pass());
+
+  ExtensionHost* host =
+      ProcessManager::Get(context)->GetBackgroundHostForExtension(extension_id);

[not at google - send to devlin, 2015/02/27 01:00:50]

This looks like it could be a problem. A message was definitely sent earlier,
and it went somewhere - so if there's not background page right now you won't
register the message ID.

At face value this would appear to be a bug, but dispatcher.cc does ack to the
background page's view, and only if there's a persistent background page - so it
should exist. However, if it doesn't, this line condemns it to die in the near
future.

The way to fix this is to move the message ID handling inside eventrouter, I
think.

[not at google - send to devlin, 2015/02/27 01:09:42]

s/persistent/event/

it is getting late in the day.

[Chirantan Ekbote, 2015/03/02 22:27:31]

If this is the case then isn't it a bigger concern that an IPC message was sent
to an extension that doesn't have a corresponding ExtensionHost?  Under what
circumstances would this be a valid state?  Maybe this should be a DCHECK or a
CHECK rather than an if statement.
I'm a little wary about doing this since we would need to post a task to the UI
thread to access the EventRouter for the profile where the event is being
dispatched, which adds more asynchronous steps to this process.  Even after
that, we would be invoking the observers on every event dispatch.  The nice
things about the way things work now is that observers figure out whether they
care about a particular ExtensionHost when it is created and are then only
notified when an event for that ExtensionHost is dispatched.

[not at google - send to devlin, 2015/03/02 22:38:27]

Correction: corresponding *background* hosts. There are many types of
ExtensionHosts, including background pages. In theory there should always be a
background host active if any other ExtensionHost is active, but I'm not aware
of that being an official invariant.

The odd part about all of this is that a IPC is sent somewhere, then the
notification is sent to the background page.
I've paged out this code over the weekend, but I find it weird that an IPC is
sent from one place, then the Ack for that IPC handled in the other. What is the
problem with things being asynchronous?

+  if (host)
+    host->OnMessageDispatched(event_name, message_id);
 }
 
+// A global identifier used to distinguish extension messages that is
+// incremented every time a message is dispatched.
+int g_extension_message_id = 0;
+
+// Protects access to |g_extension_message_id|.
+base::LazyInstance<base::Lock>::Leaky g_message_id_lock =
+    LAZY_INSTANCE_INITIALIZER;
+
 }  // namespace
 
 const char EventRouter::kRegisteredEvents[] = "events";
@@ -106,15 +119,20 @@ void EventRouter::DispatchExtensionMessage(IPC::Sender* ipc_sender,
                                            ListValue* event_args,
                                            UserGestureState user_gesture,
                                            const EventFilteringInfo& info) {
-  NotifyApiEventDispatched(browser_context_id,
-                           extension_id,
-                           event_name,
-                           make_scoped_ptr(event_args->DeepCopy()));
+  // Since this function can be called from any thread we need to protect access
+  // to |g_extension_message_id|.
+  g_message_id_lock.Get().Acquire();
+  int message_id = g_extension_message_id++;
+  g_message_id_lock.Get().Release();
+
+  NotifyEventDispatched(browser_context_id, extension_id, event_name,
+                        message_id, make_scoped_ptr(event_args->DeepCopy()));
 
   ListValue args;
   args.Set(0, new base::StringValue(event_name));
   args.Set(1, event_args);
   args.Set(2, info.AsValue().release());
+  args.Set(3, new base::FundamentalValue(message_id));
   ipc_sender->Send(new ExtensionMsg_MessageInvoke(
       MSG_ROUTING_CONTROL,
       extension_id,