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

chrome/browser/extensions/extension_event_router.h

 // 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.
 
 #ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_EVENT_ROUTER_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTENSION_EVENT_ROUTER_H_
 #pragma once
 
 #include <map>
 #include <set>
@@ skipping 32 matching lines @@
                              public extensions::EventListenerMap::Delegate {
  public:
   // These constants convey the state of our knowledge of whether we're in
   // a user-caused gesture as part of DispatchEvent.
   enum UserGestureState {
     USER_GESTURE_UNKNOWN = 0,
     USER_GESTURE_ENABLED = 1,
     USER_GESTURE_NOT_ENABLED = 2,
   };
 
-  // Sends an event via ipc_sender to the given extension. Can be called on
-  // any thread.
+  // Sends an event via ipc_sender to the given extension. Can be called on any
+  // thread. Takes ownership of |event_args|, but can be safely invoked with
+  // NULL.
   static void DispatchEvent(IPC::Sender* ipc_sender,

[miket_OOO, 2012/07/10 22:33:19]

Great, makes more sense now. I'd like to see similar comments in the earlier .h,
even if those are relatively private dispatch functions.

                             const std::string& extension_id,
                             const std::string& event_name,
-                            const base::Value& event_args,
+                            base::ListValue* event_args,

[sky, 2012/07/16 22:29:06]

Did you consider making this take a const base::ListValue& and doing a deep
copy? While not as efficient it would be less error prone.

                             const GURL& event_url,
                             UserGestureState user_gesture,
                             const extensions::EventFilteringInfo& info);
-
-  // This invocation is deprecated. All future consumers of this API should be
-  // sending Values as event arguments, using the above version.
-  static void 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);
 
   explicit ExtensionEventRouter(Profile* profile);
   virtual ~ExtensionEventRouter();
 
   // Add or remove the process/extension pair as a listener for |event_name|.
   // Note that multiple extensions can share a process due to process
   // collapsing. Also, a single extension can have 2 processes if it is a split
   // mode extension.
   void AddEventListener(const std::string& event_name,
                         content::RenderProcessHost* process,
@@ skipping 31 matching lines @@
 
   // Returns true if the extension is listening to the given event.
   bool ExtensionHasEventListener(const std::string& extension_id,
                                  const std::string& event_name);
 
   // Send an event to every registered extension renderer. If
   // |restrict_to_profile| is non-NULL, then the event will not be sent to other
   // profiles unless the extension has permission (e.g. incognito tab update ->
   // normal profile only works if extension is allowed incognito access). If
   // |event_url| is not empty, the event is only sent to extension with host
-  // permissions for this url.
+  // permissions for this url. Takes ownership of |event_args|, but can be
+  // safely invoked with NULL.
   void DispatchEventToRenderers(
       const std::string& event_name,
-      const std::string& event_args,
+      base::ListValue* event_args,
       Profile* restrict_to_profile,
       const GURL& event_url,
       extensions::EventFilteringInfo info);
 
-  // As above, but defaults |info| to EventFilteringInfo().
+  // As above, but defaults |info| to EventFilteringInfo(). Takes ownership of
+  // |event_args|, but can be safely invoked with NULL.
   void DispatchEventToRenderers(
       const std::string& event_name,
-      const std::string& event_args,
+      base::ListValue* event_args,
       Profile* restrict_to_profile,
       const GURL& event_url);
 
-  // Same as above, except only send the event to the given extension.
+  // Same as above, except only send the event to the given extension. Takes
+  // ownership of |event_args|, but can be safely invoked with NULL.
   virtual void DispatchEventToExtension(
       const std::string& extension_id,
       const std::string& event_name,
-      const base::Value& event_args,
+      base::ListValue* event_args,
       Profile* restrict_to_profile,
       const GURL& event_url);
 
-  // This invocation is deprecated. The above variant which uses a Value for
-  // event_args is to be used instead.
+  // Dispatch an event to particular extension. Also include an explicit user
+  // gesture indicator. Takes ownership of |event_args|, but can be safely
+  // invoked with NULL.
   virtual void DispatchEventToExtension(
       const std::string& extension_id,
       const std::string& event_name,
-      const std::string& event_args,
-      Profile* restrict_to_profile,
-      const GURL& event_url);
-
-  // Dispatch an event to particular extension. Also include an
-  // explicit user gesture indicator.
-  virtual void DispatchEventToExtension(
-      const std::string& extension_id,
-      const std::string& event_name,
-      const std::string& event_args,
+      base::ListValue* event_args,
       Profile* restrict_to_profile,
       const GURL& event_url,
       UserGestureState user_gesture);
 
   // Send different versions of an event to extensions in different profiles.
   // This is used in the case of sending one event to extensions that have
   // incognito access, and another event to extensions that don't (here),
   // in order to avoid sending 2 events to "spanning" extensions.
   // If |cross_incognito_profile| is non-NULL and different from
   // restrict_to_profile, send the event with cross_incognito_args to the
-  // extensions in that profile that can't cross incognito.
+  // extensions in that profile that can't cross incognito. Takes ownership of
+  // |event_args| and |cross_incognito_args|, but can be safely invoked with
+  // NULL for either.
   void DispatchEventsToRenderersAcrossIncognito(
       const std::string& event_name,
-      const std::string& event_args,
+      base::ListValue* event_args,
       Profile* restrict_to_profile,
-      const std::string& cross_incognito_args,
+      base::ListValue* cross_incognito_args,
       const GURL& event_url);
 
   // Record the Event Ack from the renderer. (One less event in-flight.)
   void OnEventAck(Profile* profile, const std::string& extension_id);
 
  private:
   // The extension and process that contains the event listener for a given
   // event.
   struct ListenerProcess;
 
@@ skipping 23 matching lines @@
   void DispatchLazyEvent(const std::string& extension_id,
                          const linked_ptr<ExtensionEvent>& event);
 
   // Dispatches the event to the specified extension running in |process|.
   void DispatchEventToProcess(const std::string& extension_id,
                               content::RenderProcessHost* process,
                               const linked_ptr<ExtensionEvent>& event);
 
   // Returns false when the event is scoped to a profile and the listening
   // extension does not have access to events from that profile. Also fills
-  // |event_args| with the proper arguments to send, which may differ if
-  // the event crosses the incognito boundary.
+  // |event_args| (if non-NULL) with the proper arguments to send, which may
+  // differ if the event crosses the incognito boundary.
   bool CanDispatchEventToProfile(
       Profile* profile,
       const extensions::Extension* extension,
       const linked_ptr<ExtensionEvent>& event,
-      const base::Value** event_args);
+      base::ListValue** event_args);
 
   // Possibly loads given extension's background page in preparation to
   // dispatch an event.
   void MaybeLoadLazyBackgroundPageToDispatchEvent(
       Profile* profile,
       const extensions::Extension* extension,
       const linked_ptr<ExtensionEvent>& event);
 
   // Track of the number of dispatched events that have not yet sent an
   // ACK from the renderer.
@@ skipping 13 matching lines @@
 
   scoped_refptr<ExtensionDevToolsManager> extension_devtools_manager_;
 
   EventListenerMap listeners_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionEventRouter);
 };
 
 struct ExtensionEvent {
   std::string event_name;
-  scoped_ptr<Value> event_args;
+  scoped_ptr<ListValue> event_args;
   GURL event_url;
   Profile* restrict_to_profile;
-  scoped_ptr<Value> cross_incognito_args;
+  scoped_ptr<ListValue> cross_incognito_args;
   ExtensionEventRouter::UserGestureState user_gesture;
   extensions::EventFilteringInfo info;
 
   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);
-
-  // TODO(gdk): This variant should be retired once the callers are switched to
-  // providing Values instead of just strings.
-  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);
-
-  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);
-
   ~ExtensionEvent();
 };
 
-
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_EVENT_ROUTER_H_