Return the true size for ArrayBuffers

Summary:
X-link: facebook/react-native#39084

The current behaviour of `jsi::ArrayBuffer::size` with Hermes is to
return the value of the `byteLength` property. While this is
superficially more consistent with the behaviour for `Array`, which
reads the length property, it is a problem in practice since the
`byteLength` property is easily overridden in JS, making it unsafe
to use when indexing into the backing storage.

While this is a behaviour change, it should be fine because:

1. This is already the behaviour with JSC (the newly added test also
runs against the JSC implementation) and also seems to be the behaviour
of Microsoft's [V8 implementation](https://github.com/microsoft/v8-jsi/blob/db1ca30d6e5d39ee3a88a9f81b258c3825735917/src/V8JsiRuntime.cpp#L1239C48-L1239C58).
2. The current behaviour is counterintuitive and likely to lead to
bugs.

Changelog: [Internal]

Reviewed By: tmikov

Differential Revision: D48226661

fbshipit-source-id: 043d60deeb9dc189705594415f4f262f7de1095a

API/hermes/hermes.cpp

@@ -690,8 +690,6 @@ class HermesRuntimeImpl final : public HermesRuntime,
   void checkStatus(vm::ExecutionStatus);
   vm::HermesValue stringHVFromAscii(const char *ascii, size_t length);
   vm::HermesValue stringHVFromUtf8(const uint8_t *utf8, size_t length);
-  size_t getLength(vm::Handle<vm::ArrayImpl> arr);
-  size_t getByteLength(vm::Handle<vm::JSArrayBuffer> arr);
 
   struct JsiProxy final : public vm::HostObjectProxy {
     HermesRuntimeImpl &rt_;
@@ -2004,8 +2002,10 @@ size_t HermesRuntimeImpl::size(const jsi::Array &arr) {
 }
 
 size_t HermesRuntimeImpl::size(const jsi::ArrayBuffer &arr) {
-  vm::GCScope gcScope(runtime_);
-  return getByteLength(arrayBufferHandle(arr));
+  auto ab = arrayBufferHandle(arr);
+  if (LLVM_UNLIKELY(!ab->attached()))
+    throw jsi::JSINativeException("ArrayBuffer is detached.");
+  return ab->size();
 }
 
 uint8_t *HermesRuntimeImpl::data(const jsi::ArrayBuffer &arr) {
@@ -2279,17 +2279,6 @@ vm::HermesValue HermesRuntimeImpl::stringHVFromUtf8(
   return *strRes;
 }
 
-size_t HermesRuntimeImpl::getByteLength(vm::Handle<vm::JSArrayBuffer> arr) {
-  auto res = vm::JSObject::getNamed_RJS(
-      arr, runtime_, vm::Predefined::getSymbolID(vm::Predefined::byteLength));
-  checkStatus(res.getStatus());
-  if (!(*res)->isNumber()) {
-    throw jsi::JSError(
-        *this, "getLength: property 'byteLength' is not a number");
-  }
-  return static_cast<size_t>((*res)->getDouble());
-}
-
 namespace {
 
 class HermesMutex : public std::recursive_mutex {

API/jsi/jsi/jsi.h

@@ -943,7 +943,8 @@ class JSI_EXPORT ArrayBuffer : public Object {
   ArrayBuffer(Runtime& runtime, std::shared_ptr<MutableBuffer> buffer)
       : ArrayBuffer(runtime.createArrayBuffer(std::move(buffer))) {}
 
-  /// \return the size of the ArrayBuffer, according to its byteLength property.
+  /// \return the size of the ArrayBuffer storage. This is not affected by
+  /// overriding the byteLength property.
   /// (C++ naming convention)
   size_t size(Runtime& runtime) const {
     return runtime.size(*this);

API/jsi/jsi/test/testlib.cpp

@@ -1439,6 +1439,18 @@ TEST_P(JSITest, MultilevelDecoratedHostObject) {
   EXPECT_EQ(1, RD2::numGets);
 }
 
+TEST_P(JSITest, ArrayBufferSizeTest) {
+  auto ab =
+      eval("var x = new ArrayBuffer(10); x").getObject(rt).getArrayBuffer(rt);
+  EXPECT_EQ(ab.size(rt), 10);
+  // Ensure we can safely write some data to the buffer.
+  memset(ab.data(rt), 0xab, 10);
+
+  // Ensure that setting the byteLength property does not change the length.
+  eval("Object.defineProperty(x, 'byteLength', {value: 20})");
+  EXPECT_EQ(ab.size(rt), 10);
+}
+
 INSTANTIATE_TEST_CASE_P(
     Runtimes,
     JSITest,