Allow "null" to mean optional in extension APIs (for realz), but normalise it

for browser implementations by stripping it from objects.

The following are all now exactly the same, where previously they weren't and
would either not work or crash the browser/renderer:

chrome.tabs.create({});
chrome.tabs.create({windowId: undefined});
chrome.tabs.create({windowId: null});

BUG=129411
TEST=as in bug

Review URL: https://chromiumcodereview.appspot.com/10456010

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

2 files changed, 19 insertions, 4 deletions

diff --git a/chrome/renderer/extensions/send_request_natives.cc b/chrome/renderer/extensions/send_request_natives.cc
index 5bf48e5..d17fc0a 100644
--- a/chrome/renderer/extensions/send_request_natives.cc
+++ b/chrome/renderer/extensions/send_request_natives.cc
@@ -41,7 +41,13 @@ v8::Handle<v8::Value> SendRequestNatives::StartRequest(
   bool for_io_thread = args[4]->BooleanValue();
 
   scoped_ptr<V8ValueConverter> converter(V8ValueConverter::create());
-  converter->SetUndefinedAllowed(true);  // Allow passing optional values.
+
+  // Make "undefined" the same as "null" for optional arguments, but for objects
+  // strip nulls (like {foo: null}, and therefore {foo: undefined} as well) to
+  // make it easier for extension APIs to check for optional arguments.
+  converter->SetUndefinedAllowed(true);
+  converter->SetStripNullFromObjects(true);
+
   scoped_ptr<Value> value_args(
       converter->FromV8Value(args[1], v8::Context::GetCurrent()));
   if (!value_args.get() || !value_args->IsType(Value::TYPE_LIST)) {
diff --git a/chrome/renderer/resources/extensions/json_schema.js b/chrome/renderer/resources/extensions/json_schema.js
index ec4b48d..34e2378 100644
--- a/chrome/renderer/resources/extensions/json_schema.js
+++ b/chrome/renderer/resources/extensions/json_schema.js
@@ -50,6 +50,10 @@ function isInstanceOfClass(instance, className) {
   return isInstanceOfClass(Object.getPrototypeOf(instance), className);
 }
 
+function isOptionalValue(value) {
+  return typeof(value) === 'undefined' || value === null;
+}
+
 /**
  * Validates an instance against a schema and accumulates errors. Usage:
  *
@@ -177,6 +181,10 @@ chromeHidden.JSONSchemaValidator.prototype.getAllTypesForSchema =
  */
 chromeHidden.JSONSchemaValidator.prototype.isValidSchemaType =
     function(type, schema) {
+  if (type == 'any')
+    return true;
+
+  // TODO(kalman): I don't understand this code. How can type be "null"?
   if (schema.optional && (type == "null" || type == "undefined"))
     return true;
 
@@ -185,7 +193,8 @@ chromeHidden.JSONSchemaValidator.prototype.isValidSchemaType =
     if (schemaTypes[i] == "any" || type == schemaTypes[i])
       return true;
   }
-  return type == "any";
+
+  return false;
 };
 
 /**
@@ -328,7 +337,7 @@ chromeHidden.JSONSchemaValidator.prototype.validateObject =
       var propPath = path ? path + "." + prop : prop;
       if (schema.properties[prop] == undefined) {
         this.addError(propPath, "invalidPropertyType");
-      } else if (prop in instance && instance[prop] !== undefined) {
+      } else if (prop in instance && !isOptionalValue(instance[prop])) {
         this.validate(instance[prop], schema.properties[prop], propPath);
       } else if (!schema.properties[prop].optional) {
         this.addError(propPath, "propertyRequired");
@@ -397,7 +406,7 @@ chromeHidden.JSONSchemaValidator.prototype.validateArray =
     // validate against the corresponding schema.
     for (var i = 0; i < schema.items.length; i++) {
       var itemPath = path ? path + "." + i : String(i);
-      if (i in instance && instance[i] !== null && instance[i] !== undefined) {
+      if (i in instance && !isOptionalValue(instance[i])) {
         this.validate(instance[i], schema.items[i], itemPath);
       } else if (!schema.items[i].optional) {
         this.addError(itemPath, "itemRequired");