Several minor changes to API framework.

- Implement 'choices' JSON schema feature
- Made 'additionalProperties' false by default
- Made all API requests flow through one native function

Review URL: http://codereview.chromium.org/77020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@13988 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 148 insertions, 86 deletions

diff --git a/chrome/renderer/extensions/extension_api_client_unittest.cc b/chrome/renderer/extensions/extension_api_client_unittest.cc
index 1c3530f..87e4376 100755
--- a/chrome/renderer/extensions/extension_api_client_unittest.cc
+++ b/chrome/renderer/extensions/extension_api_client_unittest.cc
@@ -57,6 +57,8 @@ TEST_F(ExtensionAPIClientTest, CallbackDispatching) {
     "chromium.tabs.createTab({}, callback);"
   );
 
+  EXPECT_EQ("", GetConsoleMessage());
+
   // Ok, we should have gotten a message to create a tab, grab the callback ID.
   const IPC::Message* request_msg =
       render_thread_.sink().GetUniqueMessageMatching(
@@ -96,7 +98,7 @@ TEST_F(ExtensionAPIClientTest, GetTabsForWindow) {
 
 TEST_F(ExtensionAPIClientTest, GetTab) {
   ExpectJsFail("chromium.tabs.getTab(null, function(){});",
-               "Uncaught Error: Argument 0 is required.");
+               "Uncaught Error: Parameter 0 is required.");
 
   ExecuteJavaScript("chromium.tabs.getTab(42)");
     const IPC::Message* request_msg =
@@ -116,9 +118,9 @@ TEST_F(ExtensionAPIClientTest, CreateTab) {
   ExpectJsFail("chromium.tabs.createTab({url: 42}, function(){});",
                "Uncaught Error: Invalid value for argument 0. Property "
                "'url': Expected 'string' but got 'integer'.");
-  ExpectJsFail("chromium.tabs.createTab({selected: null}, function(){});",
+  ExpectJsFail("chromium.tabs.createTab({foo: 42}, function(){});",
                "Uncaught Error: Invalid value for argument 0. Property "
-               "'selected': Expected 'boolean' but got 'null'.");
+               "'foo': Unexpected property.");
 
   ExecuteJavaScript("chromium.tabs.createTab({"
                     "  url:'http://www.google.com/',"
@@ -140,16 +142,13 @@ TEST_F(ExtensionAPIClientTest, CreateTab) {
 TEST_F(ExtensionAPIClientTest, UpdateTab) {
   ExpectJsFail("chromium.tabs.updateTab({id: null});",
                "Uncaught Error: Invalid value for argument 0. Property "
-               "'id': Expected 'integer' but got 'null'.");
+               "'id': Property is required.");
   ExpectJsFail("chromium.tabs.updateTab({id: 42, windowId: 'foo'});",
                "Uncaught Error: Invalid value for argument 0. Property "
                "'windowId': Expected 'integer' but got 'string'.");
   ExpectJsFail("chromium.tabs.updateTab({id: 42, url: 42});",
                "Uncaught Error: Invalid value for argument 0. Property "
                "'url': Expected 'string' but got 'integer'.");
-  ExpectJsFail("chromium.tabs.updateTab({id: 42, selected: null});",
-               "Uncaught Error: Invalid value for argument 0. Property "
-               "'selected': Expected 'boolean' but got 'null'.");
 
   ExecuteJavaScript("chromium.tabs.updateTab({"
                     "  id:42,"
diff --git a/chrome/renderer/extensions/extension_process_bindings.cc b/chrome/renderer/extensions/extension_process_bindings.cc
index 5952463..fcf9710 100644
--- a/chrome/renderer/extensions/extension_process_bindings.cc
+++ b/chrome/renderer/extensions/extension_process_bindings.cc
@@ -107,7 +107,7 @@ void ExtensionProcessBindings::SetFunctionNames(
 
 void ExtensionProcessBindings::ExecuteCallbackInFrame(
     WebFrame* frame, int callback_id, const std::string& response) {
-  std::string code = "chromium._dispatchCallback(";
+  std::string code = "chromium.dispatchCallback_(";
   code += IntToString(callback_id);
   code += ", '";
 
diff --git a/chrome/renderer/renderer_resources.grd b/chrome/renderer/renderer_resources.grd
index cda253b..c59e98c 100755
--- a/chrome/renderer/renderer_resources.grd
+++ b/chrome/renderer/renderer_resources.grd
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!-- This comment is only here because changes to resources are not picked up
-without changes to the corresponding grd file. -->
+without changes to the corresponding grd file.  -->
 <grit latest_public_release="0" current_release="1">
   <outputs>
     <output filename="grit/renderer_resources.h" type="rc_header">
diff --git a/chrome/renderer/resources/extension_process_bindings.js b/chrome/renderer/resources/extension_process_bindings.js
index cae9212..54ed9d7 100644
--- a/chrome/renderer/resources/extension_process_bindings.js
+++ b/chrome/renderer/resources/extension_process_bindings.js
@@ -1,5 +1,5 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.

-// Use of this source code is governed by a BSD-style license that can be

+// Copyright (c) 2009 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.
 
 // -----------------------------------------------------------------------------
@@ -17,19 +17,25 @@ var chromium;
   native function UpdateTab();
   native function MoveTab();
   native function RemoveTab();
+  native function GetBookmarks();
+  native function SearchBookmarks();
+  native function RemoveBookmark();
+  native function CreateBookmark();
+  native function MoveBookmark();
+  native function SetBookmarkTitle();
 
   if (!chromium)
     chromium = {};
 
   // Validate arguments.
-  function validate(inst, schemas) {
-    if (inst.length > schemas.length)
+  function validate(args, schemas) {
+    if (args.length > schemas.length)
       throw new Error("Too many arguments.");
 
     for (var i = 0; i < schemas.length; i++) {
-      if (inst[i]) {
+      if (i in args && args[i] !== null && args[i] !== undefined) {
         var validator = new chromium.JSONSchemaValidator();
-        validator.validate(inst[i], schemas[i]);
+        validator.validate(args[i], schemas[i]);
         if (validator.errors.length == 0)
           continue;
         
@@ -47,14 +53,16 @@ var chromium;
 
         throw new Error(message);
       } else if (!schemas[i].optional) {
-        throw new Error("Argument " + i + " is required.");
+        throw new Error("Parameter " + i + " is required.");
       }
     }
   }
 
   // callback handling
+  // TODO(aa): This function should not be publicly exposed. Pass it into V8
+  // instead and hold one per-context. See the way event_bindings.js works.
   var callbacks = [];
-  chromium._dispatchCallback = function(callbackId, str) {
+  chromium.dispatchCallback_ = function(callbackId, str) {
     try {
       if (str) {
         callbacks[callbackId](goog.json.parse(str));
@@ -97,8 +105,7 @@ var chromium;
           minItems: 1
         }
       },
-      optional: true,
-      additionalProperties: false
+      optional: true
     },
     chromium.types.optFun
   ];
@@ -135,8 +142,7 @@ var chromium;
         windowId: chromium.types.optPInt,
         url: chromium.types.optStr,
         selected: chromium.types.optBool
-      },
-      additionalProperties: false
+      }
     },
     chromium.types.optFun
   ];
@@ -154,8 +160,7 @@ var chromium;
         windowId: chromium.types.optPInt,
         url: chromium.types.optStr,
         selected: chromium.types.optBool
-      },
-      additionalProperties: false
+      }
     }
   ];
 
@@ -171,8 +176,7 @@ var chromium;
         id: chromium.types.pInt,
         windowId: chromium.types.optPInt,
         index: chromium.types.pInt
-      },
-      additionalProperties: false
+      }
     }
   ];
   
@@ -192,10 +196,10 @@ var chromium;
   //----------------------------------------------------------------------------
 
   // Bookmarks
+  // TODO(erikkay): Call validate() in these functions.
   chromium.bookmarks = {};
 
   chromium.bookmarks.get = function(ids, callback) {
-    native function GetBookmarks();
     sendRequest(GetBookmarks, ids, callback);
   };
 
@@ -206,14 +210,12 @@ var chromium;
         type: chromium.types.pInt
       },
       minItems: 1,
-      optional: true,
-      additionalProperties: false
+      optional: true
     },
     chromium.types.optFun
   ];
 
   chromium.bookmarks.search = function(query, callback) {
-    native function SearchBookmarks();
     sendRequest(SearchBookmarks, query, callback);
   };
 
@@ -223,7 +225,6 @@ var chromium;
   ];
 
   chromium.bookmarks.remove = function(bookmark, callback) {
-    native function RemoveBookmark();
     sendRequest(RemoveBookmark, bookmark, callback);
   };
 
@@ -233,14 +234,12 @@ var chromium;
       properties: {
         id: chromium.types.pInt,
         recursive: chromium.types.bool
-      },
-      additionalProperties: false
+      }
     },
     chromium.types.optFun
   ];
 
   chromium.bookmarks.create = function(bookmark, callback) {
-    native function CreateBookmark();
     sendRequest(CreateBookmark, bookmark, callback);
   };
 
@@ -252,14 +251,12 @@ var chromium;
         index: chromium.types.optPInt,
         title: chromium.types.optString,
         url: chromium.types.optString,
-      },
-      additionalProperties: false
+      }
     },
     chromium.types.optFun
   ];
 
   chromium.bookmarks.move = function(obj, callback) {
-    native function MoveBookmark();
     sendRequest(MoveBookmark, obj, callback);
   };
 
@@ -270,14 +267,12 @@ var chromium;
         id: chromium.types.pInt,
         parentId: chromium.types.optPInt,
         index: chromium.types.optPInt
-      },
-      additionalProperties: false
+      }
     },
     chromium.types.optFun
   ];
 
   chromium.bookmarks.setTitle = function(bookmark, callback) {
-    native function SetBookmarkTitle();
     sendRequest(SetBookmarkTitle, bookmark, callback);
   };
 
@@ -287,8 +282,7 @@ var chromium;
       properties: {
         id: chromium.types.pInt,
         title: chromium.types.optString
-      },
-      additionalProperties: false
+      }
     },
     chromium.types.optFun
   ];
diff --git a/chrome/renderer/resources/json_schema.js b/chrome/renderer/resources/json_schema.js
index 3e187b1..2c75e4d 100755
--- a/chrome/renderer/resources/json_schema.js
+++ b/chrome/renderer/resources/json_schema.js
@@ -15,7 +15,7 @@
 // - requires
 // - unique
 // - disallow
-// - union types
+// - union types (but replaced with 'choices')
 //
 // The following properties are not applicable to the interface exposed by
 // this class:
@@ -27,6 +27,13 @@
 // - default
 // - transient
 // - hidden
+//
+// There are also these departures from the JSON Schema proposal:
+// - function and undefined types are supported
+// - null counts as 'unspecified' for optional values
+// - added the 'choices' property, to allow specifying a list of possible types
+//   for a value
+// - made additionalProperties default to false
 //==============================================================================
 
 var chromium = chromium || {};
@@ -63,7 +70,8 @@ chromium.JSONSchemaValidator.messages = {
   numberMinValue: "Value must not be less than *.",
   numberMaxValue: "Value must not be greater than *.",
   numberMaxDecimal: "Value must not have more than * decimal places.",
-  invalidType: "Expected '*' but got '*'."
+  invalidType: "Expected '*' but got '*'.",
+  invalidChoice: "Value does not match any valid type choices."
 };
 
 /**
@@ -118,6 +126,13 @@ chromium.JSONSchemaValidator.prototype.validate = function(instance, schema,
   if (schema.extends)
     this.validate(instance, schema.extends, path);
 
+  // If the schema has a choices property, the instance must validate against at
+  // least one of the items in that array.
+  if (schema.choices) {
+    this.validateChoices(instance, schema, path);
+    return;
+  }
+
   // If the schema has an enum property, the instance must be one of those
   // values.
   if (schema.enum) {
@@ -149,6 +164,28 @@ chromium.JSONSchemaValidator.prototype.validate = function(instance, schema,
 };
 
 /**
+ * Validates an instance against a choices schema. The instance must match at
+ * least one of the provided choices.
+ */
+chromium.JSONSchemaValidator.prototype.validateChoices = function(instance,
+                                                                  schema,
+                                                                  path) {
+  var originalErrors = this.errors;
+
+  for (var i = 0; i < schema.choices.length; i++) {
+    this.errors = [];
+    this.validate(instance, schema.choices[i], path);
+    if (this.errors.length == 0) {
+      this.errors = originalErrors;
+      return;
+    }
+  }
+
+  this.errors = originalErrors;
+  this.addError(path, "invalidChoice");
+};
+
+/**
  * Validates an instance against a schema with an enum type. Populates the
  * |errors| property, and returns a boolean indicating whether the instance
  * validates.
@@ -172,28 +209,26 @@ chromium.JSONSchemaValidator.prototype.validateObject = function(instance,
                                                                  schema, path) {
   for (var prop in schema.properties) {
     var propPath = path ? path + "." + prop : prop;
-    if (instance.hasOwnProperty(prop)) {
+    if (prop in instance && instance[prop] !== null &&
+        instance[prop] !== undefined) {
       this.validate(instance[prop], schema.properties[prop], propPath);
     } else if (!schema.properties[prop].optional) {
       this.addError(propPath, "propertyRequired");
     }
   }
 
-  // The additionalProperties property can either be |false| or a schema
-  // definition. If |false|, additional properties are not allowed. If a schema
-  // defintion, all additional properties must validate against that schema.
-  if (typeof schema.additionalProperties != "undefined") {
-    for (var prop in instance) {
-      if (instance.hasOwnProperty(prop)) {
-        var propPath = path ? path + "." + prop : prop;
-        if (!schema.properties.hasOwnProperty(prop)) {
-          if (schema.additionalProperties === false)
-            this.addError(propPath, "unexpectedProperty");
-          else
-            this.validate(instance[prop], schema.additionalProperties, propPath);
-        }
-      }
-    }
+  // By default, additional properties are not allowed on instance objects. This
+  // can be overridden by setting the additionalProperties property to a schema
+  // which any additional properties must validate against.
+  for (var prop in instance) {
+    if (prop in schema.properties)
+      continue;
+
+    var propPath = path ? path + "." + prop : prop;
+    if (schema.additionalProperties)
+      this.validate(instance[prop], schema.additionalProperties, propPath);
+    else
+      this.addError(propPath, "unexpectedProperty");
   }
 };
 
@@ -218,29 +253,29 @@ chromium.JSONSchemaValidator.prototype.validateArray = function(instance,
     // If the items property is a single schema, each item in the array must
     // have that schema.
     for (var i = 0; i < instance.length; i++) {
-      this.validate(instance[i], schema.items, path + "[" + i + "]");
+      this.validate(instance[i], schema.items, path + "." + i);
     }
   } else if (typeOfItems == 'array') {
     // If the items property is an array of schemas, each item in the array must
     // validate against the corresponding schema.
     for (var i = 0; i < schema.items.length; i++) {
-      var itemPath = path ? path + "[" + i + "]" : String(i);
-      if (instance.hasOwnProperty(i)) {
+      var itemPath = path ? path + "." + i : String(i);
+      if (i in instance && instance[i] !== null && instance[i] !== undefined) {
         this.validate(instance[i], schema.items[i], itemPath);
       } else if (!schema.items[i].optional) {
         this.addError(itemPath, "itemRequired");
       }
     }
 
-    if (schema.additionalProperties === false) {
-      if (instance.length > schema.items.length) {
-        this.addError(path, "arrayMaxItems", [schema.items.length]);
-      }
-    } else if (schema.additionalProperties) {
+    if (schema.additionalProperties) {
       for (var i = schema.items.length; i < instance.length; i++) {
-        var itemPath = path ? path + "[" + i + "]" : String(i);
+        var itemPath = path ? path + "." + i : String(i);
         this.validate(instance[i], schema.additionalProperties, itemPath);
       }
+    } else {
+      if (instance.length > schema.items.length) {
+        this.addError(path, "arrayMaxItems", [schema.items.length]);
+      }
     }
   }
 };
@@ -328,6 +363,14 @@ chromium.JSONSchemaValidator.prototype.addError = function(path, key,
   types.optStr = extend(types.str, types.opt);
   types.optFun = extend(types.fun, types.opt);
   types.optPInt = extend(types.pInt, types.opt);
+  types.singleOrListOf = function(type) {
+    return {
+      choice: [
+        type,
+        { type: "array",  item: type }
+      ]
+    };
+  };
 
   chromium.types = types;
 })();
diff --git a/chrome/test/data/extensions/json_schema_test.js b/chrome/test/data/extensions/json_schema_test.js
index 497c344..caf494a 100755
--- a/chrome/test/data/extensions/json_schema_test.js
+++ b/chrome/test/data/extensions/json_schema_test.js
@@ -11,8 +11,8 @@ function assertValid(type, instance, schema) {
   var validator = new chromium.JSONSchemaValidator();
   validator["validate" + type](instance, schema, "");
   if (validator.errors.length != 0) {
-    console.log("Got unexpected errors");
-    console.log(validator.errors);
+    log("Got unexpected errors");
+    log(validator.errors);
     assert(false);
   }
 }
@@ -124,6 +124,25 @@ function testEnum() {
                                                 [schema.enum.join(", ")])]);
 }
 
+function testChoices() {
+  var schema = {
+    choices: [
+      { type: "null" },
+      { type: "undefined" },
+      { type: "integer", minimum:42, maximum:42 },
+      { type: "object", properties: { foo: { type: "string" } } }
+    ]
+  }
+    assertValid("", null, schema);
+    assertValid("", undefined, schema);
+    assertValid("", 42, schema);
+    assertValid("", {foo: "bar"}, schema);
+
+    assertNotValid("", "foo", schema, [formatError("invalidChoice", [])]);
+    assertNotValid("", [], schema, [formatError("invalidChoice", [])]);
+    assertNotValid("", {foo: 42}, schema, [formatError("invalidChoice", [])]);
+}
+
 function testExtends() {
   var parent = {
     type: "number"
@@ -155,24 +174,27 @@ function testObject() {
   };
 
   assertValid("Object", {foo:"foo",bar:42}, schema);
-  assertValid("Object", {foo:"foo",bar:42,"extra":true}, schema);
+  assertNotValid("Object", {foo:"foo",bar:42,"extra":true}, schema,
+                 [formatError("unexpectedProperty")]);
   assertNotValid("Object", {foo:"foo"}, schema,
                  [formatError("propertyRequired")]);
   assertNotValid("Object", {foo:"foo", bar:"42"}, schema,
                  [formatError("invalidType", ["integer", "string"])]);
 
-  schema.additionalProperties = false;
-  assertNotValid("Object", {foo:"foo",bar:42,"extra":true}, schema,
-                 [formatError("unexpectedProperty")]);
+  schema.additionalProperties = { type: "any" };
+  assertValid("Object", {foo:"foo",bar:42,"extra":true}, schema);
+  assertValid("Object", {foo:"foo",bar:42,"extra":"foo"}, schema);
 
-  schema.additionalProperties = {
-    type: "boolean"
-  };
+  schema.additionalProperties = { type: "boolean" };
   assertValid("Object", {foo:"foo",bar:42,"extra":true}, schema);
+  assertNotValid("Object", {foo:"foo",bar:42,"extra":"foo"}, schema,
+                 [formatError("invalidType", ["boolean", "string"])]);
 
   schema.properties.bar.optional = true;
   assertValid("Object", {foo:"foo",bar:42}, schema);
   assertValid("Object", {foo:"foo"}, schema);
+  assertValid("Object", {foo:"foo",bar:null}, schema);
+  assertValid("Object", {foo:"foo",bar:undefined}, schema);
   assertNotValid("Object", {foo:"foo", bar:"42"}, schema,
                  [formatError("invalidType", ["integer", "string"])]);
 }
@@ -190,18 +212,17 @@ function testArrayTuple() {
   };
 
   assertValid("Array", ["42", 42], schema);
-  assertValid("Array", ["42", 42, "anything"], schema);
+  assertNotValid("Array", ["42", 42, "anything"], schema,
+                 [formatError("arrayMaxItems", [schema.items.length])]);
   assertNotValid("Array", ["42"], schema, [formatError("itemRequired")]);
   assertNotValid("Array", [42, 42], schema,
                  [formatError("invalidType", ["string", "integer"])]);
 
-  schema.additionalProperties = false;
-  assertNotValid("Array", ["42", 42, "anything"], schema,
-                 [formatError("arrayMaxItems", [schema.items.length])]);
+  schema.additionalProperties = { type: "any" };
+  assertValid("Array", ["42", 42, "anything"], schema);
+  assertValid("Array", ["42", 42, []], schema);
 
-  schema.additionalProperties = {
-    type: "boolean"
-  };
+  schema.additionalProperties = { type: "boolean" };
   assertNotValid("Array", ["42", 42, "anything"], schema,
                  [formatError("invalidType", ["boolean", "string"])]);
   assertValid("Array", ["42", 42, false], schema);
@@ -209,6 +230,8 @@ function testArrayTuple() {
   schema.items[0].optional = true;
   assertValid("Array", ["42", 42], schema);
   assertValid("Array", [, 42], schema);
+  assertValid("Array", [null, 42], schema);
+  assertValid("Array", [undefined, 42], schema);
   assertNotValid("Array", [42, 42], schema,
                  [formatError("invalidType", ["string", "integer"])]);
 }
@@ -289,6 +312,7 @@ function testType() {
   assertValid("Type", true, {type:"boolean"});
   assertValid("Type", false, {type:"boolean"});
   assertValid("Type", null, {type:"null"});
+  assertValid("Type", undefined, {type:"undefined"});
 
   // not valid
   assertNotValid("Type", [], {type: "object"},
@@ -309,6 +333,8 @@ function testType() {
                  [formatError("invalidType", ["boolean", "integer"])]);
   assertNotValid("Type", false, {type: "null"},
                  [formatError("invalidType", ["null", "boolean"])]);
+  assertNotValid("Type", undefined, {type: "null"},
+                 [formatError("invalidType", ["null", "undefined"])]);
   assertNotValid("Type", {}, {type: "function"},
                  [formatError("invalidType", ["function", "object"])]);
 }
diff --git a/chrome/test/data/js_test_runner.html b/chrome/test/data/js_test_runner.html
index 55d0448..16364dc 100755
--- a/chrome/test/data/js_test_runner.html
+++ b/chrome/test/data/js_test_runner.html
@@ -13,8 +13,8 @@ See: chrome/test/data/extensions/schema_test.js for an example.
 </textarea>

 

 <!-- Add a reference to the script and the script test files here. -->

-<script src="../../../renderer/resources/schema.js"></script>

-<script src="schema_test.js"></script>

+<script src="../../renderer/resources/json_schema.js"></script>

+<script src="extensions/json_schema_test.js"></script>

 

 <script>

 function log() {