Allow omitting optional parameters for Extensions API functions

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 d9a4d735b647443976524cc925c0c76bf1041625..15c7b31815283b2765bebc36fb91decaa7598ef5 100644
--- a/chrome/renderer/resources/extensions/schema_generated_bindings.js
+++ b/chrome/renderer/resources/extensions/schema_generated_bindings.js
@@ -31,22 +31,84 @@ var chrome = chrome || {};
     }
   }
 
+  // Generate all possible signatures for a given API function schema.
+  function getSignatures(param_list) {

[Matt Perry, 2012/02/24 02:00:12]

javascript style for variable names is paramList. Same goes for the rest of the
variables in this file.

[Matt Tytel, 2012/02/24 02:29:36]

Done.

+    if (param_list.length === 0)
+      return [[]];
+
+    var signatures = [];
+    var remaining = getSignatures(param_list.slice(1));
+    for (var i = 0; i < remaining.length; i++)
+      signatures.push([param_list[0]].concat(remaining[i]))
+
+    if (param_list[0].optional)
+      return signatures.concat(remaining);
+    return signatures;
+  };
+
+  // Return true if arguments match a given signature's schema.
+  function argumentsMatchSignature(args, param_list) {
+    if (args.length != param_list.length)
+      return false;
+
+    for (var i = 0; i < param_list.length; i++) {
+      if (!chromeHidden.schema_validator.isValidSchemaType(
+          chromeHidden.JSONSchemaValidator.getType(args[i]), param_list[i]))
+        return false;
+    }
+    return true;
+  };
+
+  // Finds the function signature for the given arguments.
+  function resolveSignature(args, param_list) {
+    var candidate_signatures = getSignatures(param_list);
+    for (var i = 0; i < candidate_signatures.length; i++) {
+      if (argumentsMatchSignature(args, candidate_signatures[i]))
+        return candidate_signatures[i];
+    }
+    return null;
+  };
+
+  // Validates that a given schema for an API function is not ambiguous.
+  function isParameterSchemaAmbiguous(parameter_schema) {
+    var signaturesAmbiguous = function(signature1, signature2) {
+      if (signature1.length != signature2.length)
+        return false;
+
+      for (var i = 0; i < signature1.length; i++) {
+        if (!chromeHidden.schema_validator.checkSchemaOverlap(signature1[i],
+            signature2[i]))
+          return false;
+      }
+      return true;
+    };
+
+    var signatures = getSignatures(parameter_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)
+  chromeHidden.schema_validator = new chromeHidden.JSONSchemaValidator();
+  chromeHidden.validate = function(args, param_list) {
+    if (args.length > param_list.length)
       throw new Error("Too many arguments.");
 
-    for (var i = 0; i < schemas.length; i++) {
+    for (var i = 0; i < param_list.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)
+        chromeHidden.schema_validator.resetErrors();
+        chromeHidden.schema_validator.validate(args[i], param_list[i]);
+        if (chromeHidden.schema_validator.errors.length == 0)
           continue;
 
         var message = "Invalid value for argument " + (i + 1) + ". ";
-        for (var i = 0, err; err = validator.errors[i]; i++) {
+        for (var i = 0; i < chromeHidden.schema_validator.length; i++) {
+          var err = chromeHidden.schema_validator.errors[i];
           if (err.path) {
             message += "Property '" + err.path + "': ";
           }
@@ -58,12 +120,61 @@ var chrome = chrome || {};
         message += ".";
 
         throw new Error(message);
-      } else if (!schemas[i].optional) {
+      } else if (!param_list[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)"
+  function getParameterSignatureString(name, param_list) {
+    var getSchemaTypeString = function(schema) {
+      var schema_types =
+          chromeHidden.schema_validator.getAllTypesForSchema(schema);
+      var type_name = schema_types.join(" or ") + " " + schema.name;
+      if (schema.optional)
+        return "optional " + type_name;
+      return type_name;
+    };
+
+    var type_names = param_list.map(getSchemaTypeString);
+    return name +" (" + type_names.join(", ") + ")";

[Matt Perry, 2012/02/24 02:00:12]

space before " ("

[Matt Tytel, 2012/02/24 02:29:36]

Done. I misinterpreted your last comment on this and put the space right before
the ( instead of the ". I wondered why you wanted a space there.

+  };
+
+  // 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)"
+  function getArgumentSignatureString(name, args) {
+    var type_names = args.map(chromeHidden.JSONSchemaValidator.getType);
+    return name +" (" + type_names.join(", ") + ")";

[Matt Perry, 2012/02/24 02:00:12]

ditto

[Matt Tytel, 2012/02/24 02:29:36]

Done.

+  };
+
+  // Finds the correct signature for the given arguments, then validates the
+  // arguments against that signature. Returns a 'normalized' arguments list
+  // where nulls are inserted where optional parameters were omitted.
+  chromeHidden.updateArgumentsValidate = function(args, fun_def) {
+    var param_list = fun_def.definition.parameters;
+    var resolved_signature = resolveSignature(args, param_list);
+    if (!resolved_signature)
+      throw new Error("Invocation of form " +
+          getArgumentSignatureString(fun_def.name, args) +
+          " doesn't match definition " +
+          getParameterSignatureString(fun_def.name, param_list));
+    chromeHidden.validate(args, resolved_signature);
+
+    var normalized_args = [];
+    var ai = 0;
+    for (var si = 0; si < param_list.length; si++) {
+      if (param_list[si] === resolved_signature[ai])
+        normalized_args.push(args[ai++]);
+      else
+        normalized_args.push(null);
+    }
+    return normalized_args;
+  };
+
   // Callback handling.
   var requests = [];
   chromeHidden.handleResponse = function(requestId, name,
@@ -391,13 +502,13 @@ var chrome = chrome || {};
         mod = mod[name];
       }
 
-      // Add types to global validationTypes
+      // Add types to global schema_validator
       if (apiDef.types) {
         apiDef.types.forEach(function(t) {
           if (!isSchemaNodeSupported(t, platform, manifestVersion))
             return;
 
-          chromeHidden.validationTypes.push(t);
+          chromeHidden.schema_validator.addTypes(t);
           if (t.type == 'object' && customTypes[t.id]) {
             customTypes[t.id].prototype.setSchema(t);
           }
@@ -440,13 +551,22 @@ var chrome = chrome || {};
           var apiFunction = {};
           apiFunction.definition = functionDef;
           apiFunction.name = apiDef.namespace + "." + functionDef.name;
-          apiFunctions.register(apiFunction.name, apiFunction);
 
+          // Validate API for ambiguity only in DEBUG mode.
+          // 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.
+          if (chromeHidden.validateAPI &&
+              isParameterSchemaAmbiguous(apiFunction.definition.parameters))
+            throw new Error(apiFunction.name + " is ambiguous");
+
+          apiFunctions.register(apiFunction.name, apiFunction);
           mod[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.updateArgumentsValidate(args, this);
             if (this.updateArgumentsPostValidate)
               args = this.updateArgumentsPostValidate.apply(this, args);