Fix JSI isArray method to match JS Array.isArray

Summary:
There is currently an assumption that JSI arrays are always
`vm::JSArrays`, and thus JSI method `isArray` only return true if it is
an `vm::JSArray` .

However, the JSI spec states `isArray` should follow JS `Array.isArray`.
This diff fixes this and treats `JSI::Array` as normal objects.

Reviewed By: tmikov

Differential Revision: D67408956

fbshipit-source-id: 1b61dd500b2b3bdab4771663672c375d7ed1b7d6

Author: tsaichien

API/hermes/hermes.cpp

@@ -494,11 +494,6 @@ class HermesRuntimeImpl final : public HermesRuntime,
     return ::hermes::vm::Handle<::hermes::vm::JSObject>::vmcast(&phv(obj));
   }
 
-  static ::hermes::vm::Handle<::hermes::vm::JSArray> arrayHandle(
-      const jsi::Array &arr) {
-    return ::hermes::vm::Handle<::hermes::vm::JSArray>::vmcast(&phv(arr));
-  }
-
   static ::hermes::vm::Handle<::hermes::vm::JSArrayBuffer> arrayBufferHandle(
       const jsi::ArrayBuffer &arr) {
     return ::hermes::vm::Handle<::hermes::vm::JSArrayBuffer>::vmcast(&phv(arr));
@@ -2192,7 +2187,12 @@ void HermesRuntimeImpl::setPropertyValue(
 }
 
 bool HermesRuntimeImpl::isArray(const jsi::Object &obj) const {
-  return vm::vmisa<vm::JSArray>(phv(obj));
+  if (vm::vmisa<vm::JSArray>(phv(obj))) {
+    return true;
+  }
+  auto cr = vm::isArray(runtime_, vm::vmcast<vm::JSObject>(phv(obj)));
+  const_cast<HermesRuntimeImpl *>(this)->checkStatus(cr.getStatus());
+  return *cr;
 }
 
 bool HermesRuntimeImpl::isArrayBuffer(const jsi::Object &obj) const {
@@ -2291,7 +2291,26 @@ jsi::ArrayBuffer HermesRuntimeImpl::createArrayBuffer(
 }
 
 size_t HermesRuntimeImpl::size(const jsi::Array &arr) {
-  return vm::JSArray::getLength(*arrayHandle(arr), runtime_);
+  if (LLVM_LIKELY(vm::vmisa<vm::JSArray>(phv(arr)))) {
+    return vm::JSArray::getLength(vm::vmcast<vm::JSArray>(phv(arr)), runtime_);
+  }
+
+  vm::GCScope gcScope(runtime_);
+  struct : vm::Locals {
+    vm::PinnedValue<> lenProp;
+  } lv;
+  vm::LocalsRAII lraii{runtime_, &lv};
+  auto cr = vm::JSObject::getNamed_RJS(
+      handle(arr),
+      runtime_,
+      vm::Predefined::getSymbolID(vm::Predefined::length));
+  checkStatus(cr.getStatus());
+
+  lv.lenProp = std::move(*cr);
+  auto lenRes = toLength(runtime_, lv.lenProp);
+  checkStatus(lenRes.getStatus());
+
+  return lenRes->getNumber();
 }
 
 size_t HermesRuntimeImpl::size(const jsi::ArrayBuffer &arr) {
@@ -2316,7 +2335,7 @@ jsi::Value HermesRuntimeImpl::getValueAtIndex(const jsi::Array &arr, size_t i) {
   }
 
   auto res = vm::JSObject::getComputed_RJS(
-      arrayHandle(arr),
+      handle(arr),
       runtime_,
       runtime_.makeHandle(vm::HermesValue::encodeTrustedNumberValue(i)));
   checkStatus(res.getStatus());
@@ -2335,7 +2354,7 @@ void HermesRuntimeImpl::setValueAtIndexImpl(
   }
 
   auto res = vm::JSObject::putComputed_RJS(
-      arrayHandle(arr),
+      handle(arr),
       runtime_,
       runtime_.makeHandle(vm::HermesValue::encodeTrustedNumberValue(i)),
       vmHandleFromValue(value));

unittests/API/APITest.cpp

@@ -927,6 +927,24 @@ assert(arrayEqual(symArr, [abcSym, hoDefSym, defSym, numberSym]),
 )");
 }
 
+TEST_P(HermesRuntimeTest, ArrayTest) {
+  auto array = eval("[1, 2, 3]").getObject(*rt);
+  EXPECT_TRUE(array.isArray(*rt));
+  auto jsiArray = array.getArray(*rt);
+  EXPECT_EQ(jsiArray.size(*rt), 3);
+  EXPECT_EQ(jsiArray.getValueAtIndex(*rt, 0).asNumber(), 1);
+  jsiArray.setValueAtIndex(*rt, 1, 0);
+  EXPECT_EQ(jsiArray.getValueAtIndex(*rt, 1).asNumber(), 0);
+
+  array = eval("new Proxy([4, 5, 6], {})").getObject(*rt);
+  EXPECT_TRUE(array.isArray(*rt));
+  jsiArray = array.getArray(*rt);
+  EXPECT_EQ(jsiArray.size(*rt), 3);
+  EXPECT_EQ(jsiArray.getValueAtIndex(*rt, 0).asNumber(), 4);
+  jsiArray.setValueAtIndex(*rt, 1, 0);
+  EXPECT_EQ(jsiArray.getValueAtIndex(*rt, 1).asNumber(), 0);
+}
+
 TEST_P(HermesRuntimeTest, HasComputedTest) {
   // The only use of JSObject::hasComputed() is in HermesRuntimeImpl,
   // so we test its Proxy support here, instead of from JS.