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

chrome/browser/extensions/extension_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 "chrome/browser/extensions/extension_event_router.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/message_loop.h"
 #include "base/values.h"
@@ skipping 18 matching lines @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 
 using base::Value;
 using content::BrowserThread;
 using extensions::Extension;
 using extensions::ExtensionAPI;
 
 namespace {
 
-const char kDispatchEvent[] = "Event.dispatchJSON";
+const char kDispatchEvent[] = "Event.dispatch";
 
 void NotifyEventListenerRemovedOnIOThread(
     void* profile,
     const std::string& extension_id,
     const std::string& sub_event_name) {
   ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
       profile, extension_id, sub_event_name);
 }
 
 }  // namespace
@@ skipping 13 matching lines @@
       return true;
     return false;
   }
 };
 
 // static
 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)
     : profile_(profile),
       extension_devtools_manager_(
           ExtensionSystem::Get(profile)->devtools_manager()),
       listeners_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
   registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
                  content::NotificationService::AllSources());
   registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
@@ skipping 170 matching lines @@
   for (std::set<ListenerProcess>::const_iterator listener = listeners.begin();
        listener != listeners.end(); ++listener) {
     if (listener->extension_id == extension_id)
       return true;
   }
   return false;
 }
 
 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,
                            event_url, extensions::EventFilteringInfo());
 }
 
 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,
                          restrict_to_profile, cross_incognito_args,
                          USER_GESTURE_UNKNOWN, EventFilteringInfo()));
   DispatchEventImpl("", event);
 }
 
 void ExtensionEventRouter::DispatchEventImpl(
     const std::string& restrict_to_extension_id,
@@ skipping 52 matching lines @@
       process->GetBrowserContext());
   extensions::ProcessMap* process_map =
       listener_profile->GetExtensionService()->process_map();
   // If the event is privileged, only send to extension processes. Otherwise,
   // it's OK to send to normal renderers (e.g., for content scripts).
   if (ExtensionAPI::GetSharedInstance()->IsPrivileged(event->event_name) &&
       !process_map->Contains(extension->id(), process->GetID())) {
     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);
 }
 
 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).
   bool cross_incognito = event->restrict_to_profile &&
       profile != event->restrict_to_profile;
   if (cross_incognito &&
       !profile->GetExtensionService()->CanCrossIncognito(extension)) {
     if (!event->cross_incognito_args.get())
       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;
 }
 
 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 =
       ExtensionSystem::Get(profile)->lazy_background_task_queue();
   if (queue->ShouldEnqueueTask(profile, extension)) {
     queue->AddPendingTask(
         profile, extension->id(),
         base::Bind(&ExtensionEventRouter::DispatchPendingEvent,
                    base::Unretained(this), event));
   }
@@ skipping 84 matching lines @@
       break;
     }
     default:
       NOTREACHED();
       return;
   }
 }
 
 ExtensionEvent::ExtensionEvent(
     const std::string& event_name,
-    const Value& event_args,
+    ListValue* event_args,
     const GURL& event_url,
     Profile* restrict_to_profile,
-    const Value& cross_incognito_args,
+    ListValue* cross_incognito_args,
     ExtensionEventRouter::UserGestureState user_gesture,
     const extensions::EventFilteringInfo& info)
     : event_name(event_name),
-      event_args(event_args.DeepCopy()),
+      event_args(event_args),
       event_url(event_url),
       restrict_to_profile(restrict_to_profile),
-      cross_incognito_args(cross_incognito_args.DeepCopy()),
+      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 std::string& event_args,
-    const GURL& event_url,
-    Profile* restrict_to_profile,
-    const std::string& cross_incognito_args,
-    ExtensionEventRouter::UserGestureState user_gesture,
-    const extensions::EventFilteringInfo& info)
-    : event_name(event_name),
-      event_args(Value::CreateStringValue(event_args)),
-      event_url(event_url),
-      restrict_to_profile(restrict_to_profile),
-      cross_incognito_args(Value::CreateStringValue(cross_incognito_args)),
-      user_gesture(user_gesture),
-      info(info) {
-}
-
-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() {}