Add JS bindings validation logic for non-nullable types

mojo/public/js/bindings/validator.js

Index: mojo/public/js/bindings/validator.js
diff --git a/mojo/public/js/bindings/validator.js b/mojo/public/js/bindings/validator.js
index 837f85a37df59b15758ea8286b91c726c416c998..315c920e0a500abf334ca99fe98889e372cd2b6a 100644
--- a/mojo/public/js/bindings/validator.js
+++ b/mojo/public/js/bindings/validator.js
@@ -4,7 +4,7 @@
 
 define("mojo/public/js/bindings/validator", [
   "mojo/public/js/bindings/codec",
-], function(codec) {
+  ], function(codec) {

[yzshen1, 2014/08/20 17:04:20]

I think the indent is a little unusual.
Maybe the previous is correct?

[hansmuller, 2014/08/20 17:46:30]

Done.

 
   var validationError = {
     NONE: 'VALIDATION_ERROR_NONE',
@@ -13,7 +13,9 @@ define("mojo/public/js/bindings/validator", [
     UNEXPECTED_STRUCT_HEADER: 'VALIDATION_ERROR_UNEXPECTED_STRUCT_HEADER',
     UNEXPECTED_ARRAY_HEADER: 'VALIDATION_ERROR_UNEXPECTED_ARRAY_HEADER',
     ILLEGAL_HANDLE: 'VALIDATION_ERROR_ILLEGAL_HANDLE',
+    UNEXPECTED_INVALID_HANDLE: 'VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE',
     ILLEGAL_POINTER: 'VALIDATION_ERROR_ILLEGAL_POINTER',
+    UNEXPECTED_NULL_POINTER: 'VALIDATION_ERROR_UNEXPECTED_NULL_POINTER',
     MESSAGE_HEADER_INVALID_FLAG_COMBINATION:
         'VALIDATION_ERROR_MESSAGE_HEADER_INVALID_FLAG_COMBINATION',
     MESSAGE_HEADER_MISSING_REQUEST_ID:
@@ -22,6 +24,20 @@ define("mojo/public/js/bindings/validator", [
 
   var NULL_MOJO_POINTER = "NULL_MOJO_POINTER";
 
+  function isStringClass(cls) {
+    return cls === codec.String || cls === codec.NullableString;
+  }
+
+  function isHandleClass(cls) {
+    return cls === codec.Handle || cls === codec.NullableHandle;
+  }
+
+  function isNullable(type) {
+    return type === codec.NullableString || type === codec.NullableHandle ||
+      type instanceof codec.NullableArrayOf ||

[yzshen1, 2014/08/20 17:04:20]

wrong indent. (37 - 39)

[hansmuller, 2014/08/20 17:46:31]

Done.

+      type instanceof codec.NullablePointerTo;
+}
+
   function Validator(message) {
     this.message = message;
     this.offset = 0;
@@ -73,8 +89,13 @@ define("mojo/public/js/bindings/validator", [
     return true;
   }
 
-  Validator.prototype.validateHandle = function(offset) {
+  Validator.prototype.validateHandle = function(offset, nullable) {

[yzshen1, 2014/08/20 17:04:20]

One thing may worth considering:
Passing nullable at runtime may be unnecessary: we could have two separate
functions:
  validateHandle()
  isHandleValid()

For nullable handle field, we generate code to call validateHandle().
For non-nullable handle field, we generate code to call validateHandle() &&
isHandleValid().

We could consider similar things for structs/arrays, too.

In C++, I did such things because validation is on the performance critical
path. But I don't think I know about javascript performance very well. Maybe it
doesn't matter.

[hansmuller, 2014/08/20 17:46:31]

I don't know what the performance impact of an additional boolean function
parameter to these functions is for JS either. If it's OK with you, I'd like to
defer JS bindings performance tuning until it can be informed by benchmarks.

[yzshen1, 2014/08/20 17:54:08]

Sounds good.

     var index = this.message.buffer.getUint32(offset);
+
+    if (index === codec.kEncodedInvalidHandleValue)
+      return nullable ?
+          validationError.NONE : validationError.UNEXPECTED_INVALID_HANDLE;
+
     if (!this.claimHandle(index))
       return validationError.ILLEGAL_HANDLE;
     return validationError.NONE;
@@ -140,23 +161,30 @@ define("mojo/public/js/bindings/validator", [
     return Number.isSafeInteger(bufferOffset) ? bufferOffset : null;
   }
 
-  Validator.prototype.validateArrayPointer =
-      function(offset, elementSize, expectedElementCount, elementType) {
+  Validator.prototype.validateArrayPointer = function(
+      offset, elementSize, expectedElementCount, elementType, nullable) {
     var arrayOffset = this.decodePointer(offset);
     if (arrayOffset === null)
       return validationError.ILLEGAL_POINTER;
+
     if (arrayOffset === NULL_MOJO_POINTER)
-      return validationError.NONE;
+      return nullable ?
+          validationError.NONE : validationError.UNEXPECTED_NULL_POINTER;
+
     return this.validateArray(
         arrayOffset, elementSize, expectedElementCount, elementType);
   }
 
-  Validator.prototype.validateStructPointer = function(offset, structClass) {
+  Validator.prototype.validateStructPointer = function(
+        offset, structClass, nullable) {
     var structOffset = this.decodePointer(offset);
     if (structOffset === null)
       return validationError.ILLEGAL_POINTER;
+
     if (structOffset === NULL_MOJO_POINTER)
-      return validationError.NONE;
+      return nullable ?
+          validationError.NONE : validationError.UNEXPECTED_NULL_POINTER;
+
     return structClass.validate(this, structOffset);
   }
 
@@ -196,17 +224,19 @@ define("mojo/public/js/bindings/validator", [
     // Validate the array's elements if they are pointers or handles.
 
     var elementsOffset = offset + codec.kArrayHeaderSize;
-    if (elementType === codec.Handle)
-      return this.validateHandleElements(elementsOffset, numElements);
+    var nullable = isNullable(elementType);
+
+    if (isHandleClass(elementType))
+      return this.validateHandleElements(elementsOffset, numElements, nullable);
+    if (isStringClass(elementType))
+      return this.validateArrayElements(
+          elementsOffset, numElements, codec.Uint8, nullable)
     if (elementType instanceof codec.PointerTo)
       return this.validateStructElements(
-          elementsOffset, numElements, elementType.cls);
-    if (elementType instanceof codec.String)
-      return this.validateArrayElements(
-          elementsOffset, numElements, codec.Uint8);
+          elementsOffset, numElements, elementType.cls, nullable);
     if (elementType instanceof codec.ArrayOf)
       return this.validateArrayElements(
-          elementsOffset, numElements, elementType.cls);
+          elementsOffset, numElements, elementType.cls, nullable);
 
     return validationError.NONE;
   }
@@ -215,24 +245,26 @@ define("mojo/public/js/bindings/validator", [
   // methods below is "safe" because elementSize <= 8, offset and
   // numElements are uint32, and 0 <= i < numElements.
 
-  Validator.prototype.validateHandleElements = function(offset, numElements) {
+  Validator.prototype.validateHandleElements =
+      function(offset, numElements, nullable) {
     var elementSize = codec.Handle.encodedSize;
     for (var i = 0; i < numElements; i++) {
-      var index = this.message.buffer.getUint32(offset + i * elementSize);
-      if (!this.claimHandle(index))
-        return validationError.ILLEGAL_HANDLE;
+      var elementOffset = offset + i * elementSize;
+      var err = this.validateHandle(elementOffset, nullable);
+      if (err != validationError.NONE)
+        return err;
     }
     return validationError.NONE;
   }
 
   // The elementClass parameter is the element type of the element arrays.
   Validator.prototype.validateArrayElements =
-      function(offset, numElements, elementClass) {
+      function(offset, numElements, elementClass, nullable) {
     var elementSize = codec.PointerTo.prototype.encodedSize;
     for (var i = 0; i < numElements; i++) {
       var elementOffset = offset + i * elementSize;
       var err = this.validateArrayPointer(
-          elementOffset, elementClass.encodedSize, 0, elementClass);
+        elementOffset, elementClass.encodedSize, 0, elementClass, nullable);

[yzshen1, 2014/08/20 17:04:20]

wrong indent.

[hansmuller, 2014/08/20 17:46:31]

Done.

       if (err != validationError.NONE)
         return err;
     }
@@ -240,11 +272,12 @@ define("mojo/public/js/bindings/validator", [
   }
 
   Validator.prototype.validateStructElements =
-      function(offset, numElements, structClass) {
+      function(offset, numElements, structClass, nullable) {
     var elementSize = codec.PointerTo.prototype.encodedSize;
     for (var i = 0; i < numElements; i++) {
       var elementOffset = offset + i * elementSize;
-      var err = this.validateStructPointer(elementOffset, structClass);
+      var err =
+          this.validateStructPointer(elementOffset, structClass, nullable);
       if (err != validationError.NONE)
         return err;
     }