Fix the Java Bridge to support coercing to Java arrays

This change adds support for coercing to Java arrays when passing JavaScript
objects to the methods of Java objects injected using the Java Bridge.
- Modify JavaType to store the type of the array elements when the type of the
  value is TypeArray.
- Add logic to create Java arrays and to interrogate JavaScript objects for
  their length property and array elements.
- Modify existing CoerceJavaScriptXXXToJavaValue() methods to take an
  additional coerce_to_string argument. This is required because we require
  slightly different behaviour when coercing to Java strings, depending on
  whether or not the value is an array element.
- Modify JavaBridgeChannel to override
  NPChannelBase::OnControlMessageReceived() to avoid DCHECK().

Also fix a potential overflow when converting JavaScript numbers to Java
strings.

BUG=105547

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

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

6 files changed, 322 insertions, 38 deletions

diff --git a/content/browser/renderer_host/java/java_bound_object.cc b/content/browser/renderer_host/java/java_bound_object.cc
index 1fd4a74..ced5106 100644
--- a/content/browser/renderer_host/java/java_bound_object.cc
+++ b/content/browser/renderer_host/java/java_bound_object.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -193,7 +193,8 @@ NPVariant CallJNIMethod(jobject object, const JavaType& return_type,
 }
 
 jvalue CoerceJavaScriptNumberToJavaValue(const NPVariant& variant,
-                                         const JavaType& target_type) {
+                                         const JavaType& target_type,
+                                         bool coerce_to_string) {
   // See http://jdk6.java.net/plugin2/liveconnect/#JS_NUMBER_VALUES.
   jvalue result;
   DCHECK(variant.type == NPVariantType_Int32 ||
@@ -236,10 +237,12 @@ jvalue CoerceJavaScriptNumberToJavaValue(const NPVariant& variant,
       result.l = NULL;
       break;
     case JavaType::TypeString:
-      result.l = ConvertUTF8ToJavaString(
-          AttachCurrentThread(),
-          is_double ?  StringPrintf("%.6lg", NPVARIANT_TO_DOUBLE(variant)) :
-                       base::IntToString(NPVARIANT_TO_INT32(variant)));
+      result.l = coerce_to_string ?
+          ConvertUTF8ToJavaString(
+              AttachCurrentThread(),
+              is_double ? StringPrintf("%.6lg", NPVARIANT_TO_DOUBLE(variant)) :
+                          base::Int64ToString(NPVARIANT_TO_INT32(variant))) :
+          NULL;
       break;
     case JavaType::TypeBoolean:
       // LIVECONNECT_COMPLIANCE: Existing behavior is to convert to false. Spec
@@ -260,7 +263,8 @@ jvalue CoerceJavaScriptNumberToJavaValue(const NPVariant& variant,
 }
 
 jvalue CoerceJavaScriptBooleanToJavaValue(const NPVariant& variant,
-                                          const JavaType& target_type) {
+                                          const JavaType& target_type,
+                                          bool coerce_to_string) {
   // See http://jdk6.java.net/plugin2/liveconnect/#JS_BOOLEAN_VALUES.
   DCHECK_EQ(NPVariantType_Bool, variant.type);
   bool boolean_value = NPVARIANT_TO_BOOLEAN(variant);
@@ -275,8 +279,10 @@ jvalue CoerceJavaScriptBooleanToJavaValue(const NPVariant& variant,
       result.l = NULL;
       break;
     case JavaType::TypeString:
-      result.l = ConvertUTF8ToJavaString(AttachCurrentThread(),
-                                         boolean_value ? "true" : "false");
+      result.l = coerce_to_string ?
+          ConvertUTF8ToJavaString(AttachCurrentThread(),
+                                  boolean_value ? "true" : "false") :
+          NULL;
       break;
     case JavaType::TypeByte:
     case JavaType::TypeChar:
@@ -356,8 +362,168 @@ jvalue CoerceJavaScriptStringToJavaValue(const NPVariant& variant,
   return result;
 }
 
+// Note that this only handles primitive types and strings.
+jobject CreateJavaArray(const JavaType& type, jsize length) {
+  JNIEnv* env = AttachCurrentThread();
+  switch (type.type) {
+    case JavaType::TypeBoolean:
+      return env->NewBooleanArray(length);
+    case JavaType::TypeByte:
+      return env->NewByteArray(length);
+    case JavaType::TypeChar:
+      return env->NewCharArray(length);
+    case JavaType::TypeShort:
+      return env->NewShortArray(length);
+    case JavaType::TypeInt:
+      return env->NewIntArray(length);
+    case JavaType::TypeLong:
+      return env->NewLongArray(length);
+    case JavaType::TypeFloat:
+      return env->NewFloatArray(length);
+    case JavaType::TypeDouble:
+      return env->NewDoubleArray(length);
+    case JavaType::TypeString: {
+      ScopedJavaLocalRef<jclass> clazz(env, env->FindClass("java/lang/String"));
+      return env->NewObjectArray(length, clazz.obj(), NULL);
+    }
+    case JavaType::TypeVoid:
+      // Conversion to void must never happen.
+    case JavaType::TypeArray:
+    case JavaType::TypeObject:
+      // Not handled.
+      NOTREACHED();
+  }
+  return NULL;
+}
+
+// Note that this only handles primitive types and strings.
+void SetArrayElement(jobject array,
+                     const JavaType& type,
+                     jsize index,
+                     const jvalue& value) {
+  JNIEnv* env = AttachCurrentThread();
+  switch (type.type) {
+    case JavaType::TypeBoolean:
+      env->SetBooleanArrayRegion(static_cast<jbooleanArray>(array), index, 1,
+                                 &value.z);
+      break;
+    case JavaType::TypeByte:
+      env->SetByteArrayRegion(static_cast<jbyteArray>(array), index, 1,
+                              &value.b);
+      break;
+    case JavaType::TypeChar:
+      env->SetCharArrayRegion(static_cast<jcharArray>(array), index, 1,
+                              &value.c);
+      break;
+    case JavaType::TypeShort:
+      env->SetShortArrayRegion(static_cast<jshortArray>(array), index, 1,
+                               &value.s);
+      break;
+    case JavaType::TypeInt:
+      env->SetIntArrayRegion(static_cast<jintArray>(array), index, 1,
+                             &value.i);
+      break;
+    case JavaType::TypeLong:
+      env->SetLongArrayRegion(static_cast<jlongArray>(array), index, 1,
+                              &value.j);
+      break;
+    case JavaType::TypeFloat:
+      env->SetFloatArrayRegion(static_cast<jfloatArray>(array), index, 1,
+                               &value.f);
+      break;
+    case JavaType::TypeDouble:
+      env->SetDoubleArrayRegion(static_cast<jdoubleArray>(array), index, 1,
+                                &value.d);
+      break;
+    case JavaType::TypeString:
+      env->SetObjectArrayElement(static_cast<jobjectArray>(array), index,
+                                 value.l);
+      break;
+    case JavaType::TypeVoid:
+      // Conversion to void must never happen.
+    case JavaType::TypeArray:
+    case JavaType::TypeObject:
+      // Not handled.
+      NOTREACHED();
+  }
+  base::android::CheckException(env);
+}
+
+jvalue CoerceJavaScriptValueToJavaValue(const NPVariant& variant,
+                                        const JavaType& target_type,
+                                        bool coerce_to_string);
+
+jobject CoerceJavaScriptObjectToArray(const NPVariant& variant,
+                                      const JavaType& target_type) {
+  DCHECK_EQ(JavaType::TypeArray, target_type.type);
+  NPObject* object = NPVARIANT_TO_OBJECT(variant);
+  DCHECK_NE(&JavaNPObject::kNPClass, object->_class);
+
+  const JavaType& target_inner_type = *target_type.inner_type.get();
+  // LIVECONNECT_COMPLIANCE: Existing behavior is to return null for
+  // multi-dimensional arrays. Spec requires handling multi-demensional arrays.
+  if (target_inner_type.type == JavaType::TypeArray) {
+    return NULL;
+  }
+
+  // LIVECONNECT_COMPLIANCE: Existing behavior is to return null for object
+  // arrays. Spec requires handling object arrays.
+  if (target_inner_type.type == JavaType::TypeObject) {
+    return NULL;
+  }
+
+  // If the object does not have a length property, return null.
+  NPVariant length_variant;
+  if (!WebBindings::getProperty(0, object,
+                                WebBindings::getStringIdentifier("length"),
+                                &length_variant)) {
+    WebBindings::releaseVariantValue(&length_variant);
+    return NULL;
+  }
+
+  // If the length property does not have numeric type, or is outside the valid
+  // range for a Java array length, return null.
+  jsize length = -1;
+  if (NPVARIANT_IS_INT32(length_variant)
+      && NPVARIANT_TO_INT32(length_variant) >= 0) {
+    length = NPVARIANT_TO_INT32(length_variant);
+  } else if (NPVARIANT_IS_DOUBLE(length_variant)
+             && NPVARIANT_TO_DOUBLE(length_variant) >= 0.0
+             && NPVARIANT_TO_DOUBLE(length_variant) <= kint32max) {
+    length = static_cast<jsize>(NPVARIANT_TO_DOUBLE(length_variant));
+  }
+  WebBindings::releaseVariantValue(&length_variant);
+  if (length == -1) {
+    return NULL;
+  }
+
+  // Create the Java array. Note that we don't explicitly release the local
+  // ref to the result or any of its elements.
+  // TODO(steveblock): Handle failure to create the array.
+  jobject result = CreateJavaArray(target_inner_type, length);
+  NPVariant value_variant;
+  for (jsize i = 0; i < length; ++i) {
+    // It seems that getProperty() will set the variant to type void on failure,
+    // but this doesn't seem to be documented, so do it explicitly here for
+    // safety.
+    VOID_TO_NPVARIANT(value_variant);
+    // If this fails, for example due to a missing element, we simply treat the
+    // value as JavaScript undefined.
+    WebBindings::getProperty(0, object, WebBindings::getIntIdentifier(i),
+                             &value_variant);
+    SetArrayElement(result, target_inner_type, i,
+                    CoerceJavaScriptValueToJavaValue(value_variant,
+                                                     target_inner_type,
+                                                     false));
+    WebBindings::releaseVariantValue(&value_variant);
+  }
+
+  return result;
+}
+
 jvalue CoerceJavaScriptObjectToJavaValue(const NPVariant& variant,
-                                         const JavaType& target_type) {
+                                         const JavaType& target_type,
+                                         bool coerce_to_string) {
   // This covers both JavaScript objects (including arrays) and Java objects.
   // See http://jdk6.java.net/plugin2/liveconnect/#JS_OTHER_OBJECTS,
   // http://jdk6.java.net/plugin2/liveconnect/#JS_ARRAY_VALUES and
@@ -387,7 +553,9 @@ jvalue CoerceJavaScriptObjectToJavaValue(const NPVariant& variant,
     case JavaType::TypeString:
       // LIVECONNECT_COMPLIANCE: Existing behavior is to convert to
       // "undefined". Spec requires calling toString() on the Java object.
-      result.l = ConvertUTF8ToJavaString(AttachCurrentThread(), "undefined");
+      result.l = coerce_to_string ?
+          ConvertUTF8ToJavaString(AttachCurrentThread(), "undefined") :
+          NULL;
       break;
     case JavaType::TypeByte:
     case JavaType::TypeShort:
@@ -413,9 +581,7 @@ jvalue CoerceJavaScriptObjectToJavaValue(const NPVariant& variant,
         // requires raising a JavaScript exception.
         result.l = NULL;
       } else {
-        // TODO(steveblock): Handle converting JavaScript objects to Java
-        // arrays.
-        result.l = NULL;
+        result.l = CoerceJavaScriptObjectToArray(variant, target_type);
       }
       break;
     case JavaType::TypeVoid:
@@ -427,7 +593,8 @@ jvalue CoerceJavaScriptObjectToJavaValue(const NPVariant& variant,
 }
 
 jvalue CoerceJavaScriptNullOrUndefinedToJavaValue(const NPVariant& variant,
-                                                  const JavaType& target_type) {
+                                                  const JavaType& target_type,
+                                                  bool coerce_to_string) {
   // See http://jdk6.java.net/plugin2/liveconnect/#JS_NULL.
   DCHECK(variant.type == NPVariantType_Null ||
          variant.type == NPVariantType_Void);
@@ -437,13 +604,11 @@ jvalue CoerceJavaScriptNullOrUndefinedToJavaValue(const NPVariant& variant,
       result.l = NULL;
       break;
     case JavaType::TypeString:
-      if (variant.type == NPVariantType_Void) {
-        // LIVECONNECT_COMPLIANCE: Existing behavior is to convert undefined to
-        // "undefined". Spec requires converting undefined to NULL.
-        result.l =  ConvertUTF8ToJavaString(AttachCurrentThread(), "undefined");
-      } else {
-        result.l = NULL;
-      }
+      // LIVECONNECT_COMPLIANCE: Existing behavior is to convert undefined to
+      // "undefined". Spec requires converting undefined to NULL.
+      result.l = (coerce_to_string && variant.type == NPVariantType_Void) ?
+          ConvertUTF8ToJavaString(AttachCurrentThread(), "undefined") :
+          NULL;
       break;
     case JavaType::TypeByte:
     case JavaType::TypeChar:
@@ -472,8 +637,13 @@ jvalue CoerceJavaScriptNullOrUndefinedToJavaValue(const NPVariant& variant,
   return result;
 }
 
+// coerce_to_string means that we should try to coerce all JavaScript values to
+// strings when required, rather than simply converting to NULL. This is used
+// to maintain current behaviour, which differs slightly depending upon whether
+// or not the coercion in question is for an array element.
 jvalue CoerceJavaScriptValueToJavaValue(const NPVariant& variant,
-                                        const JavaType& target_type) {
+                                        const JavaType& target_type,
+                                        bool coerce_to_string) {
   // Note that in all these conversions, the relevant field of the jvalue must
   // always be explicitly set, as jvalue does not initialize its fields.
 
@@ -483,16 +653,20 @@ jvalue CoerceJavaScriptValueToJavaValue(const NPVariant& variant,
   switch (variant.type) {
     case NPVariantType_Int32:
     case NPVariantType_Double:
-      return CoerceJavaScriptNumberToJavaValue(variant, target_type);
+      return CoerceJavaScriptNumberToJavaValue(variant, target_type,
+                                               coerce_to_string);
     case NPVariantType_Bool:
-      return CoerceJavaScriptBooleanToJavaValue(variant, target_type);
+      return CoerceJavaScriptBooleanToJavaValue(variant, target_type,
+                                                coerce_to_string);
     case NPVariantType_String:
       return CoerceJavaScriptStringToJavaValue(variant, target_type);
     case NPVariantType_Object:
-      return CoerceJavaScriptObjectToJavaValue(variant, target_type);
+      return CoerceJavaScriptObjectToJavaValue(variant, target_type,
+                                               coerce_to_string);
     case NPVariantType_Null:
     case NPVariantType_Void:
-      return CoerceJavaScriptNullOrUndefinedToJavaValue(variant, target_type);
+      return CoerceJavaScriptNullOrUndefinedToJavaValue(variant, target_type,
+                                                        coerce_to_string);
   }
   NOTREACHED();
   return jvalue();
@@ -564,7 +738,8 @@ bool JavaBoundObject::Invoke(const std::string& name, const NPVariant* args,
   std::vector<jvalue> parameters(arg_count);
   for (size_t i = 0; i < arg_count; ++i) {
     parameters[i] = CoerceJavaScriptValueToJavaValue(args[i],
-                                                     method->parameter_type(i));
+                                                     method->parameter_type(i),
+                                                     true);
   }
 
   // Call
diff --git a/content/browser/renderer_host/java/java_method.cc b/content/browser/renderer_host/java/java_method.cc
index 1da69c4..fb4ef80 100644
--- a/content/browser/renderer_host/java/java_method.cc
+++ b/content/browser/renderer_host/java/java_method.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -73,15 +73,20 @@ std::string BinaryNameToJNIName(const std::string& binary_name,
       return "D";
     case JavaType::TypeVoid:
       return "V";
-    case JavaType::TypeArray:
-      return "[";
-    default:
-      DCHECK(type->type == JavaType::TypeString ||
-             type->type == JavaType::TypeObject);
+    case JavaType::TypeArray: {
+      // For array types, the binary name uses the JNI name encodings.
+      std::string jni_name = binary_name;
+      ReplaceSubstringsAfterOffset(&jni_name, 0, ".", "/");
+      return jni_name;
+    }
+    case JavaType::TypeString:
+    case JavaType::TypeObject:
       std::string jni_name = "L" + binary_name + ";";
       ReplaceSubstringsAfterOffset(&jni_name, 0, ".", "/");
       return jni_name;
   }
+  NOTREACHED();
+  return EmptyString();
 }
 
 }  // namespace
diff --git a/content/browser/renderer_host/java/java_type.cc b/content/browser/renderer_host/java/java_type.cc
index f3ada3c..9821873 100644
--- a/content/browser/renderer_host/java/java_type.cc
+++ b/content/browser/renderer_host/java/java_type.cc
@@ -1,11 +1,84 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
 #include "content/browser/renderer_host/java/java_type.h"
 
+#include "base/logging.h"
+
+namespace {
+
+JavaType JavaTypeFromJNIName(const std::string& jni_name) {
+  JavaType result;
+  DCHECK(!jni_name.empty());
+  switch (jni_name[0]) {
+    case 'Z':
+      result.type = JavaType::TypeBoolean;
+      break;
+    case 'B':
+      result.type = JavaType::TypeByte;
+      break;
+    case 'C':
+      result.type = JavaType::TypeChar;
+      break;
+    case 'S':
+      result.type = JavaType::TypeShort;
+      break;
+    case 'I':
+      result.type = JavaType::TypeInt;
+      break;
+    case 'J':
+      result.type = JavaType::TypeLong;
+      break;
+    case 'F':
+      result.type = JavaType::TypeFloat;
+      break;
+    case 'D':
+      result.type = JavaType::TypeDouble;
+      break;
+    case '[':
+      result.type = JavaType::TypeArray;
+      // LIVECONNECT_COMPLIANCE: We don't support multi-dimensional arrays, so
+      // there's no need to populate the inner types.
+      break;
+    case 'L':
+      result.type = jni_name == "Ljava.lang.String;" ?
+                    JavaType::TypeString :
+                    JavaType::TypeObject;
+      break;
+    default:
+      // Includes void (V).
+      NOTREACHED();
+  }
+  return result;
+}
+
+}  // namespace
+
+JavaType::JavaType() {
+}
+
+JavaType::JavaType(const JavaType& other) {
+  *this = other;
+}
+
+JavaType::~JavaType() {
+}
+
+JavaType& JavaType::operator=(const JavaType& other) {
+  type = other.type;
+  if (other.inner_type.get()) {
+    DCHECK_EQ(JavaType::TypeArray, type);
+    inner_type.reset(new JavaType(*other.inner_type));
+  } else {
+    inner_type.reset();
+  }
+  return *this;
+}
+
 JavaType JavaType::CreateFromBinaryName(const std::string& binary_name) {
   JavaType result;
+  DCHECK(!binary_name.empty());
   if (binary_name == "boolean") {
     result.type = JavaType::TypeBoolean;
   } else if (binary_name == "byte") {
@@ -26,6 +99,9 @@ JavaType JavaType::CreateFromBinaryName(const std::string& binary_name) {
     result.type = JavaType::TypeVoid;
   } else if (binary_name[0] == '[') {
     result.type = JavaType::TypeArray;
+    // The inner type of an array is represented in JNI format.
+    result.inner_type.reset(new JavaType(JavaTypeFromJNIName(
+        binary_name.substr(1))));
   } else if (binary_name == "java.lang.String") {
     result.type = JavaType::TypeString;
   } else {
diff --git a/content/browser/renderer_host/java/java_type.h b/content/browser/renderer_host/java/java_type.h
index f4975c2..f9cc4a20 100644
--- a/content/browser/renderer_host/java/java_type.h
+++ b/content/browser/renderer_host/java/java_type.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -7,7 +7,16 @@
 
 #include <string>
 
+#include "base/memory/scoped_ptr.h"
+
+// The type of a Java value. A light-weight enum-like structure intended for
+// use by value and in STL containers.
 struct JavaType {
+  JavaType();
+  JavaType(const JavaType& other);
+  ~JavaType();
+  JavaType& operator=(const JavaType& other);
+
   // Java's reflection API represents types as a string using an extended
   // 'binary name'.
   static JavaType CreateFromBinaryName(const std::string& binary_name);
@@ -31,6 +40,7 @@ struct JavaType {
   };
 
   Type type;
+  scoped_ptr<JavaType> inner_type;  // Used for TypeArray only.
 };
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_JAVA_JAVA_TYPE_H_
diff --git a/content/renderer/java/java_bridge_channel.cc b/content/renderer/java/java_bridge_channel.cc
index 103e177..f1ecebc 100644
--- a/content/renderer/java/java_bridge_channel.cc
+++ b/content/renderer/java/java_bridge_channel.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -6,6 +6,7 @@
 
 #include "content/common/child_process.h"
 #include "content/common/java_bridge_messages.h"
+#include "content/common/plugin_messages.h"
 
 JavaBridgeChannel* JavaBridgeChannel::GetJavaBridgeChannel(
     const IPC::ChannelHandle& channel_handle,
@@ -29,3 +30,17 @@ int JavaBridgeChannel::GenerateRouteID() {
   DCHECK_NE(MSG_ROUTING_NONE, route_id);
   return route_id;
 }
+
+bool JavaBridgeChannel::OnControlMessageReceived(const IPC::Message& msg) {
+  // We need to intercept these two message types because the default
+  // implementation of NPChannelBase::OnControlMessageReceived() is to
+  // DCHECK(false). However, we don't need to do anything, as we don't need to
+  // worry about the window system hanging when a modal dialog is displayed.
+  // This is because, unlike in the case of plugins, the host does not need to
+  // pump the message queue to avoid hangs.
+  if (msg.type() == PluginMsg_SignalModalDialogEvent::ID ||
+      msg.type() == PluginMsg_ResetModalDialogEvent::ID) {
+    return true;
+  }
+  return NPChannelBase::OnControlMessageReceived(msg);
+}
diff --git a/content/renderer/java/java_bridge_channel.h b/content/renderer/java/java_bridge_channel.h
index 4114a9f..7bf500c 100644
--- a/content/renderer/java/java_bridge_channel.h
+++ b/content/renderer/java/java_bridge_channel.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -19,6 +19,9 @@ class JavaBridgeChannel : public NPChannelBase {
   // NPChannelBase implementation:
   virtual int GenerateRouteID() OVERRIDE;
 
+  // NPChannelBase override:
+  virtual bool OnControlMessageReceived(const IPC::Message& msg) OVERRIDE;
+
  private:
   JavaBridgeChannel() {}