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 16 matching lines @@
     return !!resolved;
   }
 
   function forEach(dict, f) {
     for (key in dict) {
       if (dict.hasOwnProperty(key))
         f(key, dict[key]);
     }
   }
 
+  // Returns the defined signature of the API function.
+  chromeHidden.fullSignature = function(name, schemas) {

[Aaron Boodman, 2012/02/06 01:30:52]

Function names should be a verb phrase, like getFullSignature(). Also, reading
the name and comments for this, I'm not sure what it does. Can you clarify the
naming?

+    var getTypeName = function(schema) {
+      var typeName = schema.type + " " + schema.name;
+      if (schema.optional)
+        return "optional " + typeName;
+      return typeName;
+    };
+
+    var typeNames = schemas.map(getTypeName);
+    return name +"(" + typeNames.join(", ") + ")";
+  };
+
+  // Return true if arguments match a given candidate's schema.
+  chromeHidden.matchCandidate = function(args, schemas) {

[Aaron Boodman, 2012/02/06 01:30:52]

argumentsMatchSignature()?

+    if (args.length != schemas.length)
+      return false;
+
+    var validator = new chromeHidden.JSONSchemaValidator();
+    validator.addTypes(chromeHidden.validationTypes);
+    for (var i = 0; i < schemas.length; i++) {
+      if (args[i] !== null || !schemas[i].optional)
+        validator.validate(args[i], schemas[i]);

[Aaron Boodman, 2012/02/06 01:30:52]

This does not do the right thing. It will check all the details of schemas[i]
and come up with wrong answers for the edge cases we talked about in the thread.

We only want to consider the coarse type here, like string/int/boolean/etc. You
can use the JSONSchemaValidator.getSchemaType() method.

+    }
+    return validator.errors.length == 0;
+  };
+
+  // Generate candidates for a given API function scehma.
+  chromeHidden.getCandidates = function(schemas) {

[Aaron Boodman, 2012/02/06 01:30:52]

Can we call this something other than 'candidate'. Maybe
getOverloadedSignatures() or something?

+    if (schemas.length === 0)
+      return [[]];
+
+    var candidates = [];
+    var remaining = chromeHidden.getCandidates(schemas.slice(1));
+    for (var i = 0; i < remaining.length; i++) {
+      var candidate = remaining[i].slice(0);
+      candidate.unshift(schemas[0]);
+      candidates.push(candidate);
+    }
+    if (schemas[0].optional)
+      return candidates.concat(remaining);
+    return candidates;
+  };
+
   // Validate arguments.
   chromeHidden.validationTypes = [];
   chromeHidden.validate = function(args, schemas) {
-    if (args.length > schemas.length)
-      throw new Error("Too many arguments.");
+    var candidates = chromeHidden.getCandidates(schemas);
+    var match = null;
+    for (var i = 0; i < candidates.length && match === null; i++) {
+      if (chromeHidden.matchCandidate(args, candidates[i]))
+        match = candidates[i];
+    }
+    if (match === null)
+      throw new Error("Invocation does not match definition.");
 
-    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);
-        validator.validate(args[i], schemas[i]);
-        if (validator.errors.length == 0)
-          continue;
+    // Insert nulls into omitted optional parameters.
+    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;

[Aaron Boodman, 2012/02/06 01:30:52]

Hm. This doesn't seem right. We never throw the validation errors.

+  };
 
-        var message = "Invalid value for argument " + (i + 1) + ". ";
-        for (var i = 0, err; err = validator.errors[i]; i++) {
-          if (err.path) {
-            message += "Property '" + err.path + "': ";
-          }
-          message += err.message;
-          message = message.substring(0, message.length - 1);
-          message += ", ";
-        }
-        message = message.substring(0, message.length - 2);
-        message += ".";
+  // Returns true if there is a non-null value that both would accept.
+  chromeHidden.schemaOverlap = function(schema1, schema2) {
+    if (schema1.choices) {
+      for (var i = 0; i < schema1.choices.length; i++)
+        if (chromeHidden.schemaOverlap(schema1.choices[i], schema2))
+          return true;
+    } else if (schema2.choices) {
+      for (var i = 0; i < schema2.choices.length; i++)
+        if (chromeHidden.schemaOverlap(schema1, schema2.choices[i]))
+          return true;
+    } else {
+      var type1 = schema1.type ||
+                  chromeHidden.validationTypes[schema1['$ref']].type;
+      var type2 = schema2.type ||
+                  chromeHidden.validationTypes[schema2['$ref']].type;
 
-        throw new Error(message);
-      } else if (!schemas[i].optional) {
-        throw new Error("Parameter " + (i + 1) + " is required.");
+      if (type1 == "any" || type2 == "any")
+        return true;
+      return type1 == type2;
+    }
+    return false;
+  };
+
+  // Validate schema for API function.
+  chromeHidden.schemaAmbiguous = function(schema) {
+    var candidatesAmbiguous = function(candidate1, candidate2) {
+      if (candidate1.length != candidate2.length)
+        return false;
+
+      for (var i = 0; i < candidate1.length; i++) {
+        if (!chromeHidden.schemaOverlap(candidate1[i], candidate2[i]))
+          return false;
+      }
+      return true;
+    };
+
+    candidates = chromeHidden.getCandidates(schema);
+    for (var i = 0; i < candidates.length; i++) {
+      for (var j = i + 1; j < candidates.length; j++) {
+        if (candidatesAmbiguous(candidates[i], candidates[j]))
+          return true;
       }
     }
+    return false;
   };
 
   // Callback handling.
   var requests = [];
   chromeHidden.handleResponse = function(requestId, name,
                                          success, response, error) {
     try {
       var request = requests[requestId];
       if (success) {
         delete chrome.extension.lastError;
@@ 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;
+          if (chromeHidden.validateAPI &&
+              chromeHidden.schemaAmbiguous(apiFunction.definition.parameters))
+            throw new Error(apiFunction.name + " is ambiguous");
+
           apiFunctions.register(apiFunction.name, apiFunction);
 
           module[functionDef.name] = (function() {
             var args = arguments;
             if (this.updateArgumentsPreValidate)
               args = this.updateArgumentsPreValidate.apply(this, args);
-            chromeHidden.validate(args, this.definition.parameters);
+            try {

[Aaron Boodman, 2012/02/06 01:30:52]

I think it would be a bit cleaner to just have validate() return true/false to
indicate whether it was able to find a signature.

Or alternately, separate out finding the signature from validate() entirely. So
this code would look like:

var matchedSignature =
chromeHidden.getFunctionSignature(this.definition.parameters, args);
if (!matchedSignature)
  throw new Error("Invocation doesn't match...");

chromeHidden.validate(args, matchedSignature);

+              args = chromeHidden.validate(args, this.definition.parameters);
+            } catch (err) {
+              throw new Error("Invocation doesn't match definition " +
+                  chromeHidden.fullSignature(this.name,

[Aaron Boodman, 2012/02/06 01:30:52]

Can we also include the signature that was actually called in the error message?

+                                             this.definition.parameters));
+            }
             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 117 matching lines @@
     // See http://crbug.com/100242
     if (apiExists("webstorePrivate")) {
       chrome.webstorePrivate.beginInstallWithManifest2 =
           chrome.webstorePrivate.beginInstallWithManifest3;
     }
 
     if (apiExists("test"))
       chrome.test.getApiDefinitions = GetExtensionAPIDefinition;
   });
 })();