Bindings layer for declarative events API

chrome/renderer/resources/extensions/event.js

 // Copyright (c) 2011 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.
 
 var chrome = chrome || {};
 (function () {
   native function GetChromeHidden();
   native function AttachEvent(eventName);
   native function DetachEvent(eventName);
   native function Print();
@@ skipping 40 matching lines @@
 
   // 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.
   //
   // Example:
   //   chrome.tabs.onChanged = new chrome.Event("tab-changed");
   //   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 "supportsListeners" and "supportsRules".
+  chrome.Event = function(opt_eventName, opt_argSchemas, opt_eventOptions) {

[not at google - send to devlin, 2012/02/01 07:23:12]

eventCapabilities might be a better name?

[battre, 2012/02/01 17:35:39]

Even if we continue adding stuff, such in this example?
        "options": {
          "supportsListeners": false,
          "supportsRules": true,
          "conditions": ["RequestMatcher"],
          "actions": ["CancelRequest", "ModifyRequest"]
        }
Would you call the schema features "capabilities"?

[not at google - send to devlin, 2012/02/01 23:07:23]

Yep, never mind. They're also defined as "options" in the JSON I suppose.

     this.eventName_ = opt_eventName;
     this.listeners_ = [];
+    this.eventOptions_ = opt_eventOptions ||
+        {"supportsListeners": true, "supportsRules": false};
 
     // Validate event parameters if we are in debug.
     if (opt_argSchemas &&
         chromeHidden.validateCallbacks &&
         chromeHidden.validate) {
 
       this.validate_ = function(args) {
         try {
           chromeHidden.validate(args, opt_argSchemas);
         } catch (exception) {
           return "Event validation error during " + opt_eventName + " -- " +
                  exception;
         }
       };
+    } else {
+      this.validate_ = function() {}
     }
+
+    this.ruleIds_ = {};
+    this.lastGeneratedRuleId_ = 0;
   };
 
   // A map of event names to the event object that is registered to that name.
   var attachedNamedEvents = {};
 
   // An array of all attached event objects, used for detaching on unload.
   var allAttachedEvents = [];
 
+  // An array of all events that have had rules added the them at any time.
+  var allEventsWithRules = [];
+
   // A map of functions that massage event arguments before they are dispatched.
   // Key is event name, value is function.
   var eventArgumentMassagers = {};
 
   chromeHidden.Event = {};
 
   chromeHidden.Event.registerArgumentMassager = function(name, fn) {
     if (eventArgumentMassagers[name])
       throw new Error("Massager already registered for event: " + name);
     eventArgumentMassagers[name] = fn;
@@ skipping 23 matching lines @@
   };
 
   // Test if a named event has any listeners.
   chromeHidden.Event.hasListener = function(name) {
     return (attachedNamedEvents[name] &&
             attachedNamedEvents[name].listeners_.length > 0);
   };
 
   // Registers a callback to be called when this event is dispatched.
   chrome.Event.prototype.addListener = function(cb) {
+    if (!this.eventOptions_.supportsListeners)
+      throw new Error("This event does not support listeners.");
     if (this.listeners_.length == 0) {
       this.attach_();
     }
     this.listeners_.push(cb);
   };
 
   // Unregisters a callback.
   chrome.Event.prototype.removeListener = function(cb) {
+    if (!this.eventOptions_.supportsListeners)
+      throw new Error("This event does not support listeners.");
     var idx = this.findListener_(cb);
     if (idx == -1) {
       return;
     }
 
     this.listeners_.splice(idx, 1);
     if (this.listeners_.length == 0) {
       this.detach_();
     }
   };
 
   // Test if the given callback is registered for this event.
   chrome.Event.prototype.hasListener = function(cb) {
+    if (!this.eventOptions_.supportsListeners)
+      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) {
+  chrome.Event.prototype.hasListeners = function() {
+    if (!this.eventOptions_.supportsListeners)
+      throw new Error("This event does not support listeners.");
     return this.listeners_.length > 0;
   };
 
   // Returns the index of the given callback if registered, or -1 if not
   // found.
   chrome.Event.prototype.findListener_ = function(cb) {
     for (var i = 0; i < this.listeners_.length; i++) {
       if (this.listeners_[i] == cb) {
         return i;
       }
     }
 
     return -1;
   };
 
   // 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_.supportsListeners)
+      throw new Error("This event does not support listeners.");
     var args = Array.prototype.slice.call(arguments);
     if (this.validate_) {

[not at google - send to devlin, 2012/02/01 07:23:12]

now that there's always set a validate_ method, this check is unnecessary

       var validationErrors = this.validate_(args);
       if (validationErrors) {
         return validationErrors;
       }
     }
     for (var i = 0; i < this.listeners_.length; i++) {
       try {
         this.listeners_[i].apply(null, args);
       } catch (e) {
         console.error("Error in event handler for '" + this.eventName_ +
@@ skipping 34 matching lines @@
 
     delete attachedNamedEvents[this.eventName_];
   };
 
   chrome.Event.prototype.destroy_ = function() {
     this.listeners_ = [];
     this.validate_ = [];
     this.detach_();
   };
 
+  // Takes a list of JSON datatype identifiers and returns a schema fragment
+  // that verifies that a JSON object corresponds to an array of only these
+  // data types.
+  chrome.Event.prototype.buildArrayOfChoicesSchema_ = function(typesList) {
+    return {
+      "type": "array",
+      "items": {
+        "choices": typesList.forEach(function(el) {return {"$ref": el};})

[not at google - send to devlin, 2012/02/01 08:15:57]

What does this do? I thought forEach has no return value?

[battre, 2012/02/01 17:35:39]

This should have been "map" instead of "forEach".

Done.

+      }
+    };
+  }
+
+  // Validate conditions and actions against specific schemas of this
+  // event object type.
+  // |rules| is an array of JSON objects that follow the Rule type of the
+  // declarative extension APIs. |conditions| is an array of JSON type
+  // identifiers that are allowed to occur in the conditions attribute of each
+  // rule. Likewise, |actions| is an array of JSON type identifiers that are
+  // allowed to occur in the actions attribute of each rule.
+  chrome.Event.prototype.validateRules_ = function(rules, conditions, actions) {
+    if (!conditions || !actions) {
+      throw new Error("Error in API specification.");
+    }
+    var conditionsSchema = this.buildArrayOfChoicesSchema_(conditions);
+    var actionsSchema = this.buildArrayOfChoicesSchema_(actions);
+    rules.forEach(function(rule) {
+      chromeHidden.validate([rule.conditions], [conditionsSchema]);
+      chromeHidden.validate([rule.actions], [actionsSchema]);
+    })
+  }
+
+  chrome.Event.prototype.addMissingIds_ = function(rules) {

[not at google - send to devlin, 2012/02/01 07:23:12]

This, and addMissingPriorities, should be done in a custom_bindings.js file; see
omnibox_custom_bindings.js as an example, but it would look something like:

=== experimental.declarative_custom_bindings.js === 


(function() {

native function GetChromeHidden();

GetChromeHidden().registerCustomHook(
    'experimental.declarative',
    function(bindingsAPI) {
  var apiFunctions = bindingsAPI.apiFunctions;

  apiFunctions.setUpdateArgumentsPostValidate(
      "experimental.declarative.addRules",
      function(event, rules) {
    addMissingIds(rules);
    addMissingPriorities(rules);
    return [event, rules];
  }); 
});

}());

[battre, 2012/02/01 17:35:39]

This does not work:
We want to tell the extension which IDs were assigned to the rules. The custom
hooks work on a copy of the parameters passed to the function. I.e. these
updates are sent to the browser but are invisible to the extension. -> I will
change this so that extension implementation in the browser is responsible for
ID generation and returns the new rules to the extension. I think this is
cleaner.

+    for (var i = 0; i < rules.length; ++i) {
+      // TODO(battre): check for "".
+      if (!("id" in rules[i])) {
+        // Generate a unique ID.
+        var newRuleId = "";
+        do {
+          newRuleId = "_" + (this.lastGeneratedRuleId_++) + "_"
+        } while (newRuleId in this.ruleIds_);
+        // And store it.
+        rules[i]["id"] = newRuleId;
+      }
+      this.ruleIds_[rules[i]["id"]] = 1;
+    }
+  }
+
+  chrome.Event.prototype.addMissingPriorities_ = function(rules) {
+    for (var i = 0; i < rules.length; ++i) {
+      if (!("priority" in rules[i]))
+        rules[i]["priority"] = 100;
+    }
+  }
+
+  chrome.Event.prototype.addRules = function(rules, opt_cb) {

[not at google - send to devlin, 2012/02/01 07:23:12]

I think that this should just be a thin wrapper around
experimental.declarative.addRules which basically curries in the event name. So
just

chrome.experimental.declarative.addRules(this.eventName_, rules, opt_cb);

plus maybe any extra validation.

[battre, 2012/02/01 17:35:39]

Isn't it a pretty thin wrapper? Which part would you like to get rid of?

[not at google - send to devlin, 2012/02/01 23:07:23]

Yep, it is now. By "thin wrapper" I meant... what I said, something which
curries in the appropriate fields into an already-self-describing
"experimental.declarative" API.

Which is what this looks like now, so cool.

+    if (!this.eventOptions_.supportsRules)
+      throw new Error("This event does not support rules.");
+
+    var found = false;
+    for (var i = 0; i < allEventsWithRules.length; ++i) {
+      if (allEventsWithRules === this) {
+        found = true;
+        break;
+      }
+    }
+    if (!found)
+      allEventsWithRules.push(this);

[not at google - send to devlin, 2012/02/01 07:23:12]

This implies an O(n) iteration over all events each time you call addRules,
right? How about setting a property on the event, like

if (rules.length > 0)
  this.hasRules_ = true;

[battre, 2012/02/01 17:35:39]

I have removed this logic, see below.

+
+    this.validateRules_(rules,

[not at google - send to devlin, 2012/02/01 07:23:12]

I would have thought that the schema-based validation should be the
responsibility of the rule factories, like net.RequestMatcher or whatever.
You'll have to do similar work in there to pull out the necessary fields anyway.

It might help to see what implementations of those classes would look like, but
I'd imagine something like

function RequestMatcher(configuration) {
  validateAgainstSchema(schema, configuration);
  this.buildConditionFromRequestMatcher_(configuration);
}

I imagine you'd still want to validate here that the filters/actions or whatever
part of the rules are the correct instances?

[not at google - send to devlin, 2012/02/01 08:15:57]

I may have gotten my wires a little bit crossed when I made this comment. Let me
rephrase:

Going by the design doc, the caller of the webRequest API (or whtaever), when
setting up rules, will need to go like "new RequestMatcher()" or whatever, and
pass in an object.  You'll need to provide a way of actually implementing these;
will it be something along the lines of

function RequestMatcher(o) {
  var self = this;
  forEach(o, function(key, value) {
    self[key] = value;
  });
  validate(getRequestMatcherSchema(), this);
  this.id = 'net.RequestMatcher'; // not sure about this step?
}

I'm just a bit fuzzy on the details without having an example.
  

[battre, 2012/02/01 17:35:39]

I am not sure how how we will implement these constructors exactly. However, we
need to check that we don't pass "WindowSizeChanged" conditions into the
webrequest API. Therefore, I think that the validation at this place makes
sense.

+                        this.eventOptions_.conditions,
+                        this.eventOptions_.actions);
+    this.addMissingIds_(rules);
+    this.addMissingPriorities_(rules);
+    var callback = opt_cb ? opt_cb.bind(undefined, rules) : undefined;
+
+    return chromeHidden.validatingSendRequest.call(

[not at google - send to devlin, 2012/02/01 07:23:12]

This is interesting. All you really need to do, at least at the moment, is call
the API function itself directly (if it exists; which might not happen if the
extension doesn't have the experimental permission)

so like

if (chrome.experimental && chrome.experimental.declarative)
  chrome.experimental.declarative.addRules(this.eventName_, rules, callback);

However, this brings up the point that as it stands the experimental.declarative
API will be defined as a directly-callable API, as well as one that uses Events.
I'm not sure this is desirable. I guess I don't really see a problem, though
it's a bit ambiguous given you can also do it through the Event objects.

Anyway, to not generate the API we'd need to add an extra property to the schema
like disableBindingsGeneration or something.

However, if we DO go along with having the API callable, then most of this logic
will need to be moved to a custom_bindings file, as I've commented above.

[battre, 2012/02/01 17:35:39]

Wow, this became significantly simpler.

Done.

+        this,
+        "experimental.declarative.addRules",
+        [this.eventName_, rules, callback]);
+  }
+
+  chrome.Event.prototype.removeRules = function(ruleIdentifiers, opt_cb) {
+    if (!this.eventOptions_.supportsRules)
+      throw new Error("This event does not support rules.");
+    return chromeHidden.validatingSendRequest.call(
+        this,
+        "experimental.declarative.removeRules",
+        [this.eventName_, ruleIdentifiers, opt_cb]);
+  }
+
+  chrome.Event.prototype.getRules = function(ruleIdentifiers, cb) {
+    if (!this.eventOptions_.supportsRules)
+      throw new Error("This event does not support rules.");
+    return chromeHidden.validatingSendRequest.call(
+        this,
+        "experimental.declarative.getRules",
+        [this.eventName_, ruleIdentifiers, cb]);
+  }
+
   // 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).
   chromeHidden.onLoad = new chrome.Event();
   chromeHidden.onUnload = new chrome.Event();
 
   chromeHidden.dispatchOnLoad = function(extensionId, isExtensionProcess,
                                          isIncognitoContext) {
     chromeHidden.onLoad.dispatch(extensionId, isExtensionProcess,
                                  isIncognitoContext);
   };
 
   chromeHidden.dispatchOnUnload = function() {
     chromeHidden.onUnload.dispatch();
     for (var i = 0; i < allAttachedEvents.length; ++i) {
       var event = allAttachedEvents[i];

[not at google - send to devlin, 2012/02/01 07:23:12]

I.e. with the above suggestion, this would be

var event = ...;
if (event.hasRules_)
  event.removeRules(?);

(I'm puzzled as to why this is necessary; it seems odd to me that events/rules
are unloaded by the renderers rather than the browser or whatever figuring that
out by itself. But whatever, I'm presumably missing a lot of prior context).

[battre, 2012/02/01 17:35:39]

Solved, see below.

       if (event)
         event.detach_();
     }
+    for (var i = 0; i < allEventsWithRules.length; ++i) {
+      var event = allEventsWithRules[i];
+      if (event && event.eventOptions_.supportsRules)
+        event.removeRules([]);

[not at google - send to devlin, 2012/02/01 07:23:12]

Why [] ? Is this to signify removing all rules?

This seems surprising. It would be consistent for other APIs for removing all
rules to be "event.removeRules(null)" instead. Removing [] would remove no
rules, I would have thought.

[battre, 2012/02/01 17:35:39]

I have removed this logic because
a) it did not work reliably (dispatchOnUnload was not called in my browser
tests) and 
b) we can trigger the removal of rules from the "extension unloaded" signal sent
by the ExtensionService in the browser

+    }
   };
 
   chromeHidden.dispatchError = function(msg) {
     console.error(msg);
   };
 })();