Fix the wrong conversion of Int8Array and Uint{16, 32}Array js value to Java value

This CL fixes problems that -1 of Int8Array is converted to 255 since it
is the casting as the char type and that a maximum value of
Uint{16, 32}Arrays are converted to -1 since they are the casting as the
signed types in TypedArraySerializerImpl.
For Uint32Array, this CL serializes/deserializes the value as the binary
type since base::Value supports the only int for the integer type.

Bug: 1148997
Change-Id: I52af228070606e4cd3f293dd80e4f0ca53b6b082
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2589535Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Shimi Zhang <ctzsm@chromium.org>
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Cr-Commit-Position: refs/heads/master@{#841033}

content/browser/android/java/gin_java_script_to_java_types_coercion.cc

@@ -67,7 +67,7 @@ jint RoundDoubleToInt(const double& x) {
 }
 
 jvalue CoerceJavaScriptIntegerToJavaValue(JNIEnv* env,
-                                          const base::Value* value,
+                                          int64_t integer_value,
                                           const JavaType& target_type,
                                           bool coerce_to_string,
                                           GinJavaBridgeError* error) {
@@ -78,29 +78,27 @@ jvalue CoerceJavaScriptIntegerToJavaValue(JNIEnv* env,
   // all but the lowest n buts, where n is the number of bits in the target
   // type.
   jvalue result;
-  int int_value;
-  value->GetAsInteger(&int_value);
   switch (target_type.type) {
     case JavaType::TypeByte:
-      result.b = static_cast<jbyte>(int_value);
+      result.b = static_cast<jbyte>(integer_value);
       break;
     case JavaType::TypeChar:
-      result.c = static_cast<jchar>(int_value);
+      result.c = static_cast<jchar>(integer_value);
       break;
     case JavaType::TypeShort:
-      result.s = static_cast<jshort>(int_value);
+      result.s = static_cast<jshort>(integer_value);
       break;
     case JavaType::TypeInt:
-      result.i = int_value;
+      result.i = static_cast<jint>(integer_value);
       break;
     case JavaType::TypeLong:
-      result.j = int_value;
+      result.j = integer_value;
       break;
     case JavaType::TypeFloat:
-      result.f = int_value;
+      result.f = integer_value;
       break;
     case JavaType::TypeDouble:
-      result.d = int_value;
+      result.d = integer_value;
       break;
     case JavaType::TypeObject:
       // LIVECONNECT_COMPLIANCE: Existing behavior is to convert to null. Spec
@@ -108,10 +106,11 @@ jvalue CoerceJavaScriptIntegerToJavaValue(JNIEnv* env,
       result.l = NULL;
       break;
     case JavaType::TypeString:
-      result.l = coerce_to_string ? ConvertUTF8ToJavaString(
-                                        env, base::NumberToString(int_value))
-                                        .Release()
-                                  : NULL;
+      result.l = coerce_to_string
+                     ? ConvertUTF8ToJavaString(
+                           env, base::NumberToString(integer_value))
+                           .Release()
+                     : NULL;
       break;
     case JavaType::TypeBoolean:
       // LIVECONNECT_COMPLIANCE: Existing behavior is to convert to false. Spec
@@ -663,14 +662,22 @@ jvalue CoerceGinJavaBridgeValueToJavaValue(JNIEnv* env,
       return CoerceJavaScriptNullOrUndefinedToJavaValue(
           env, value, target_type, coerce_to_string, error);
     case GinJavaBridgeValue::TYPE_NONFINITE: {
-      float float_value;
-      gin_value->GetAsNonFinite(&float_value);
+      float float_value = 0.f;
+      if (!gin_value->GetAsNonFinite(&float_value))
+        return jvalue();
       return CoerceJavaScriptDoubleToJavaValue(
           env, float_value, target_type, coerce_to_string, error);
     }
     case GinJavaBridgeValue::TYPE_OBJECT_ID:
       return CoerceJavaScriptObjectToJavaValue(
           env, value, target_type, coerce_to_string, object_refs, error);
+    case GinJavaBridgeValue::TYPE_UINT32: {
+      uint32_t uint32_value = 0;
+      if (!gin_value->GetAsUInt32(&uint32_value))
+        return jvalue();
+      return CoerceJavaScriptIntegerToJavaValue(env, uint32_value, target_type,
+                                                coerce_to_string, error);
+    }
     default:
       NOTREACHED();
   }
@@ -702,7 +709,7 @@ jvalue CoerceJavaScriptValueToJavaValue(JNIEnv* env,
   switch (value->type()) {
     case base::Value::Type::INTEGER:
       return CoerceJavaScriptIntegerToJavaValue(
-          env, value, target_type, coerce_to_string, error);
+          env, value->GetInt(), target_type, coerce_to_string, error);
     case base::Value::Type::DOUBLE: {
       double double_value;
       value->GetAsDouble(&double_value);

content/common/android/gin_java_bridge_value.cc

@@ -55,6 +55,14 @@ std::unique_ptr<base::Value> GinJavaBridgeValue::CreateObjectIDValue(
   return gin_value.SerializeToBinaryValue();
 }
 
+// static
+std::unique_ptr<base::Value> GinJavaBridgeValue::CreateUInt32Value(
+    uint32_t in_value) {
+  GinJavaBridgeValue gin_value(TYPE_UINT32);
+  gin_value.pickle_.WriteUInt32(in_value);
+  return gin_value.SerializeToBinaryValue();
+}
+
 // static
 bool GinJavaBridgeValue::ContainsGinJavaBridgeValue(const base::Value* value) {
   if (!value->is_blob())
@@ -106,6 +114,15 @@ bool GinJavaBridgeValue::GetAsObjectID(int32_t* out_object_id) const {
   }
 }
 
+bool GinJavaBridgeValue::GetAsUInt32(uint32_t* out_value) const {
+  if (GetType() == TYPE_UINT32) {
+    base::PickleIterator iter(pickle_);
+    return iter.ReadUInt32(out_value);
+  } else {
+    return false;
+  }
+}
+
 GinJavaBridgeValue::GinJavaBridgeValue(Type type) :
     pickle_(sizeof(Header)) {
   Header* header = pickle_.headerT<Header>();

content/common/android/gin_java_bridge_value.h

@@ -30,6 +30,8 @@ class GinJavaBridgeValue {
     TYPE_NONFINITE,
     // Bridge Object ID
     TYPE_OBJECT_ID,
+    // Uint32 type
+    TYPE_UINT32,
     TYPE_LAST_VALUE
   };
 
@@ -41,6 +43,8 @@ class GinJavaBridgeValue {
       double in_value);
   CONTENT_EXPORT static std::unique_ptr<base::Value> CreateObjectIDValue(
       int32_t in_value);
+  CONTENT_EXPORT static std::unique_ptr<base::Value> CreateUInt32Value(
+      uint32_t in_value);
 
   // De-serialization
   CONTENT_EXPORT static bool ContainsGinJavaBridgeValue(
@@ -53,6 +57,7 @@ class GinJavaBridgeValue {
 
   CONTENT_EXPORT bool GetAsNonFinite(float* out_value) const;
   CONTENT_EXPORT bool GetAsObjectID(int32_t* out_object_id) const;
+  CONTENT_EXPORT bool GetAsUInt32(uint32_t* out_value) const;
 
  private:
   explicit GinJavaBridgeValue(Type type);

content/common/android/gin_java_bridge_value_unittest.cc

@@ -76,6 +76,18 @@ TEST_F(GinJavaBridgeValueTest, BasicValues) {
   EXPECT_EQ(42, native_object_id);
 
   EXPECT_FALSE(undefined_value->GetAsNonFinite(&native_float));
+
+  std::unique_ptr<base::Value> in_uint32(GinJavaBridgeValue::CreateUInt32Value(
+      std::numeric_limits<uint32_t>::max()));
+  ASSERT_TRUE(in_uint32.get());
+  EXPECT_TRUE(GinJavaBridgeValue::ContainsGinJavaBridgeValue(in_uint32.get()));
+  std::unique_ptr<const GinJavaBridgeValue> uint32_value(
+      GinJavaBridgeValue::FromValue(in_uint32.get()));
+  ASSERT_TRUE(uint32_value.get());
+  EXPECT_TRUE(uint32_value->IsType(GinJavaBridgeValue::TYPE_UINT32));
+  uint32_t out_uint32_value;
+  EXPECT_TRUE(uint32_value->GetAsUInt32(&out_uint32_value));
+  EXPECT_EQ(std::numeric_limits<uint32_t>::max(), out_uint32_value);
 }
 
 TEST_F(GinJavaBridgeValueTest, BrokenValues) {

content/public/android/javatests/src/org/chromium/content/browser/JavaBridgeArrayCoercionTest.java

@@ -745,6 +745,38 @@ public class JavaBridgeArrayCoercionTest {
         Assert.assertNull(mTestObject.waitForCustomTypeArray());
     }
 
+    // Test passing a typed Int8Array to a method which takes a Java array.
+    @Test
+    @SmallTest
+    @Feature({"AndroidWebView", "Android-JavaBridge"})
+    @UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
+    public void testPassInt8ArrayWithNagativeValue(boolean useMojo) throws Throwable {
+        mActivityTestRule.executeJavaScript("buffer = new ArrayBuffer(1);");
+        mActivityTestRule.executeJavaScript("int8_array = new Int8Array(buffer);");
+        mActivityTestRule.executeJavaScript("int8_array[0] = -1;");
+
+        mActivityTestRule.executeJavaScript("testObject.setByteArray(int8_array);");
+        Assert.assertEquals(-1, mTestObject.waitForByteArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setCharArray(int8_array);");
+        Assert.assertEquals(65535, mTestObject.waitForCharArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setShortArray(int8_array);");
+        Assert.assertEquals(-1, mTestObject.waitForShortArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setIntArray(int8_array);");
+        Assert.assertEquals(-1, mTestObject.waitForIntArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setLongArray(int8_array);");
+        Assert.assertEquals(-1L, mTestObject.waitForLongArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setFloatArray(int8_array);");
+        Assert.assertEquals(-1.0f, mTestObject.waitForFloatArray()[0], ASSERTION_DELTA);
+
+        mActivityTestRule.executeJavaScript("testObject.setDoubleArray(int8_array);");
+        Assert.assertEquals(-1.0, mTestObject.waitForDoubleArray()[0], ASSERTION_DELTA);
+    }
+
     // Test passing a typed Uint8Array to a method which takes a Java array.
     @Test
     @SmallTest
@@ -789,6 +821,37 @@ public class JavaBridgeArrayCoercionTest {
         Assert.assertNull(mTestObject.waitForCustomTypeArray());
     }
 
+    @Test
+    @SmallTest
+    @Feature({"AndroidWebView", "Android-JavaBridge"})
+    @UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
+    public void testPassUint8ArrayWithMaxValue(boolean useMojo) throws Throwable {
+        mActivityTestRule.executeJavaScript("buffer = new ArrayBuffer(1);");
+        mActivityTestRule.executeJavaScript("uint8_array = new Uint8Array(buffer);");
+        mActivityTestRule.executeJavaScript("uint8_array[0] = 255;");
+
+        mActivityTestRule.executeJavaScript("testObject.setByteArray(uint8_array);");
+        Assert.assertEquals(-1, mTestObject.waitForByteArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setCharArray(uint8_array);");
+        Assert.assertEquals(255, mTestObject.waitForCharArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setShortArray(uint8_array);");
+        Assert.assertEquals(255, mTestObject.waitForShortArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setIntArray(uint8_array);");
+        Assert.assertEquals(255, mTestObject.waitForIntArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setLongArray(uint8_array);");
+        Assert.assertEquals(255L, mTestObject.waitForLongArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setFloatArray(uint8_array);");
+        Assert.assertEquals(255.0f, mTestObject.waitForFloatArray()[0], ASSERTION_DELTA);
+
+        mActivityTestRule.executeJavaScript("testObject.setDoubleArray(uint8_array);");
+        Assert.assertEquals(255.0, mTestObject.waitForDoubleArray()[0], ASSERTION_DELTA);
+    }
+
     // Test passing a typed Int16Array to a method which takes a Java array.
     @Test
     @SmallTest
@@ -877,6 +940,41 @@ public class JavaBridgeArrayCoercionTest {
         Assert.assertNull(mTestObject.waitForCustomTypeArray());
     }
 
+    // Test passing a typed Uint16Array of max values to a method which takes a Java array.
+    @Test
+    @SmallTest
+    @Feature({"AndroidWebView", "Android-JavaBridge"})
+    @UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
+    public void testPassUint16ArrayWithMaxValue(boolean useMojo) throws Throwable {
+        mActivityTestRule.executeJavaScript("buffer = new ArrayBuffer(2);");
+        mActivityTestRule.executeJavaScript("uint16_array = new Uint16Array(buffer);");
+        mActivityTestRule.executeJavaScript("uint16_array[0] = 65535;");
+
+        mActivityTestRule.executeJavaScript("testObject.setBooleanArray(uint16_array);");
+        Assert.assertFalse(mTestObject.waitForBooleanArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setByteArray(uint16_array);");
+        Assert.assertEquals(-1, mTestObject.waitForByteArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setCharArray(uint16_array);");
+        Assert.assertEquals(65535, mTestObject.waitForCharArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setShortArray(uint16_array);");
+        Assert.assertEquals(-1, mTestObject.waitForShortArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setIntArray(uint16_array);");
+        Assert.assertEquals(65535, mTestObject.waitForIntArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setLongArray(uint16_array);");
+        Assert.assertEquals(65535L, mTestObject.waitForLongArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setFloatArray(uint16_array);");
+        Assert.assertEquals(65535.0f, mTestObject.waitForFloatArray()[0], ASSERTION_DELTA);
+
+        mActivityTestRule.executeJavaScript("testObject.setDoubleArray(uint16_array);");
+        Assert.assertEquals(65535.0, mTestObject.waitForDoubleArray()[0], ASSERTION_DELTA);
+    }
+
     // Test passing a typed Int32Array to a method which takes a Java array.
     @Test
     @SmallTest
@@ -965,6 +1063,42 @@ public class JavaBridgeArrayCoercionTest {
         Assert.assertNull(mTestObject.waitForCustomTypeArray());
     }
 
+    // Test passing a typed Uint32Array of max values to a method which takes a Java array.
+    @Test
+    @SmallTest
+    @Feature({"AndroidWebView", "Android-JavaBridge"})
+    @UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
+    public void testPassUint32ArrayWithMaxValue(boolean useMojo) throws Throwable {
+        mActivityTestRule.executeJavaScript("buffer = new ArrayBuffer(4);");
+        mActivityTestRule.executeJavaScript("uint32_array = new Uint32Array(buffer);");
+        mActivityTestRule.executeJavaScript("uint32_array[0] = 4294967295;");
+
+        mActivityTestRule.executeJavaScript("testObject.setBooleanArray(uint32_array);");
+        Assert.assertFalse(mTestObject.waitForBooleanArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setByteArray(uint32_array);");
+        Assert.assertEquals(-1, mTestObject.waitForByteArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setCharArray(uint32_array);");
+        Assert.assertEquals(65535, mTestObject.waitForCharArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setShortArray(uint32_array);");
+        Assert.assertEquals(-1, mTestObject.waitForShortArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setIntArray(uint32_array);");
+        Assert.assertEquals(-1, mTestObject.waitForIntArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setLongArray(uint32_array);");
+        Assert.assertEquals(4294967295L, mTestObject.waitForLongArray()[0]);
+
+        mActivityTestRule.executeJavaScript("testObject.setFloatArray(uint32_array);");
+        Assert.assertEquals((new Long(4294967295L)).floatValue(),
+                mTestObject.waitForFloatArray()[0], ASSERTION_DELTA);
+
+        mActivityTestRule.executeJavaScript("testObject.setDoubleArray(uint32_array);");
+        Assert.assertEquals(4294967295.0, mTestObject.waitForDoubleArray()[0], ASSERTION_DELTA);
+    }
+
     // Test passing a typed Float32Array to a method which takes a Java array.
     @Test
     @SmallTest

content/renderer/java/gin_java_bridge_value_converter.cc

@@ -84,7 +84,14 @@ class TypedArraySerializerImpl : public TypedArraySerializer {
                      *end = element + typed_array_->Length();
          element != end;
          ++element) {
-      out->Append(std::make_unique<base::Value>(ListType(*element)));
+      // Serialize the uint32 value as the binary type since base::Value
+      // supports only int for the integer type, and the uint8 and the uint16
+      // with Base::Value since they fit into int.
+      if (std::is_same<ElementType, uint32_t>::value) {
+        out->Append(GinJavaBridgeValue::CreateUInt32Value(*element));
+      } else {
+        out->Append(std::make_unique<base::Value>(ListType(*element)));
+      }
     }
   }
 
@@ -100,14 +107,19 @@ class TypedArraySerializerImpl : public TypedArraySerializer {
 // static
 std::unique_ptr<TypedArraySerializer> TypedArraySerializer::Create(
     v8::Local<v8::TypedArray> typed_array) {
-  if (typed_array->IsInt8Array() ||
-      typed_array->IsUint8Array() ||
-      typed_array->IsUint8ClampedArray()) {
-    return TypedArraySerializerImpl<char, int>::Create(typed_array);
-  } else if (typed_array->IsInt16Array() || typed_array->IsUint16Array()) {
+  if (typed_array->IsInt8Array()) {
+    return TypedArraySerializerImpl<int8_t, int>::Create(typed_array);
+  } else if (typed_array->IsUint8Array() ||
+             typed_array->IsUint8ClampedArray()) {
+    return TypedArraySerializerImpl<uint8_t, int>::Create(typed_array);
+  } else if (typed_array->IsInt16Array()) {
     return TypedArraySerializerImpl<int16_t, int>::Create(typed_array);
-  } else if (typed_array->IsInt32Array() || typed_array->IsUint32Array()) {
+  } else if (typed_array->IsUint16Array()) {
+    return TypedArraySerializerImpl<uint16_t, int>::Create(typed_array);
+  } else if (typed_array->IsInt32Array()) {
     return TypedArraySerializerImpl<int32_t, int>::Create(typed_array);
+  } else if (typed_array->IsUint32Array()) {
+    return TypedArraySerializerImpl<uint32_t, int>::Create(typed_array);
   } else if (typed_array->IsFloat32Array()) {
     return TypedArraySerializerImpl<float, double>::Create(typed_array);
   } else if (typed_array->IsFloat64Array()) {

content/renderer/java/gin_java_bridge_value_converter_unittest.cc

@@ -137,9 +137,22 @@ TEST_F(GinJavaBridgeValueConverterTest, TypedArrays) {
     base::ListValue* list;
     ASSERT_TRUE(list_value->GetAsList(&list)) << typed_array_type;
     EXPECT_EQ(1u, list->GetSize()) << typed_array_type;
-    double first_element;
-    ASSERT_TRUE(list->GetDouble(0, &first_element)) << typed_array_type;
-    EXPECT_EQ(42.0, first_element) << typed_array_type;
+
+    const base::Value* value;
+    list->Get(0, &value);
+    if (value->type() == base::Value::Type::BINARY) {
+      std::unique_ptr<const GinJavaBridgeValue> gin_value(
+          GinJavaBridgeValue::FromValue(value));
+      EXPECT_EQ(gin_value->GetType(), GinJavaBridgeValue::TYPE_UINT32);
+      uint32_t first_element = 0;
+      ASSERT_TRUE(gin_value->GetAsUInt32(&first_element));
+      EXPECT_EQ(42u, first_element) << typed_array_type;
+
+    } else {
+      double first_element;
+      ASSERT_TRUE(value->GetAsDouble(&first_element)) << typed_array_type;
+      EXPECT_EQ(42.0, first_element) << typed_array_type;
+    }
   }
 }