Refactor extension event distribution to use Values instead of JSON strings.

chrome/browser/extensions/extension_event_router.cc

Index: chrome/browser/extensions/extension_event_router.cc
diff --git a/chrome/browser/extensions/extension_event_router.cc b/chrome/browser/extensions/extension_event_router.cc
index ae763ec9011fa7cff6d80c14574f0af73b8ecb6d..2bbf9ee8538933b5cb934c7ffed69a1351a20934 100644
--- a/chrome/browser/extensions/extension_event_router.cc
+++ b/chrome/browser/extensions/extension_event_router.cc
@@ -36,7 +36,7 @@ using extensions::ExtensionAPI;
 
 namespace {
 
-const char kDispatchEvent[] = "Event.dispatchJSON";
+const char kDispatchEvent[] = "Event.dispatch";
 
 void NotifyEventListenerRemovedOnIOThread(
     void* profile,
@@ -70,34 +70,23 @@ void ExtensionEventRouter::DispatchEvent(
     IPC::Sender* ipc_sender,
     const std::string& extension_id,
     const std::string& event_name,
-    const Value& event_args,
+    ListValue* event_args,
     const GURL& event_url,
     UserGestureState user_gesture,
     const extensions::EventFilteringInfo& info) {
-  // TODO(gdk): Reduce number of DeepCopy() calls throughout the event dispatch
-  // chain, starting by replacing the event_args with a Value*.
   ListValue args;
   args.Set(0, Value::CreateStringValue(event_name));
-  args.Set(1, event_args.DeepCopy());
+  args.Set(1, event_args);
   args.Set(2, info.AsValue().release());
-
   ipc_sender->Send(new ExtensionMsg_MessageInvoke(MSG_ROUTING_CONTROL,
       extension_id, kDispatchEvent, args, event_url,
       user_gesture == USER_GESTURE_ENABLED));
-}
 
-// static
-void ExtensionEventRouter::DispatchEvent(
-    IPC::Sender* ipc_sender,
-    const std::string& extension_id,
-    const std::string& event_name,
-    const std::string& event_args,
-    const GURL& event_url,
-    UserGestureState user_gesture,
-    const extensions::EventFilteringInfo& info) {
-  scoped_ptr<Value> event_args_value(Value::CreateStringValue(event_args));
-  DispatchEvent(ipc_sender, extension_id, event_name, *event_args_value.get(),
-                event_url, user_gesture, info);
+  // The event_args for this dispatch are possibly going to be re-used by
+  // dispatches to other listeners (to avoid copying them), therefore we need to
+  // ensure that args' destruction doesn't result in event_args' deletion.

[asargent_no_longer_on_chrome, 2012/07/09 18:51:50]

In the header documentation you specify that this method takes ownership of
event_args, but you don't delete it here. Either the documentation is wrong and
the caller still owns it, or you need to delete it, right?

+  Value* removed_event_args = NULL;
+  args.Remove(1, &removed_event_args);
 }
 
 ExtensionEventRouter::ExtensionEventRouter(Profile* profile)
@@ -288,21 +277,20 @@ bool ExtensionEventRouter::HasEventListenerImpl(
 
 void ExtensionEventRouter::DispatchEventToRenderers(
     const std::string& event_name,
-    const std::string& event_args,
+    ListValue* event_args,
     Profile* restrict_to_profile,
     const GURL& event_url,
     extensions::EventFilteringInfo info) {
-  DCHECK(!event_args.empty());
-  StringValue event_args_value(event_args);
   linked_ptr<ExtensionEvent> event(
-      new ExtensionEvent(event_name, event_args_value, event_url,
-                         restrict_to_profile, USER_GESTURE_UNKNOWN, info));
+      new ExtensionEvent(event_name, event_args, event_url, restrict_to_profile,
+                         NULL, USER_GESTURE_UNKNOWN,
+                         info));
   DispatchEventImpl("", event);
 }
 
 void ExtensionEventRouter::DispatchEventToRenderers(
     const std::string& event_name,
-    const std::string& event_args,
+    ListValue* event_args,
     Profile* restrict_to_profile,
     const GURL& event_url) {
   DispatchEventToRenderers(event_name, event_args, restrict_to_profile,
@@ -312,49 +300,35 @@ void ExtensionEventRouter::DispatchEventToRenderers(
 void ExtensionEventRouter::DispatchEventToExtension(
     const std::string& extension_id,
     const std::string& event_name,
-    const Value& event_args,
+    ListValue* event_args,
     Profile* restrict_to_profile,
     const GURL& event_url) {
   DCHECK(!extension_id.empty());
   linked_ptr<ExtensionEvent> event(
-      new ExtensionEvent(event_name, event_args, event_url,
-                         restrict_to_profile, USER_GESTURE_UNKNOWN,
-                         EventFilteringInfo()));
+      new ExtensionEvent(event_name, event_args, event_url, restrict_to_profile,
+                         NULL, USER_GESTURE_UNKNOWN, EventFilteringInfo()));
   DispatchEventImpl(extension_id, event);
 }
 
 void ExtensionEventRouter::DispatchEventToExtension(
     const std::string& extension_id,
     const std::string& event_name,
-    const std::string& event_args,
-    Profile* restrict_to_profile,
-    const GURL& event_url) {
-  StringValue event_args_value(event_args);
-  DispatchEventToExtension(extension_id, event_name, event_args_value,
-                           restrict_to_profile, event_url);
-}
-
-void ExtensionEventRouter::DispatchEventToExtension(
-    const std::string& extension_id,
-    const std::string& event_name,
-    const std::string& event_args,
+    ListValue* event_args,
     Profile* restrict_to_profile,
     const GURL& event_url,
     UserGestureState user_gesture) {
   DCHECK(!extension_id.empty());
-  StringValue event_args_value(event_args);
   linked_ptr<ExtensionEvent> event(
-      new ExtensionEvent(event_name, event_args_value, event_url,
-                         restrict_to_profile, user_gesture,
-                         EventFilteringInfo()));
+      new ExtensionEvent(event_name, event_args, event_url, restrict_to_profile,
+                         NULL, user_gesture, EventFilteringInfo()));
   DispatchEventImpl(extension_id, event);
 }
 
 void ExtensionEventRouter::DispatchEventsToRenderersAcrossIncognito(
     const std::string& event_name,
-    const std::string& event_args,
+    ListValue* event_args,
     Profile* restrict_to_profile,
-    const std::string& cross_incognito_args,
+    ListValue* cross_incognito_args,
     const GURL& event_url) {
   linked_ptr<ExtensionEvent> event(
       new ExtensionEvent(event_name, event_args, event_url,
@@ -427,14 +401,14 @@ void ExtensionEventRouter::DispatchEventToProcess(
     return;
   }
 
-  const Value* event_args = NULL;
+  ListValue* event_args = NULL;
   if (!CanDispatchEventToProfile(listener_profile, extension,
                                  event, &event_args)) {
     return;
   }
 
   DispatchEvent(process, extension_id,
-                event->event_name, *event_args,
+                event->event_name, event_args,
                 event->event_url, event->user_gesture,
                 event->info);
   IncrementInFlightEvents(listener_profile, extension);
@@ -444,8 +418,9 @@ bool ExtensionEventRouter::CanDispatchEventToProfile(
     Profile* profile,
     const Extension* extension,
     const linked_ptr<ExtensionEvent>& event,
-    const Value** event_args) {
-  *event_args = event->event_args.get();
+    ListValue** event_args) {
+  if (event_args)
+    *event_args = event->event_args.get();
 
   // Is this event from a different profile than the renderer (ie, an
   // incognito tab event sent to a normal process, or vice versa).
@@ -457,7 +432,9 @@ bool ExtensionEventRouter::CanDispatchEventToProfile(
       return false;
     // Send the event with different arguments to extensions that can't
     // cross incognito.
-    *event_args = event->cross_incognito_args.get();
+    if (event_args) {
+      *event_args = event->cross_incognito_args.get();
+    }
   }
 
   return true;
@@ -467,8 +444,7 @@ void ExtensionEventRouter::MaybeLoadLazyBackgroundPageToDispatchEvent(
     Profile* profile,
     const Extension* extension,
     const linked_ptr<ExtensionEvent>& event) {
-  const Value* event_args = NULL;
-  if (!CanDispatchEventToProfile(profile, extension, event, &event_args))
+  if (!CanDispatchEventToProfile(profile, extension, event, NULL))
     return;
 
   extensions::LazyBackgroundTaskQueue* queue =
@@ -573,53 +549,21 @@ void ExtensionEventRouter::Observe(
 
 ExtensionEvent::ExtensionEvent(
     const std::string& event_name,
-    const Value& event_args,
-    const GURL& event_url,
-    Profile* restrict_to_profile,
-    const Value& cross_incognito_args,
-    ExtensionEventRouter::UserGestureState user_gesture,
-    const extensions::EventFilteringInfo& info)
-    : event_name(event_name),
-      event_args(event_args.DeepCopy()),
-      event_url(event_url),
-      restrict_to_profile(restrict_to_profile),
-      cross_incognito_args(cross_incognito_args.DeepCopy()),
-      user_gesture(user_gesture),
-      info(info) {
-}
-
-ExtensionEvent::ExtensionEvent(
-    const std::string& event_name,
-    const std::string& event_args,
+    ListValue* event_args,
     const GURL& event_url,
     Profile* restrict_to_profile,
-    const std::string& cross_incognito_args,
+    ListValue* cross_incognito_args,
     ExtensionEventRouter::UserGestureState user_gesture,
     const extensions::EventFilteringInfo& info)
     : event_name(event_name),
-      event_args(Value::CreateStringValue(event_args)),
+      event_args(event_args),
       event_url(event_url),
       restrict_to_profile(restrict_to_profile),
-      cross_incognito_args(Value::CreateStringValue(cross_incognito_args)),
+      cross_incognito_args(cross_incognito_args),
       user_gesture(user_gesture),
       info(info) {
+  if (!event_args)
+    this->event_args.reset(new ListValue());
 }
 
-ExtensionEvent::ExtensionEvent(
-    const std::string& event_name,
-    const Value& event_args,
-    const GURL& event_url,
-    Profile* restrict_to_profile,
-    ExtensionEventRouter::UserGestureState user_gesture,
-    const extensions::EventFilteringInfo& info)
-    : event_name(event_name),
-      event_args(event_args.DeepCopy()),
-      event_url(event_url),
-      restrict_to_profile(restrict_to_profile),
-      cross_incognito_args(NULL),
-      user_gesture(user_gesture),
-      info(info) {
-}
-
-ExtensionEvent::~ExtensionEvent() {
-}
+ExtensionEvent::~ExtensionEvent() {}