Allow omitting optional parameters for Extensions API functions

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 13 matching lines @@
   var internalAPIs = {};
   chromeHidden.internalAPIs = internalAPIs;
 
   function forEach(dict, f) {
     for (key in dict) {
       if (dict.hasOwnProperty(key))
         f(key, dict[key]);
     }
   }
 
+  // Returns a list of types that this schema allows.
+  chromeHidden.getAllTypesForSchema = function(schema) {

[Aaron Boodman, 2012/02/16 21:36:54]

Button a function on chromeHidden basically exports it to all the other
javascript files that make up the bindings layer in this context. It looks like
that isn't needed here, so you should just make these plain functions, like
forEach, above.

[Matt Tytel, 2012/02/24 01:29:27]

Done.

+    var validator = new chromeHidden.JSONSchemaValidator();
+    validator.addTypes(chromeHidden.validationTypes);
+    var schema_types = [];
+    if (schema.type)
+      schema_types.push(schema.type);
+    if (schema.choices) {
+      for (var i = 0; i < schema.choices.length; i++)
+        schema_types.push(schema.choices[i].type);
+    }
+    if (schema['$ref'])
+      schema_types.push(validator.types[schema['$ref']].type);

[Aaron Boodman, 2012/02/16 21:36:54]

What if it has choices?

[Matt Tytel, 2012/02/24 01:29:27]

Done. I don't think any have any choices or link to any other refs but might as
well check.

+    return schema_types;
+  };
+
+  // Returns true if |schema| would accept an argument of type |type|.
+  chromeHidden.isValidSchemaType = function(type, schema) {
+    schema_types = chromeHidden.getAllTypesForSchema(schema);
+    for (var i = 0; i < schema_types.length; i++) {
+      if (schema_types[i] == "any" || type == schema_types[i])
+        return true;
+    }
+    return type == "any";
+  };
+
+  // Returns true if there is a non-null value that both schemas would accept.
+  chromeHidden.checkSchemaOverlap = function(schema1, schema2) {
+    schema_types1 = chromeHidden.getAllTypesForSchema(schema1);
+    for (var i = 0; i < schema_types1.length; i++) {
+      if (chromeHidden.isValidSchemaType(schema_types1[i], schema2))
+        return true;
+    }
+    return false;
+  };
+
+  // Generate all possible signatures for a given API function schema.
+  chromeHidden.getSignatures = function(schemas) {
+    if (schemas.length === 0)
+      return [[]];
+
+    var signatures = [];
+    var remaining = chromeHidden.getSignatures(schemas.slice(1));
+    for (var i = 0; i < remaining.length; i++)
+      signatures.push([schemas[0]].concat(remaining[i]))
+
+    if (schemas[0].optional)
+      return signatures.concat(remaining);
+    return signatures;
+  };
+
+  // Return true if arguments match a given signature's schema.
+  chromeHidden.argumentsMatchSignature = function(args, schemas) {
+    if (args.length != schemas.length)
+      return false;
+
+    for (var i = 0; i < schemas.length; i++) {
+      if (!chromeHidden.isValidSchemaType(
+          chromeHidden.JSONSchemaValidator.getType(args[i]), schemas[i]))
+        return false;
+    }
+    return true;
+  };
+
+  // Finds the function signature for the given arguments.
+  chromeHidden.matchSignature = function(args, schemas) {
+    var signatures = chromeHidden.getSignatures(schemas);
+    for (var i = 0; i < signatures.length; i++) {
+      if (chromeHidden.argumentsMatchSignature(args, signatures[i]))
+        return signatures[i];
+    }
+    return null;
+  };
+
+  // Validates that a given schema for an API function is not ambiguous.
+  chromeHidden.isSchemaAmbiguous = function(schema) {

[Aaron Boodman, 2012/02/16 21:36:54]

The terms in use here are getting a bit overloaded and confusing.

A 'schema' is pretty ambiguous since it is hierarchical. It could represent an
entire API, just one function, a single parameter, etc.

How about terms like:

* definedSignature: the signature with optionals.
* resolution: the process to choose one of the candidates.
* candidateSignatures: the list of signatures without optionals.
* resolvedSignature: the result of resolution.

If you ever have to use the word schema, it might be best to qualify it, like
parameterSchema, or namespaceSchema, to make it more clear which one you're
talking about.

[Matt Tytel, 2012/02/24 01:29:27]

Done.

+    var signaturesAmbiguous = function(signature1, signature2) {
+      if (signature1.length != signature2.length)
+        return false;
+
+      for (var i = 0; i < signature1.length; i++) {
+        if (!chromeHidden.checkSchemaOverlap(signature1[i], signature2[i]))
+          return false;
+      }
+      return true;
+    };
+
+    var signatures = chromeHidden.getSignatures(schema);
+    for (var i = 0; i < signatures.length; i++) {
+      for (var j = i + 1; j < signatures.length; j++) {
+        if (signaturesAmbiguous(signatures[i], signatures[j]))
+          return true;
+      }
+    }
+    return false;
+  };
+
   // Validate arguments.
   chromeHidden.validationTypes = [];
   chromeHidden.validate = function(args, schemas) {
     if (args.length > schemas.length)
       throw new Error("Too many arguments.");
 
     for (var i = 0; i < schemas.length; i++) {
       if (i in args && args[i] !== null && args[i] !== undefined) {
         var validator = new chromeHidden.JSONSchemaValidator();
         validator.addTypes(chromeHidden.validationTypes);
@@ skipping 13 matching lines @@
         message = message.substring(0, message.length - 2);
         message += ".";
 
         throw new Error(message);
       } else if (!schemas[i].optional) {
         throw new Error("Parameter " + (i + 1) + " is required.");
       }
     }
   };
 
+  // Returns a string representing the defined signature of the API function.
+  // Example return value for chrome.windows.getCurrent:
+  // "windows.getCurrent(optional object populate, function callback)"
+  chromeHidden.getSchemaSignatureString = function(name, schemas) {
+    var getSchemaTypeString = function(schema) {
+      var schema_types = chromeHidden.getAllTypesForSchema(schema);
+      var type_name = schema_types.join(" or ") + " " + schema.name;
+      if (schema.optional)
+        return "optional " + type_name;
+      return type_name;
+    };
+
+    var type_names = schemas.map(getSchemaTypeString);
+    return name +" (" + type_names.join(", ") + ")";
+  };
+
+  // Returns a string representing a call to an API function.
+  // Example return value for call: chrome.windows.get(1, callback) is:
+  // "windows.get(int, function)"
+  chromeHidden.getArgumentSignatureString = function(name, args) {
+    var type_names = args.map(chromeHidden.JSONSchemaValidator.getType);
+    return name +" (" + type_names.join(", ") + ")";
+  };
+
+  // Finds the correct signature for the given arguments.

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

I think this comment (and function name) is not quite right. Something along the
lines of "padMissingOptionalArguments" or something -- the matching/validating
is just a sideeffect of that process.

[Matt Tytel, 2012/02/24 01:29:27]

Done.

+  chromeHidden.matchSignatureAndValidate = function(args, fun_def) {
+    var schemas = fun_def.definition.parameters;
+    var match = chromeHidden.matchSignature(args, schemas);
+    if (!match)
+      throw new Error("Invocation of form " +
+          chromeHidden.getArgumentSignatureString(fun_def.name, args) +
+          " doesn't match definition " +
+          chromeHidden.getSchemaSignatureString(fun_def.name, schemas));
+    chromeHidden.validate(args, match);
+
+    var normalized_args = [];
+    var ai = 0;
+    for (var si = 0; si < schemas.length; si++) {
+      if (schemas[si] === match[ai])
+        normalized_args.push(args[ai++]);
+      else
+        normalized_args.push(null);
+    }
+    return normalized_args;
+  };
+
   // Callback handling.
   var requests = [];
   chromeHidden.handleResponse = function(requestId, name,
                                          success, response, error) {
     try {
       var request = requests[requestId];
       if (success) {
         delete chrome.extension.lastError;
       } else {
         if (!error) {
@@ skipping 352 matching lines @@
 
           if (functionDef.name in module ||
               addUnprivilegedAccessGetter(module, functionDef.name,
                                           functionDef.unprivileged)) {
             return;
           }
 
           var apiFunction = {};
           apiFunction.definition = functionDef;
           apiFunction.name = apiDef.namespace + "." + functionDef.name;
+
+          // Validate API for ambiguity only in DEBUG mode.

[Aaron Boodman, 2012/02/16 21:36:54]

Can you add this here please:

TODO(aa): It would be best to run this in a unit test, but in order to do that
we would need to better factor this code so that it didn't depend on so much
v8::Extension machinery.

[Matt Tytel, 2012/02/24 01:29:27]

Done.

+          if (chromeHidden.validateAPI &&
+              chromeHidden.isSchemaAmbiguous(apiFunction.definition.parameters))
+            throw new Error(apiFunction.name + " is ambiguous");
           apiFunctions.register(apiFunction.name, apiFunction);
 
           module[functionDef.name] = (function() {
-            var args = arguments;
+            var args = Array.prototype.slice.call(arguments);
             if (this.updateArgumentsPreValidate)
               args = this.updateArgumentsPreValidate.apply(this, args);
-            chromeHidden.validate(args, this.definition.parameters);
+
+            args = chromeHidden.matchSignatureAndValidate(args, this);
             if (this.updateArgumentsPostValidate)
               args = this.updateArgumentsPostValidate.apply(this, args);
 
             var retval;
             if (this.handleRequest) {
               retval = this.handleRequest.apply(this, args);
             } else {
               retval = sendRequest(this.name, args,
                                    this.definition.parameters,
                                    {customCallback: this.customCallback});
@@ skipping 119 matching lines @@
     // See http://crbug.com/100242
     if (chrome.webstorePrivate) {
       chrome.webstorePrivate.beginInstallWithManifest2 =
           chrome.webstorePrivate.beginInstallWithManifest3;
     }
 
     if (chrome.test)
       chrome.test.getApiDefinitions = GetExtensionAPIDefinition;
   });
 })();