Bindings layer for declarative events API

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 45d7080a6c478b2fbc9002e6ccba527ea80c57a2..dbacecee85f38a16b0c1511ae2c8a67375624a68 100644
--- a/chrome/renderer/resources/extensions/event.js
+++ b/chrome/renderer/resources/extensions/event.js
@@ -8,6 +8,8 @@ var chrome = chrome || {};
   native function AttachEvent(eventName);
   native function DetachEvent(eventName);
   native function Print();
+  //native function AddRules(rules);
+  //native function RemoveRules(rules);
 
   var chromeHidden = GetChromeHidden();
 
@@ -58,9 +60,22 @@ var chrome = chrome || {};
   //   chrome.tabs.onChanged.addListener(function(data) { alert(data); });
   //   chromeHidden.Event.dispatch("tab-changed", "hi");
   // will result in an alert dialog that says 'hi'.
-  chrome.Event = function(opt_eventName, opt_argSchemas) {
+  //
+  // If opt_eventOptions exists, it is a dictionary that contains the boolean
+  // entries "supports_listeners" and "supports_rules".
+  chrome.Event = function(opt_eventName, opt_argSchemas, opt_eventOptions,
+                          opt_typesAPI) {
     this.eventName_ = opt_eventName;
     this.listeners_ = [];
+    this.eventOptions_ = opt_eventOptions ||
+        {'supports_listeners': true, 'supports_rules': false};

[Aaron Boodman, 2012/01/19 23:09:46]

Nit: don't need the single quotes in the object literal here.

[Matt Perry, 2012/01/20 00:07:55]

also, use camelCase for dict keys

[battre, 2012/01/24 20:36:16]

Done.

[battre, 2012/01/24 20:36:16]

Done.

+    if (opt_typesAPI) {
+      this.sendRequest_ = opt_typesAPI.sendRequest;
+      this.apiDefinitions_ = opt_typesAPI.apiDefinitions;
+    } else {
+      this.sendRequest_ = function() {};
+      this.apiDefinitions_ = {};
+    }
 
     // Validate event parameters if we are in debug.
     if (opt_argSchemas &&
@@ -127,6 +142,8 @@ var chrome = chrome || {};
 
   // Registers a callback to be called when this event is dispatched.
   chrome.Event.prototype.addListener = function(cb) {
+    if (!this.eventOptions_.supports_listeners)
+      throw new Error("This event does not support listeners.");
     if (this.listeners_.length == 0) {
       this.attach_();
     }
@@ -135,6 +152,8 @@ var chrome = chrome || {};
 
   // Unregisters a callback.
   chrome.Event.prototype.removeListener = function(cb) {
+    if (!this.eventOptions_.supports_listeners)
+      throw new Error("This event does not support listeners.");
     var idx = this.findListener_(cb);
     if (idx == -1) {
       return;
@@ -148,11 +167,15 @@ var chrome = chrome || {};
 
   // Test if the given callback is registered for this event.
   chrome.Event.prototype.hasListener = function(cb) {
+    if (!this.eventOptions_.supports_listeners)
+      throw new Error("This event does not support listeners.");
     return this.findListener_(cb) > -1;
   };
 
   // Test if any callbacks are registered for this event.
   chrome.Event.prototype.hasListeners = function(cb) {

[Aaron Boodman, 2012/01/19 23:09:46]

whoops: the cb param here not required.

[battre, 2012/01/24 20:36:16]

Done.

+    if (!this.eventOptions_.supports_listeners)
+      throw new Error("This event does not support listeners.");
     return this.listeners_.length > 0;
   };
 
@@ -171,6 +194,8 @@ var chrome = chrome || {};
   // Dispatches this event object to all listeners, passing all supplied
   // arguments to this function each listener.
   chrome.Event.prototype.dispatch = function(varargs) {
+    if (!this.eventOptions_.supports_listeners)
+      throw new Error("This event does not support listeners.");
     var args = Array.prototype.slice.call(arguments);
     if (this.validate_) {
       var validationErrors = this.validate_(args);
@@ -227,6 +252,54 @@ var chrome = chrome || {};
     this.detach_();
   };
 
+  chrome.Event.prototype.getFunctionDefinition_ =
+      function(namespace, functionName) {
+    var filterNamespace = function(val) {return val.namespace === namespace;};
+    var apiSchema = this.apiDefinitions_.filter(filterNamespace)[0];
+    var filterFunctionName = function (val) {return val.name === functionName;};
+    return apiSchema.functions.filter(filterFunctionName)[0];
+  }
+
+  chrome.Event.prototype.addRules = function(rules, opt_cb) {
+    if (!this.eventOptions_.supports_rules)

[Aaron Boodman, 2012/01/19 23:09:46]

nit: maybe just put supports_rules_ on |this|?

[battre, 2012/01/24 20:36:16]

hm... I think having this as a separate dictionary is not a bad idea because
that way we can document what we expect in the extensions_api.json file.

+      throw new Error("This event does not support listeners.");

[Aaron Boodman, 2012/01/19 23:09:46]

s/listener/rule/

[battre, 2012/01/24 20:36:16]

Done.

+    // TODO: wrap callback in such a way that
+    //       - the rules are stored
+    //       - rule IDs are updated
+
+    // Validate conditions and actions against specific schemas of this
+    // event object type.
+    var conditionsSchema = this.eventOptions_.conditions;
+    var actionsSchema = this.eventOptions_.actions;
+    if (!conditionsSchema || !actionsSchema) {
+      throw new Error("Error in API specification.");
+    }
+    rules.forEach(function(rule) {
+      chromeHidden.validate([rule.conditions], [conditionsSchema]);
+      chromeHidden.validate([rule.actions], [actionsSchema]);

[Aaron Boodman, 2012/01/19 23:09:46]

Interesting :) I hadn't considered that we could just use JSON Schema so
directly here.

+    })
+
+    var functionDef =
+        this.getFunctionDefinition_("experimental.declarative", "addRules");

[Aaron Boodman, 2012/01/19 23:09:46]

I think it might be easier to just use raw IPC here. Define a set of native
functions like addRule/removeRule/etc, and then define IPC messages that get
picked up in the browser and send to the right registry.

I don't think we get that much by reusing ExtensionFunctionDispatcher.

Matt, what do you think?

[battre, 2012/01/19 23:45:48]

I started off with this approach. Here is the deal:
If you register rules, these need to be sent to the RuleRegistry, which lives in
the Browser, they need to be validated and a response needs to be generated. If
we use native functions we are replicating everything that is already available
to sendRequest:
- validation of arguments
- asynchronous messaging with callbacks (addRules needs to "return" something to
the callback function)
- association of messages to their extensions?
- possibly some security features?

And we need to do a lot of plumbing with messages. I preferred this approach.

[Matt Perry, 2012/01/20 00:07:55]

This seems reasonable to me. No sense rolling custom IPC messages when
sendRequest does what we need.

I admit that I'm kind of confused on how you're going about this, though. The
getFunctionDefinition_ followed by sendRequest_ seems complex and hard to
understand. Why doesn't the standard usage of sendRequest work here?

[battre, 2012/01/24 20:36:16]

We don't have access to these functions, they are implementation details of a
(function() {})() block in schema_generated_bindings.js

+    return this.sendRequest_.call(this,
+                                  "experimental.declarative.addRules",
+                                  [this.eventName_, rules, opt_cb],
+                                  functionDef.parameters);
+  }
+
+  chrome.Event.prototype.removeRules = function(cb) {
+    if (!this.eventOptions_.supports_rules)
+      throw new Error("This event does not support listeners.");
+    // TODO(battre): Remove rules.
+  }
+
+  chrome.Event.prototype.getRules = function(rule_identifiers, cb) {
+    if (!this.eventOptions_.supports_rules)
+      throw new Error("This event does not support listeners.");
+      // TODO(battre): Get rules.
+    cb.apply(this, []);
+  }
+
   // Special load events: we don't use the DOM unload because that slows
   // down tab shutdown.  On the other hand, onUnload might not always fire,
   // since Chrome will terminate renderers on shutdown (SuddenTermination).
@@ -246,6 +319,7 @@ var chrome = chrome || {};
       if (event)
         event.detach_();
     }
+    // TODO(battre): Remove all rules
   };
 
   chromeHidden.dispatchError = function(msg) {