Bindings layer for declarative events API

chrome/renderer/resources/extensions/schema_generated_bindings.js

 // Copyright (c) 2012 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.
 
 // This script contains privileged chrome extension related javascript APIs.
 // It is loaded by pages whose URL has the chrome-extension protocol.
 
 var chrome = chrome || {};
 (function() {
   native function GetChromeHidden();
@@ skipping 165 matching lines @@
 
     var requestId = GetNextRequestId();
     request.id = requestId;
     requests[requestId] = request;
     var hasCallback =
         (request.callback || opt_args.customCallback) ? true : false;
     return nativeFunction(functionName, sargs, requestId, hasCallback,
                           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.
   // That said, both of those APIs are always injected into pages anyway (see
   // chrome/common/extensions/extension_permission_set.cc).
   function setIcon(details, name, parameters, actionType) {
     var iconSize = 19;
     if ("iconIndex" in details) {
       sendRequest(name, [details], parameters);
     } else if ("imageData" in details) {
@@ skipping 152 matching lines @@
       if (platforms[i][0].test(navigator.appVersion)) {
         return platforms[i][1];
       }
     }
     return "unknown";
   }
 
   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
     //   and validating either here, in a unit_test or both.
     // TODO(rafaelw): Handle synchronous functions.
     // TODO(rafaelw): Consider providing some convenient override points
     //   for api functions that wish to insert themselves into the call.
     var platform = getPlatform();
 
     apiDefinitions.forEach(function(apiDef) {
@@ skipping 84 matching lines @@
           if (eventDef.name in module ||
               addUnprivilegedAccessGetter(module, eventDef.name,
                                           eventDef.unprivileged)) {
             return;
           }
 
           var eventName = apiDef.namespace + "." + eventDef.name;
           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);
           }
         });
       }
 
       function addProperties(m, def) {
         // Parse any values defined for properties.
         if (def.properties) {
           forEach(def.properties, function(prop, property) {
             if (prop in m ||
                 addUnprivilegedAccessGetter(m, prop, property.unprivileged)) {
@@ skipping 70 matching lines @@
     // See http://crbug.com/100242
     if (apiExists("webstorePrivate")) {
       chrome.webstorePrivate.beginInstallWithManifest2 =
           chrome.webstorePrivate.beginInstallWithManifest3;
     }
 
     if (apiExists("test"))
       chrome.test.getApiDefinitions = GetExtensionAPIDefinition;
   });
 })();