Filtered events.

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 8961ce56fc112758d971b496ccaa842b070af33b..9e35a13b64408cc37a029e6f59541be8e95b42ca 100644
--- a/chrome/renderer/resources/extensions/event.js
+++ b/chrome/renderer/resources/extensions/event.js
@@ -5,6 +5,9 @@
   var eventBindingsNatives = requireNative('event_bindings');
   var AttachEvent = eventBindingsNatives.AttachEvent;
   var DetachEvent = eventBindingsNatives.DetachEvent;
+  var AttachFilteredEvent = eventBindingsNatives.AttachFilteredEvent;
+  var DetachFilteredEvent = eventBindingsNatives.DetachFilteredEvent;
+  var MatchAgainstEventFilter = eventBindingsNatives.MatchAgainstEventFilter;
   var sendRequest = require('sendRequest').sendRequest;
   var utils = require('utils');
 
@@ -74,6 +77,77 @@
     };
   })();
 
+  // Handles adding/removing/dispatching listeners for unfiltered events.
+  var UnfilteredAttachmentStrategy = function(event) {

[Matt Perry, 2012/06/13 01:24:27]

How much overhead would it be if we just treated unfiltered events as filtered
events with a NULL filter? That would probably simplify the code a ton, yes?

[koz (OOO until 15th September), 2012/06/14 02:15:55]

Yeah, it would. I don't think the overhead would be extremely high... I was
originally reluctant to remove the unfiltered events "fast path" because
filtered events are inherently less efficient.

The main consideration is that with filtered events it's O(N^2) to add N
listeners on a single event (because we need to figure out if another filter
equal to the added one already exists), whereas for unfiltered events it's O(N).
But probably extensions that add many listeners to the same event are rare?

The other cost would be adding unnecessary entries into EventFilter, which means
that its URLMatcher would have to rebuild its internal data structures on every
add, but we do this for filters without events for code simplicity.

I can investigate this path more today. If it proves feasible do you think I
should change this patch, or land this first and then remove the code that
handles unfiltered events? That approach seems more incremental to me.

[Matt Perry, 2012/06/14 18:48:56]

Could EventFilter skip rebuilding if there are no URL filters?
Doesn't matter a whole lot to me. Though I think I disagree with your definition
of incremental :)

[koz (OOO until 15th September), 2012/06/15 00:07:14]

Yes, that could be done. It would require making the code more complex though
(which isn't to say it's not worth it.)
Heh yeah :-) Well, my thinking is that it's an incremental change in the sense
that only one API is translated over to the "new way", rather than all of them.
Having a unified attachment strategy is more complicated than either of the
Filtered / Unfiltered ones, but it is less complicated than Filtered +
Unfiltered.

[Matt Perry, 2012/06/15 00:10:40]

That's a good point. I'm convinced.

+    this.event_ = event;
+  };
+
+  UnfilteredAttachmentStrategy.prototype.onAddedListener =
+      function(listener) {
+    // Only attach / detach on the first / last listener removed.
+    if (this.event_.listeners_.length == 0)
+      AttachEvent(this.event_.eventName_);
+  };
+
+  UnfilteredAttachmentStrategy.prototype.onRemovedListener =
+      function(listener) {
+    if (this.event_.listeners_.length == 0)
+      this.detach(true);
+  };
+
+  UnfilteredAttachmentStrategy.prototype.detach = function(manual) {
+    var i = allAttachedEvents.indexOf(this.event_);
+    if (i >= 0)
+      delete allAttachedEvents[i];
+    DetachEvent(this.event_.eventName_, manual);
+  };
+
+  UnfilteredAttachmentStrategy.prototype.getListenersByIDs = function(ids) {
+    return this.event_.listeners_;
+  };
+
+  var FilteredAttachmentStrategy = function(event) {
+    this.event_ = event;
+    this.listenerMap_ = {};
+  };
+
+  FilteredAttachmentStrategy.idToEventMap = {};
+
+  FilteredAttachmentStrategy.prototype.onAddedListener = function(listener) {
+    var id = AttachFilteredEvent(this.event_.eventName_,
+        listener.filters || {});
+    listener.id = id;
+    this.listenerMap_[id] = listener;
+    FilteredAttachmentStrategy.idToEventMap[id] = this.event_;
+  };
+
+  FilteredAttachmentStrategy.prototype.onRemovedListener = function(listener) {
+    this.detachListener(listener, true);
+  };
+
+  FilteredAttachmentStrategy.prototype.detachListener =
+      function(listener, manual) {
+    if (listener.id == undefined)
+      throw new Error("listener.id undefined - '" + listener + "'");
+    var id = listener.id;
+    delete this.listenerMap_[id];
+    delete FilteredAttachmentStrategy.idToEventMap[id];
+    DetachFilteredEvent(id, manual);
+  };
+
+  FilteredAttachmentStrategy.prototype.detach = function(manual) {
+    for (var i in this.listenerMap_)
+      this.detachListener(this.listenerMap_[i], manual);
+  };
+
+  FilteredAttachmentStrategy.prototype.getListenersByIDs = function(ids) {
+    console.log('FilteredAttachmentStrategy.getListenersByIDs(' + ids + ')');
+    var result = [];
+    for (var i = 0; i < ids.length; i++)
+      result.push(this.listenerMap_[ids[i]]);
+    return result;
+  };
+
   // Event object.  If opt_eventName is provided, this object represents
   // the unique instance of that named event, and dispatching an event
   // with that name will route through this object's listeners. Note that
@@ -91,11 +165,20 @@
     this.eventName_ = opt_eventName;
     this.listeners_ = [];
     this.eventOptions_ = opt_eventOptions ||
-        {"supportsListeners": true, "supportsRules": false};
+        {supportsFilters: false,
+         supportsListeners: true,
+         supportsRules: false,
+        };
 
     if (this.eventOptions_.supportsRules && !opt_eventName)
       throw new Error("Events that support rules require an event name.");
 
+    if (this.eventOptions_.supportsFilters) {
+      this.attachmentStrategy_ = new FilteredAttachmentStrategy(this);
+    } else {
+      this.attachmentStrategy_ = new UnfilteredAttachmentStrategy(this);
+    }
+
     // Validate event arguments (the data that is passed to the callbacks)
     // if we are in debug.
     if (opt_argSchemas &&
@@ -136,15 +219,21 @@
   // Dispatches a named event with the given JSON array, which is deserialized
   // before dispatch. The JSON array is the list of arguments that will be
   // sent with the event callback.
-  chromeHidden.Event.dispatchJSON = function(name, args) {
+  chromeHidden.Event.dispatchJSON = function(name, args, filteringInfo) {
+    var listenerIDs;
+
+    if (filteringInfo) {
+      filteringInfo = chromeHidden.JSON.parse(filteringInfo);
+      listenerIDs = MatchAgainstEventFilter(name, filteringInfo);
+      console.log('listenerIDs = ' + listenerIDs);
+    }
     if (attachedNamedEvents[name]) {
       if (args) {
         args = chromeHidden.JSON.parse(args);
         if (eventArgumentMassagers[name])
           eventArgumentMassagers[name](args);
       }
-      var result = attachedNamedEvents[name].dispatch.apply(
-          attachedNamedEvents[name], args);
+      var result = attachedNamedEvents[name].dispatch_(args, listenerIDs);
       if (result && result.validationErrors)
         return result.validationErrors;
     }
@@ -165,13 +254,34 @@
   };
 
   // Registers a callback to be called when this event is dispatched.
-  chrome.Event.prototype.addListener = function(cb) {
+  chrome.Event.prototype.addListener = function(cb, filters) {
     if (!this.eventOptions_.supportsListeners)
       throw new Error("This event does not support listeners.");
+    if (filters) {
+      if (!this.eventOptions_.supportsFilters)
+        throw new Error("This event does not support filters.");
+      if (filters.url && !(filters.url instanceof Array))
+        throw new Error("filters.url should be an array");
+    }
+    var listener = {callback: cb, filters: filters};
+    this.attach_(listener);
+    this.listeners_.push(listener);
+  };
+
+  chrome.Event.prototype.attach_ = function(listener) {
+    this.attachmentStrategy_.onAddedListener(listener);
     if (this.listeners_.length == 0) {
-      this.attach_();
+      allAttachedEvents[allAttachedEvents.length] = this;
+      if (!this.eventName_)
+        return;
+
+      if (attachedNamedEvents[this.eventName_]) {
+        throw new Error("chrome.Event '" + this.eventName_ +
+                        "' is already attached.");
+      }
+
+      attachedNamedEvents[this.eventName_] = this;
     }
-    this.listeners_.push(cb);
   };
 
   // Unregisters a callback.
@@ -183,9 +293,19 @@
       return;
     }
 
-    this.listeners_.splice(idx, 1);
+    var removedListener = this.listeners_.splice(idx, 1)[0];
+    this.attachmentStrategy_.onRemovedListener(removedListener);
+
     if (this.listeners_.length == 0) {
-      this.detach_(true);
+      if (!this.eventName_)
+        return;
+
+      if (!attachedNamedEvents[this.eventName_]) {
+        throw new Error("chrome.Event '" + this.eventName_ +
+                        "' is not attached.");
+      }
+
+      delete attachedNamedEvents[this.eventName_];
     }
   };
 
@@ -207,7 +327,7 @@
   // found.
   chrome.Event.prototype.findListener_ = function(cb) {
     for (var i = 0; i < this.listeners_.length; i++) {
-      if (this.listeners_[i] == cb) {
+      if (this.listeners_[i].callback == cb) {
         return i;
       }
     }
@@ -215,20 +335,20 @@
     return -1;
   };
 
-  // Dispatches this event object to all listeners, passing all supplied
-  // arguments to this function each listener.
-  chrome.Event.prototype.dispatch = function(varargs) {
+  chrome.Event.prototype.dispatch_ = function(args, listenerIDs) {
     if (!this.eventOptions_.supportsListeners)
       throw new Error("This event does not support listeners.");
-    var args = Array.prototype.slice.call(arguments);
     var validationErrors = this.validateEventArgs_(args);
     if (validationErrors) {
       return {validationErrors: validationErrors};
     }
+
+    var listeners = this.attachmentStrategy_.getListenersByIDs(listenerIDs);
+
     var results = [];
-    for (var i = 0; i < this.listeners_.length; i++) {
+    for (var i = 0; i < listeners.length; i++) {
       try {
-        var result = this.listeners_[i].apply(null, args);
+        var result = listeners[i].callback.apply(null, args);
         if (result !== undefined)
           results.push(result);
       } catch (e) {
@@ -238,39 +358,17 @@
     }
     if (results.length)
       return {results: results};
-  };
-
-  // Attaches this event object to its name.  Only one object can have a given
-  // name.
-  chrome.Event.prototype.attach_ = function() {
-    AttachEvent(this.eventName_);
-    allAttachedEvents[allAttachedEvents.length] = this;
-    if (!this.eventName_)
-      return;
-
-    if (attachedNamedEvents[this.eventName_]) {
-      throw new Error("chrome.Event '" + this.eventName_ +
-                      "' is already attached.");
-    }
+  }
 
-    attachedNamedEvents[this.eventName_] = this;
+  // Dispatches this event object to all listeners, passing all supplied
+  // arguments to this function each listener.
+  chrome.Event.prototype.dispatch = function(varargs) {
+    return this.dispatch_(Array.prototype.slice.call(arguments), undefined);
   };
 
   // Detaches this event object from its name.
-  chrome.Event.prototype.detach_ = function(manual) {
-    var i = allAttachedEvents.indexOf(this);
-    if (i >= 0)
-      delete allAttachedEvents[i];
-    DetachEvent(this.eventName_, manual);
-    if (!this.eventName_)
-      return;
-
-    if (!attachedNamedEvents[this.eventName_]) {
-      throw new Error("chrome.Event '" + this.eventName_ +
-                      "' is not attached.");
-    }
-
-    delete attachedNamedEvents[this.eventName_];
+  chrome.Event.prototype.detach_ = function() {
+    this.attachmentStrategy_.detach(false);
   };
 
   chrome.Event.prototype.destroy_ = function() {
@@ -279,6 +377,15 @@
     this.detach_(false);
   };
 
+  // Gets the declarative API object, or undefined if this extension doesn't
+  // have access to it.
+  //
+  // This is defined as a function (rather than a variable) because it isn't
+  // accessible until the schema bindings have been generated.
+  function getDeclarativeAPI() {

[Matt Perry, 2012/06/13 01:24:27]

is this used?

[koz (OOO until 15th September), 2012/06/14 02:15:55]

Hm, not sure where this came from. Removed.

+    return chromeHidden.internalAPIs.declarative;
+  }
+
   chrome.Event.prototype.addRules = function(rules, opt_cb) {
     if (!this.eventOptions_.supportsRules)
       throw new Error("This event does not support rules.");
@@ -357,7 +464,7 @@
     for (var i = 0; i < allAttachedEvents.length; ++i) {
       var event = allAttachedEvents[i];
       if (event)
-        event.detach_(false);
+        event.detach_();
     }
   };