Add extension event histogram values for messaging, webRequest, and webview.

extensions/browser/event_router.cc

 // Copyright (c) 2012 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 "extensions/browser/event_router.h"
 
 #include <utility>
 
 #include "base/atomic_sequence_num.h"
 #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/values.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "extensions/browser/api_activity_monitor.h"
 #include "extensions/browser/event_router_factory.h"
 #include "extensions/browser/extension_host.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/browser/lazy_background_task_queue.h"
 #include "extensions/browser/notification_types.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/browser/process_map.h"
+#include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_api.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/extension_urls.h"
 #include "extensions/common/features/feature.h"
 #include "extensions/common/features/feature_provider.h"
 #include "extensions/common/manifest_handlers/background_info.h"
 #include "extensions/common/manifest_handlers/incognito_info.h"
 #include "extensions/common/permissions/permissions_data.h"
 
@@ skipping 101 matching lines @@
   return EventRouterFactory::GetForBrowserContext(browser_context);
 }
 
 // static
 std::string EventRouter::GetBaseEventName(const std::string& full_event_name) {
   size_t slash_sep = full_event_name.find('/');
   return full_event_name.substr(0, slash_sep);
 }
 
 // static
-void EventRouter::DispatchEvent(IPC::Sender* ipc_sender,
-                                void* browser_context_id,
-                                const std::string& extension_id,
-                                const std::string& event_name,
-                                scoped_ptr<ListValue> event_args,
-                                UserGestureState user_gesture,
-                                const EventFilteringInfo& info) {
+void EventRouter::DispatchEventToSender(IPC::Sender* ipc_sender,
+                                        void* browser_context_id,
+                                        const GURL& url,
+                                        const std::string& extension_id,
+                                        events::HistogramValue histogram_value,
+                                        const std::string& event_name,
+                                        scoped_ptr<ListValue> event_args,
+                                        UserGestureState user_gesture,
+                                        const EventFilteringInfo& info) {
   int event_id = g_extension_event_id.GetNext();
 
-  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+  if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    DoDispatchEventToSenderBookkeepingOnUI(browser_context_id, extension_id,
+                                           url, event_id, histogram_value,
+                                           event_name);
+  } else {
     // This is called from WebRequest API.
     // TODO(lazyboy): Skip this entirely: http://crbug.com/488747.
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
-        base::Bind(&EventRouter::IncrementInFlightEventsOnUI,
-                   browser_context_id, extension_id, event_id, event_name));
-  } else {
-    IncrementInFlightEventsOnUI(browser_context_id, extension_id, event_id,
-                                event_name);
+        base::Bind(&EventRouter::DoDispatchEventToSenderBookkeepingOnUI,
+                   browser_context_id, extension_id, url, event_id,
+                   histogram_value, event_name));
   }
 
   DispatchExtensionMessage(ipc_sender, browser_context_id, extension_id,
                            event_id, event_name, event_args.get(), user_gesture,
                            info);
 }
 
 EventRouter::EventRouter(BrowserContext* browser_context,
                          ExtensionPrefs* extension_prefs)
     : browser_context_(browser_context),
@@ skipping 337 matching lines @@
        it != listeners.end(); it++) {
     const EventListener* listener = *it;
     if (restrict_to_extension_id.empty() ||
         restrict_to_extension_id == listener->extension_id()) {
       if (listener->process()) {
         EventDispatchIdentifier dispatch_id(listener->GetBrowserContext(),
                                             listener->extension_id());
         if (!ContainsKey(already_dispatched, dispatch_id)) {
           DispatchEventToProcess(listener->extension_id(),
                                  listener->listener_url(), listener->process(),
-                                 event, listener->filter());
+                                 event, listener->filter(), false);

[Devlin, 2015/08/05 21:34:09]

there are a few more of the anonymous bools than I'd like.  Can you document
them with the typical /* didn't enqueue */?

[not at google - send to devlin, 2015/08/07 20:55:03]

Sure.

         }
       }
     }
   }
 }
 
 void EventRouter::DispatchLazyEvent(
     const std::string& extension_id,
     const linked_ptr<Event>& event,
     std::set<EventDispatchIdentifier>* already_dispatched,
@@ skipping 23 matching lines @@
           std::make_pair(incognito_context, extension_id));
     }
   }
 }
 
 void EventRouter::DispatchEventToProcess(
     const std::string& extension_id,
     const GURL& listener_url,
     content::RenderProcessHost* process,
     const linked_ptr<Event>& event,
-    const base::DictionaryValue* listener_filter) {
+    const base::DictionaryValue* listener_filter,
+    bool did_enqueue) {
   BrowserContext* listener_context = process->GetBrowserContext();
   ProcessMap* process_map = ProcessMap::Get(listener_context);
 
   // NOTE: |extension| being NULL does not necessarily imply that this event
   // shouldn't be dispatched. Events can be dispatched to WebUI and webviews as
   // well.  It all depends on what GetMostLikelyContextType returns.
   const Extension* extension =
       ExtensionRegistry::Get(browser_context_)->enabled_extensions().GetByID(
           extension_id);
 
@@ skipping 47 matching lines @@
       !event->will_dispatch_callback.Run(listener_context, extension,
                                          event->event_args.get(),
                                          listener_filter)) {
     return;
   }
 
   int event_id = g_extension_event_id.GetNext();
   DispatchExtensionMessage(process, listener_context, extension_id, event_id,
                            event->event_name, event->event_args.get(),
                            event->user_gesture, event->filter_info);
+  ReportEvent(event->histogram_value, listener_url, did_enqueue);
 
   if (extension) {
     IncrementInFlightEvents(listener_context, extension, event_id,
                             event->event_name);
   }
 }
 
 bool EventRouter::CanDispatchEventToBrowserContext(
     BrowserContext* context,
     const Extension* extension,
@@ skipping 38 matching lines @@
     queue->AddPendingTask(context, extension->id(),
                           base::Bind(&EventRouter::DispatchPendingEvent,
                                      base::Unretained(this), dispatched_event));
     return true;
   }
 
   return false;
 }
 
 // static
-void EventRouter::IncrementInFlightEventsOnUI(void* browser_context_id,
-                                              const std::string& extension_id,
-                                              int event_id,
-                                              const std::string& event_name) {
+void EventRouter::DoDispatchEventToSenderBookkeepingOnUI(
+    void* browser_context_id,
+    const std::string& extension_id,
+    const GURL& url,
+    int event_id,
+    events::HistogramValue histogram_value,
+    const std::string& event_name) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   BrowserContext* browser_context =
       reinterpret_cast<BrowserContext*>(browser_context_id);
   if (!ExtensionsBrowserClient::Get()->IsValidContext(browser_context))
     return;
-  EventRouter* event_router = EventRouter::Get(browser_context);
-  if (!event_router)
-    return;
   const Extension* extension =
       ExtensionRegistry::Get(browser_context)->enabled_extensions().GetByID(
           extension_id);
   if (!extension)
     return;
+  EventRouter* event_router = EventRouter::Get(browser_context);
   event_router->IncrementInFlightEvents(browser_context, extension, event_id,
                                         event_name);
+  event_router->ReportEvent(histogram_value, url, false);
 }
 
 void EventRouter::IncrementInFlightEvents(BrowserContext* context,
                                           const Extension* extension,
                                           int event_id,
                                           const std::string& event_name) {
   // TODO(chirantan): Turn this on once crbug.com/464513 is fixed.
   // DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // Only increment in-flight events if the lazy background page is active,
@@ skipping 15 matching lines @@
   // The event ACK is routed to the background host, so this should never be
   // NULL.
   CHECK(host);
   // TODO(mpcomplete): We should never get this message unless
   // HasLazyBackgroundPage is true. Find out why we're getting it anyway.
   if (host->extension() &&
       BackgroundInfo::HasLazyBackgroundPage(host->extension()))
     pm->DecrementLazyKeepaliveCount(host->extension());
 }
 
+void EventRouter::ReportEvent(events::HistogramValue histogram_value,
+                              const GURL& url,
+                              bool did_enqueue) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // At this point, we're only interested in reporting events for extensions.
+  if (!url.SchemeIs(kExtensionScheme))

[Devlin, 2015/08/05 21:34:09]

Doesn't this mean we won't log any events for messaging?  Or webRequest?

[not at google - send to devlin, 2015/08/07 20:55:03]

The URL was the target URL which assuming messaging works fine will be correct.
I've changed this to be an extension now though, so moot.

+    return;
+
+  ExtensionRegistry* registry = ExtensionRegistry::Get(browser_context_);
+  const Extension* extension =
+      registry->enabled_extensions().GetExtensionOrAppByURL(url);
+  if (!extension) {
+    NOTREACHED() << "Reporting event for non-existent extension at " << url;

[Devlin, 2015/08/05 21:34:09]

Doing a NOTREACHED(); return; is typically a discouraged (anti)pattern, with the
exception of IPCs.  Is there a reason not to just DCHECK (or CHECK, if you like)
here?

[not at google - send to devlin, 2015/08/07 20:55:03]

Deliberate choice - I don't trust call sites, especially webview, but I want the
tests to fail at least. Maybe once this patch has baked this could be upgraded
to a CHECK.

+    return;
+  }
+
+  // TODO(kalman): UMA for dispatched event.
+  // TODO(kalman): UMA specifically for component extensions.

[not at google - send to devlin, 2015/08/05 20:23:43]

I will add the UMA_HISTOGRAM_ENUMERATION calls in a follow up, to reduce the
size of this change, and isolate the conversation about that. As you can see all
the info that I plan on using has been plumbed into here.

+
+  switch (GetExtensionPageType(extension, url, did_enqueue)) {
+    case EXTENSION_PAGE_NONE:
+      break;
+    case EXTENSION_PAGE_VIEW:
+      // TODO(kalman): UMA for dispatched event to view.
+      break;
+    case EXTENSION_PAGE_PERSISTENT_BACKGROUND:
+      // TODO(kalman): UMA for dispatched event to persistent background page.
+      break;
+    case EXTENSION_PAGE_DORMANT_EVENT:
+      // TODO(kalman): UMA for dispatched event that woke up event page.
+      break;
+    case EXTENSION_PAGE_AWAKE_EVENT:
+      // TODO(kalman): UMA for dispatched event to awake up event page.
+      break;
+  }
+}
+
+EventRouter::ExtensionPageType EventRouter::GetExtensionPageType(
+    const Extension* extension,
+    const GURL& url,
+    bool did_enqueue) {
+  if (!extension)

[Devlin, 2015/08/05 21:34:09]

When could this happen?

[not at google - send to devlin, 2015/08/07 20:55:03]

In a previous version of the patch, I guess - and it was a great find, because
in removing it, it led to nice CL-wide change. Instead of passing the URL in
here, I'm just going to pass the Extension, which makes this simpler and less
error-prone since it's explicitly only logging Extension events.

+    return EXTENSION_PAGE_NONE;
+  if (url == BackgroundInfo::GetBackgroundURL(extension)) {
+    if (BackgroundInfo::HasPersistentBackgroundPage(extension))
+      return EXTENSION_PAGE_PERSISTENT_BACKGROUND;
+    if (BackgroundInfo::HasLazyBackgroundPage(extension))
+      return did_enqueue ? EXTENSION_PAGE_DORMANT_EVENT
+                         : EXTENSION_PAGE_AWAKE_EVENT;
+  }
+  return EXTENSION_PAGE_VIEW;
+}
+
 void EventRouter::DispatchPendingEvent(const linked_ptr<Event>& event,
                                        ExtensionHost* host) {
   if (!host)
     return;
 
   if (listeners_.HasProcessListener(host->render_process_host(),
                                     host->extension()->id())) {
     // URL events cannot be lazy therefore can't be pending, hence the GURL().
     DispatchEventToProcess(host->extension()->id(), GURL(),
-                           host->render_process_host(), event, nullptr);
+                           host->render_process_host(), event, nullptr, true);
   }
 }
 
 void EventRouter::Observe(int type,
                           const content::NotificationSource& source,
                           const content::NotificationDetails& details) {
   switch (type) {
     case extensions::NOTIFICATION_EXTENSION_ENABLED: {
       // If the extension has a lazy background page, make sure it gets loaded
       // to register the events the extension is interested in.
@@ skipping 85 matching lines @@
                                      const std::string& extension_id,
                                      const GURL& listener_url,
                                      content::BrowserContext* browser_context)
     : event_name(event_name),
       extension_id(extension_id),
       listener_url(listener_url),
       browser_context(browser_context) {
 }
 
 }  // namespace extensions