Plumb listenerIDs correctly for events that clobber chrome.Event.prototype.dispatch.

chrome/renderer/resources/extensions/event.js

Index: chrome/renderer/resources/extensions/event.js
diff --git a/chrome/renderer/resources/extensions/event.js b/chrome/renderer/resources/extensions/event.js
index 4b784ad0e17dd3803fc8b48bf208db35b7ddb3bf..7d04ac4aec2bb45bca83c5bd6fda85f8286ee0ad 100644
--- a/chrome/renderer/resources/extensions/event.js
+++ b/chrome/renderer/resources/extensions/event.js
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+  var DCHECK = requireNative('debug').DCHECK;
   var eventBindingsNatives = requireNative('event_bindings');
   var AttachEvent = eventBindingsNatives.AttachEvent;
   var DetachEvent = eventBindingsNatives.DetachEvent;
@@ -208,10 +209,13 @@
 
   chromeHidden.Event = {};
 
-  chromeHidden.Event.registerArgumentMassager = function(name, fn) {
+  // callback is a function(args, dispatch). args are the args we recieve from
+  // dispatchJSON(), and dispatch is a function(args) that dispatches args to
+  // its listeners.
+  chromeHidden.Event.registerArgumentMassager = function(name, callback) {
     if (eventArgumentMassagers[name])
       throw new Error("Massager already registered for event: " + name);
-    eventArgumentMassagers[name] = fn;
+    eventArgumentMassagers[name] = callback;
   };
 
   // Dispatches a named event with the given JSON array, which is deserialized
@@ -223,40 +227,32 @@
     if (filteringInfo) {
       listenerIDs = MatchAgainstEventFilter(name, filteringInfo);
     }
-    if (attachedNamedEvents[name]) {
-      if (args) {
-        // TODO(asargent): This is an antiquity. Until all callers of
-        // dispatchJSON use actual values, this must remain here to catch the
-        // cases where a caller has hard-coded a JSON string to pass in.
-        if (typeof(args) == "string") {
-          args = chromeHidden.JSON.parse(args);
-        }
-        if (eventArgumentMassagers[name])
-          eventArgumentMassagers[name](args);
-      }
+    var event = attachedNamedEvents[name];
+    if (!event)
+      return;
 
-      var event = attachedNamedEvents[name];
-      var result;
-      // TODO(koz): We have to do this differently for unfiltered events (which
-      // have listenerIDs = null) because some bindings write over
-      // event.dispatch (eg: experimental.app.custom_bindings.js) and so expect
-      // events to go through it. These places need to be fixed so that they
-      // expect a listenerIDs parameter.
-      if (listenerIDs)
-        result = event.dispatch_(args, listenerIDs);
-      else
-        result = event.dispatch.apply(event, args);
+    // TODO(asargent): This is an antiquity. Until all callers of
+    // dispatchJSON use actual values, this must remain here to catch the
+    // cases where a caller has hard-coded a JSON string to pass in.
+    if (typeof(args) == "string")
+      args = chromeHidden.JSON.parse(args);
+
+    var dispatchArgs = function(args) {
+      result = event.dispatch_(args, listenerIDs);
       if (result && result.validationErrors)
-        return result.validationErrors;
-    }
+        DCHECK(false, result.validationErrors);

[not at google - send to devlin, 2012/07/03 07:47:04]

Either incorporate the condition into the DCHECK, or expose a NOTREACHED?

[koz (OOO until 15th September), 2012/07/05 04:38:32]

Done.

+    };
+
+    if (eventArgumentMassagers[name])
+      eventArgumentMassagers[name](args, dispatchArgs);
+    else
+      dispatchArgs(args);
   };
 
   // Dispatches a named event with the given arguments, supplied as an array.
   chromeHidden.Event.dispatch = function(name, args) {

[benwells, 2012/07/03 07:38:18]

Why do we have this function as well as dispatchJSON? This only seems to be used
from a unit test.

[koz (OOO until 15th September), 2012/07/05 04:38:32]

Removed.

-    if (attachedNamedEvents[name]) {
-      attachedNamedEvents[name].dispatch.apply(
-          attachedNamedEvents[name], args);
-    }
+    if (attachedNamedEvents[name])
+      attachedNamedEvents[name].dispatch_(args, undefined);
   };
 
   // Test if a named event has any listeners.