Bindings layer for declarative events API

chrome/renderer/resources/extensions/schema_generated_bindings.js

Index: chrome/renderer/resources/extensions/schema_generated_bindings.js
diff --git a/chrome/renderer/resources/extensions/schema_generated_bindings.js b/chrome/renderer/resources/extensions/schema_generated_bindings.js
index 413476272c462cfa1d4e332edc3ff488334285c4..82c82de33eb3b2aa9ab02aedad5bbf6a21d6c625 100644
--- a/chrome/renderer/resources/extensions/schema_generated_bindings.js
+++ b/chrome/renderer/resources/extensions/schema_generated_bindings.js
@@ -183,6 +183,24 @@ var chrome = chrome || {};
                           opt_args.forIOThread);
   }
 
+  function getFunctionDefinition(namespace, functionName) {
+    var filterNamespace = function(val) {return val.namespace === namespace;};
+    var apiSchema = chromeHidden.apiDefinitions.filter(filterNamespace)[0];
+    var filterFunctionName = function (val) {return val.name === functionName;};

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

This is expensive, O(number of APIs * number of functions per API) each time
validatingSendRequest is called.

I like Aaron's suggestion in sendRequest. That is, check if argSchemas is
defined, if not then populate it like you've done here.

However... you'd need to transform the API definitions into a more useful form,
that doesn't involve a linear lookup each time. (like being able to do
schemas[namespace].functions[functionName] for this).

For now, I'd just cache the functionName->schema mapping in sendRequest, and put
that schema transformation as a TODO. It's part of a larger refactor related to
moving all the schemas from using lists-with-ids to using dictionaries... which
I will get around to doing, eventually.

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

I have moved this logic into the custom bindings. Please have a look. I think
this is now sufficiently fast and simple.

Done.

+    return apiSchema.functions.filter(filterFunctionName)[0];
+  }
+
+  // Sends an API request like sendRequest() but performs schema validation
+  // first.
+  // |qualifiedFunctionName| is the full function name including the api name.
+  chromeHidden.validatingSendRequest = function(qualifiedFunctionName, args) {

[Aaron Boodman, 2012/02/01 00:27:25]

I don't think this really validates. sendRequest doesn't normally do the
validation, it is the code that calls sendRequest. The only thing sendRequest
does with the schemas is call prepareRequest, which has special handling for the
callback.

Do you need validation to happen here? If so, a bit more refactoring is needed.
Otherwise, change the name of this function (or just make sendRequest do the
lookup if schemas are undefined).

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

I think we should get sendRequest to do the lookup itself, so a bit of
refactoring.  I'm looking at where sendRequest is called though (as in, existing
code), and that code seems to
 1. pull the schema definition of out of "this.definition"
 2. pass it to sendRequest
so basically, it uses "this.description" unnecessarily, not to mention the
wtfness of using "this" at all.  I think it would be fairly straightfoward for
sendRequest to pull out the schema information more or less how you've done it
for validatingSendRequest.

Anyway -- koz and I are going through this change in more detail (trying to
understand it etc), but just commenting on this now since aa's comment showed up
in my inbox.

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

Please have a look at the new implementation. This function is gone now.

+    var lastSeparator = qualifiedFunctionName.lastIndexOf(".");
+    var functionName = qualifiedFunctionName.substr(lastSeparator + 1);
+    var namespace = qualifiedFunctionName.substr(0, lastSeparator);
+    var functionDef = getFunctionDefinition(namespace, functionName);
+    return sendRequest(qualifiedFunctionName, args, functionDef.parameters);
+  };
+
   // TODO(kalman): It's a shame to need to define this function here, since it's
   // only used in 2 APIs (browserAction and pageAction). It would be nice to
   // only load this if one of those APIs has been loaded.
@@ -355,6 +373,7 @@ var chrome = chrome || {};
   chromeHidden.onLoad.addListener(function(extensionId, isExtensionProcess,
                                            isIncognitoProcess) {
     var apiDefinitions = GetExtensionAPIDefinition();
+    chromeHidden.apiDefinitions = apiDefinitions;

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

When the logic from event.js is moved into
experimental.declarative_custom_bindings.js, you won't have to put
apiDefinitions or validatingSendRequest/sendRequest on chromeHidden.

 
     // Read api definitions and setup api functions in the chrome namespace.
     // TODO(rafaelw): Consider defining a json schema for an api definition
@@ -459,10 +478,11 @@ var chrome = chrome || {};
           var customEvent = customEvents[apiDef.namespace];
           if (customEvent) {
             module[eventDef.name] = new customEvent(
-                eventName, eventDef.parameters, eventDef.extraParameters);
+                eventName, eventDef.parameters, eventDef.extraParameters,
+                eventDef.options);
           } else {
             module[eventDef.name] = new chrome.Event(
-                eventName, eventDef.parameters);
+                eventName, eventDef.parameters, eventDef.options);
           }
         });
       }