JsonSchema support for declaring "object" constructors.

This adds support for an optional declaration of "isInstanceOf" on "type":"object". If present, the candidate object will be tested for js:instanceof window[schema.isInstanceOf].

Several api functions are modified to take advantage of declaring their "class".

Also, support added to docs for modules, methods & events to declare "nodocs":"true" to be excluded from the docs.

Also, check to warn on an attempt to use cygwin python to generate docs.

BUG=27213
TEST=NONE

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

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

9 files changed, 169 insertions, 81 deletions

diff --git a/chrome/browser/extensions/extension_popup_api.cc b/chrome/browser/extensions/extension_popup_api.cc
index 8ef2fe8..26db2e1 100644
--- a/chrome/browser/extensions/extension_popup_api.cc
+++ b/chrome/browser/extensions/extension_popup_api.cc
@@ -75,12 +75,8 @@ bool PopupShowFunction::RunImpl() {
   EXTENSION_FUNCTION_VALIDATE(args_->IsType(Value::TYPE_LIST));
   const ListValue* args = args_as_list();
 
-  DictionaryValue* popup_info = NULL;
-  EXTENSION_FUNCTION_VALIDATE(args->GetDictionary(0, &popup_info));
-
   std::string url_string;
-  EXTENSION_FUNCTION_VALIDATE(popup_info->GetString(kUrlKey,
-                                                    &url_string));
+  EXTENSION_FUNCTION_VALIDATE(args->GetString(0, &url_string));
 
   DictionaryValue* dom_anchor = NULL;
   EXTENSION_FUNCTION_VALIDATE(args->GetDictionary(1, &dom_anchor));
diff --git a/chrome/common/extensions/api/extension_api.json b/chrome/common/extensions/api/extension_api.json
index 1715b4d..6c09c51 100755
--- a/chrome/common/extensions/api/extension_api.json
+++ b/chrome/common/extensions/api/extension_api.json
@@ -12,20 +12,33 @@
         }
       },
       {
+        "id": "Event",
+        "type": "object",
+        "description": "An object which allows the addition and removal of listeners for a Chrome event.",
+        "properties": {
+          "addListener": {"type": "function"},
+          "removeListener": {"type": "function"},
+          "hasListener": {"type": "function"},
+          "hasListeners": {"type": "function"}
+        },
+        "additionalProperties": { "type": "any"}
+      },
+      {
         "id": "Port",
         "type": "object",
         "description": "An object which allows two way communication with other pages.",
         "properties": {
           "name": {"type": "string"},
-          "onDisconnect": {"type": "object"},
-          "onMessage": {"type": "object"},
+          "onDisconnect": { "$ref": "Event" },
+          "onMessage": { "$ref": "Event" },
           "postMessage": {"type": "function"},
           "sender": {
             "$ref": "MessageSender",
             "optional": true,
             "description": "This property will <b>only</b> be present on ports passed to onConnect/onConnectExternal listeners."
           }
-        }
+        },
+        "additionalProperties": { "type": "any"}
       }
     ],
     "properties": {
@@ -104,7 +117,7 @@
         "returns": {
           "type": "array",
           "description": "Array of global objects",
-          "items": { "type": "object" }
+          "items": { "type": "object", "isInstanceOf": "DOMWindow", "properties": {}, "additionalProperties": { "type": "any" } }
         }
       },
       {
@@ -113,8 +126,8 @@
         "description": "Returns the global JavaScript object for the background page running inside the current extension. Returns null if the extension has no backround page.",
         "parameters": [],
         "returns": {
-          "type": "object"
-        }
+          "type": "object", "isInstanceOf": "DOMWindow", "properties": {}, "additionalProperties": { "type": "any" }
+         }
       },
       {
         "name": "getToolstrips",
@@ -126,7 +139,7 @@
         "returns": {
           "type": "array",
           "description": "Array of global objects",
-          "items": { "type": "object" }
+          "items": { "type": "object", "isInstanceOf": "DOMWindow", "properties": {}, "additionalProperties": { "type": "any" } }
         }
       },
       {
@@ -139,7 +152,7 @@
         "returns": {
           "type": "array",
           "description": "Array of global objects",
-          "items": { "type": "object" }
+          "items": { "type": "object", "isInstanceOf": "DOMWindow", "properties": {}, "additionalProperties": { "type": "any" } }
         }
       }
     ],
@@ -184,16 +197,19 @@
   },
   {
     "namespace": "experimental.extension",
+    "nodoc": true,
     "types": [],
     "functions": [
       {
         "name": "getPopupView",
         "type": "function",
         "description": "Returns a reference to the window object of the popup view.",
-        "nodocs": "true",
         "parameters": [],
         "returns": {
-          "type": "object"
+          "type": "object",
+          "isInstanceOf": "DOMWindow",
+          "properties": {},
+          "additionalProperties": { "type": "any" }
         }
       }
     ],
@@ -709,11 +725,7 @@
             "description": "When all scripts are executed, this callback is called.",
             "parameters": []
           }
-        ],
-        "returns": {
-          "type": "boolean",
-          "description": "Whether this call is successful"
-        }
+        ]
       },
       {
         "name": "insertCSS",
@@ -738,11 +750,7 @@
             "description": "When all css are inserted, this callback is called.",
             "parameters": []
           }
-        ],
-        "returns": {
-          "type": "boolean",
-          "description": "Whether this call is successful"
-        }
+        ]
       }
     ],
     "events": [
@@ -872,6 +880,7 @@
   },
   {
     "namespace": "pageActions",
+    "nodoc": true,
     "types": [],
     "functions": [
       {
@@ -962,7 +971,10 @@
             "properties": {
               "tabId": {"type": "integer", "minimum": 0, "description": "The id of the tab for which you want to modify the page action."},
               "imageData": {
-                "type": "any",
+                "type": "object",
+                "isInstanceOf": "ImageData",
+                "properties": {},
+                "additionalProperties": { "type": "any" },
                 "description": "Pixel data for an image. Must be an ImageData object (for example, from a canvas element).",
                 "optional": true
               },
@@ -1032,7 +1044,10 @@
             "type": "object",
             "properties": {
               "imageData": {
-                "type": "any",
+                "type": "object",
+                "isInstanceOf": "ImageData",
+                "properties": {},
+                "additionalProperties": { "type": "any" },
                 "description": "Pixel data for an image. Must be an ImageData object (for example, from a <code>canvas</code> element).",
                 "optional": true
               },
@@ -1374,6 +1389,7 @@
   },
   {
     "namespace": "experimental.history",
+    "nodoc": true,
     "types": [
       {
         "id": "HistoryItem",
@@ -1528,6 +1544,7 @@
   },
   {
     "namespace": "toolstrip",
+    "nodoc": true,
     "types": [],
     "functions": [
       {
@@ -1586,6 +1603,7 @@
       {
         "name": "getMessage",
         "type": "function",
+        "nodoc": true,
         "description": "Get a message from the extension language catalog, for a current locale.",
         "parameters": [
           { "type": "string",
@@ -1617,30 +1635,31 @@
   },
   {
     "namespace": "experimental.popup",
+    "nodoc": true,
     "types": [],
     "functions": [
       {
         "name": "show",
         "type": "function",
         "description": "Displays a pop-up window hosting an extension view.",
-        "nodocs": "true",
         "customHandler": "true",
         "parameters": [
           {
-            "type": "object",
-            "name": "popupInfo",
-            "properties": {
-              "url": { "type": "string", "description": "The URL of the contents to which the pop-up will be navigated." }
-            }
+            "type": "string",
+            "name": "url",
+            "description": "The URL of the contents to which the pop-up will be navigated."
           },
           {
             "type": "object",
-            "name": "domAnchor",
+            "name": "showDetails",
             "properties": {
-              "top": { "type": "integer", "minimum": 0, "description": "Top pixel position of the dom-anchor." },
-              "left": { "type": "integer", "minimum": 0, "description": "Left pixel position of the dom-anchor." },
-              "width": { "type": "integer", "minimum": 0, "description": "Pixel width of the dom-anchor." },
-              "height": { "type": "integer", "minimum": 0, "description": "Pixel height of the dom-anchor." }
+              "relativeTo": {
+                "type": "object",
+                "properties": {},
+                "additionalProperties": { "type": "any" },
+                "isInstanceOf": "HTMLElement",
+                "description": "The HTMLElement whose position which serve as the anchor position of the popup."
+              }
             }
           },
           {
@@ -1654,10 +1673,12 @@
         "name": "getAnchorWindow",
         "type": "function",
         "description": "Returns a reference to the window object of the extension view that launched the popup.",
-        "nodocs": "true",
         "parameters": [],
         "returns": {
-          "type": "object"
+          "type": "object",
+          "isInstanceOf": "DOMWindow",
+          "properties": {},
+          "additionalProperties": { "type": "any" }
         }
       }
     ],
@@ -1672,14 +1693,13 @@
   },
   {
     "namespace": "devtools",
-    "types": [
-    ],
+    "types": [],
     "functions": [
       {
         "name": "getTabEvents",
         "type": "function",
         "description": "EXPERIMENTAL support for timeline API",
-        "nodocs": "true",
+        "nodoc": "true",
         "parameters": [
           {
             "name": "tab_id",
@@ -1688,6 +1708,8 @@
         ],
         "returns": {
           "type": "object",
+          "properties": {},
+          "additionalProperties": { "type": "any" },
           "description": "DevTools tab events object"
         }
       }
diff --git a/chrome/common/extensions/docs/build/build.py b/chrome/common/extensions/docs/build/build.py
index 8d55ab1..c44bf8a 100755
--- a/chrome/common/extensions/docs/build/build.py
+++ b/chrome/common/extensions/docs/build/build.py
@@ -141,8 +141,9 @@ def GetAPIModuleNames():
   except ValueError, msg:
     raise Exception("File %s has a syntax error: %s" %
                     (_extension_api_json, msg))
-
-  return set(module['namespace'].encode() for module in extension_api)
+  # Exclude modules with a "nodoc" property.
+  return set(module['namespace'].encode() for module in extension_api
+             if "nodoc" not in module)
 
 def GetStaticFileNames():
   static_files = os.listdir(_static_dir)
@@ -151,6 +152,11 @@ def GetStaticFileNames():
              if file.endswith(".html"))
 
 def main():
+  # Prevent windows from using cygwin python.
+  if (sys.platform == "cygwin"):
+    raise Exception("Building docs not supported for cygwin python.\n"
+                    "Please run the build.bat script.");
+
   parser = OptionParser()
   parser.add_option("--test-shell-path", dest="test_shell_path")
   (options, args) = parser.parse_args()
diff --git a/chrome/common/extensions/docs/js/api_page_generator.js b/chrome/common/extensions/docs/js/api_page_generator.js
index dbd30dc..2e2a0af 100755
--- a/chrome/common/extensions/docs/js/api_page_generator.js
+++ b/chrome/common/extensions/docs/js/api_page_generator.js
@@ -366,6 +366,9 @@ function getTypeName(schema) {
   if (schema.type == "array")
     return "array of " + getTypeName(schema.items);
 
+  if (schema.isInstanceOf)
+    return schema.isInstanceOf;
+
   return schema.type;
 }
 
diff --git a/chrome/common/extensions/docs/template/api_template.html b/chrome/common/extensions/docs/template/api_template.html
index c5193ee..faac001 100644
--- a/chrome/common/extensions/docs/template/api_template.html
+++ b/chrome/common/extensions/docs/template/api_template.html
@@ -172,7 +172,8 @@
                   <li jsdisplay="functions && functions.length > 0">
                     <a href="#methods">Methods</a>
                     <ol>
-                      <li jsselect="functions.sort(sortByName)">
+                      <li jsselect="functions.sort(sortByName)"
+                          jsdisplay="!($this.nodoc)">
                         <a jscontent="name"
                            jsvalues=".href:'#method-' + name"
                            href="#method-anchor">methodName</a>
@@ -182,7 +183,8 @@
                   <li jsdisplay="events && events.length > 0">
                     <a href="#events">Events</a>
                     <ol>
-                      <li jsselect="events.sort(sortByName)">
+                      <li jsselect="events.sort(sortByName)"
+                          jsdisplay="!($this.nodoc)">
                         <a jscontent="name"
                            jsvalues=".href:'#event-' + name"
                            href="#event-anchor">eventName</a>
@@ -238,7 +240,8 @@
             <h3>Methods</h3>
 
             <!-- iterates over all functions -->
-            <div class="apiItem" jsselect="functions.sort(sortByName)">
+            <div class="apiItem" jsselect="functions.sort(sortByName)"
+                 jsdisplay="!($this.nodoc)">
               <a jsvalues=".name:'method-' + name"></a> <!-- method-anchor -->
               <h4 jscontent="name">method name</h4>
 
@@ -303,7 +306,8 @@
             <h3 id="events">Events</h3>
 
             <!-- iterates over all events -->
-            <div jsselect="events.sort(sortByName)" class="apiItem">
+            <div jsselect="events.sort(sortByName)" class="apiItem"
+                 jsdisplay="!($this.nodoc)">
               <a jsvalues=".name:'event-' + name"></a>
               <h4 jscontent="name">event name</h4>
 
diff --git a/chrome/renderer/resources/extension_process_bindings.js b/chrome/renderer/resources/extension_process_bindings.js
index 6a0584b..85b65ea 100644
--- a/chrome/renderer/resources/extension_process_bindings.js
+++ b/chrome/renderer/resources/extension_process_bindings.js
@@ -316,23 +316,22 @@ var chrome = chrome || {};
           apiFunctions[apiFunction.name] = apiFunction;
 
           module[functionDef.name] = bind(apiFunction, function() {
-            // If the function is marked with a custom handler, then bypass
-            // validation of the arguments.  This flag is required for
-            // extensions taking DOM objects as arguments.  DOM objects
-            // cannot be described using the type system in extension_api.json,
-            // and so cannot be validated.
-            // A good practice is to extract the primitive data needed to
-            // service the request, and then invoke validate against that data.
-            // See popup.show(...) for an example.
-            if (!functionDef.customHandler) {
-              chromeHidden.validate(arguments, this.definition.parameters);
-            }
-
+            chromeHidden.validate(arguments, this.definition.parameters);
+            
+            var retval;
             if (this.handleRequest)
-              return this.handleRequest.apply(this, arguments);
+              retval = this.handleRequest.apply(this, arguments);
             else
-              return sendRequest(this.name, arguments,
+              retval = sendRequest(this.name, arguments,
                                  this.definition.parameters);
+
+            // Validate return value if defined - only in debug.
+            if (chromeHidden.validateCallbacks &&
+                chromeHidden.validate &&
+                this.definition.returns) {
+              chromeHidden.validate([retval], [this.definition.returns]);
+            }
+            return retval;
           });
         });
       }
@@ -420,20 +419,9 @@ var chrome = chrome || {};
 
     apiFunctions["experimental.popup.show"].handleRequest =
         function(url, showDetails, callback) {
-      if (!showDetails || !showDetails.relativeTo) {
-        throw new Error("showDetails.relativeTo argument missing.");
-      }
-
-      var position = getAbsoluteRect(showDetails.relativeTo);
-      var popUpInfo = {
-        "url": url
-      };
-      var domAnchor = position;
-      var modifiedArgs = [popUpInfo, domAnchor, callback];
-      chromeHidden.validate(modifiedArgs, this.definition.parameters);
-
-      return sendRequest(this.name, modifiedArgs,
-                         this.definition.parameters);
+      var internalArgs = [url, getAbsoluteRect(showDetails.relativeTo),
+          callback];
+      return sendRequest(this.name, internalArgs, this.definition.parameters);
     }
 
     apiFunctions["experimental.extension.getPopupView"].handleRequest =
diff --git a/chrome/renderer/resources/json_schema.js b/chrome/renderer/resources/json_schema.js
index 750f89d..6d0ef53 100644
--- a/chrome/renderer/resources/json_schema.js
+++ b/chrome/renderer/resources/json_schema.js
@@ -33,7 +33,9 @@
 // - 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
+// - by default an "object" typed schema does not allow additional properties.
+//   if present, "additionalProperties" is to be a schema against which all
+//   additional properties will be validated.
 //==============================================================================
 
 (function() {
@@ -77,7 +79,8 @@ chromeHidden.JSONSchemaValidator.messages = {
   invalidChoice: "Value does not match any valid type choices.",
   invalidPropertyType: "Missing property type.",
   schemaRequired: "Schema value required.",
-  unknownSchemaReference: "Unknown schema reference: *."
+  unknownSchemaReference: "Unknown schema reference: *.",
+  notInstance: "Object must be an instance of *."
 };
 
 /**
@@ -272,6 +275,39 @@ chromeHidden.JSONSchemaValidator.prototype.validateObject = function(
     }
   }
 
+  // If "instanceof" property is set, check that this object inherits from
+  // the specified constructor (function).
+  if (schema.isInstanceOf) {
+    var isInstance = function() {
+      var constructor = this[schema.isInstanceOf];
+      if (constructor) {
+        return (instance instanceof constructor);
+      }
+
+      // Special-case constructors that can not always be found on the global
+      // object, but for which we to allow validation.
+      var allowedNamedConstructors = {
+        "DOMWindow": true,
+        "ImageData": true
+      }
+      if (!allowedNamedConstructors[schema.isInstanceOf]) {
+        throw "Attempt to validate against an instance ctor that could not be" +
+              "found: " + schema.isInstanceOf;
+      }
+      return (schema.isInstanceOf == instance.constructor.name)
+    }();
+
+    if (!isInstance)
+      this.addError(propPath, "notInstance", [schema.isInstanceOf]);
+  }
+
+  // Exit early from additional property check if "type":"any" is defined.
+  if (schema.additionalProperties &&
+      schema.additionalProperties.type &&
+      schema.additionalProperties.type == "any") {
+    return;
+  }
+
   // 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.
diff --git a/chrome/test/data/extensions/api_test/popup_api/toolband.html b/chrome/test/data/extensions/api_test/popup_api/toolband.html
index dd87769..fba3577 100644
--- a/chrome/test/data/extensions/api_test/popup_api/toolband.html
+++ b/chrome/test/data/extensions/api_test/popup_api/toolband.html
@@ -15,7 +15,6 @@ window.onload = function() {
             chrome.experimental.extension.getPopupView() != undefined);
       }));
     },
-
     function accessPopup() {
       var popupView = chrome.experimental.extension.getPopupView();
       chrome.test.assertTrue(popupView != undefined,
diff --git a/chrome/test/data/extensions/json_schema_test.js b/chrome/test/data/extensions/json_schema_test.js
index 1713b27..3f55c08 100644
--- a/chrome/test/data/extensions/json_schema_test.js
+++ b/chrome/test/data/extensions/json_schema_test.js
@@ -14,7 +14,9 @@ function assertValid(type, instance, schema, types) {
   validator["validate" + type](instance, schema, "");
   if (validator.errors.length != 0) {
     log("Got unexpected errors");
-    log(validator.errors);
+    for (var i = 0; i < validator.errors.length; i++) {
+      log(validator.errors[i].message + "  path: " + validator.errors[i].path);
+    }
     assert(false);
   }
 }
@@ -165,6 +167,16 @@ function testExtends() {
   assertValid("", 43, schema);
 }
 
+function ClassA() {
+  this.a = "a";
+}
+function ClassB() {
+}
+ClassB.prototype = new ClassA();
+function ClassC() {
+  this.a = "a";
+}
+
 function testObject() {
   var schema = {
     properties: {
@@ -201,6 +213,28 @@ function testObject() {
   assertValid("Object", {foo:"foo",bar:undefined}, schema);
   assertNotValid("Object", {foo:"foo", bar:"42"}, schema,
                  [formatError("invalidType", ["integer", "string"])]);
+
+  var classASchema = {
+    properties: {
+      "a": { type: "string" }
+    },
+    isInstanceOf: "ClassA"
+  };
+
+  var classBSchema = {
+    properties: {},
+    isInstanceOf: "ClassB"
+  };
+
+  var a = new ClassA();
+  var b = new ClassB();
+  var c = new ClassC();
+
+  assertValid("Object", a, classASchema);
+  assertValid("Object", b, classBSchema);
+  assertValid("Object", b, classASchema);
+  assertNotValid("Object", c, classASchema,
+                 [formatError("notInstance", [classASchema.isInstanceOf])]);              
 }
 
 function testTypeReference() {