Use NSData for the JS->ObjC ArrayBuffer argument path (#57596)

Summary:
Pull Request resolved: #57596

The original implementation used `NSMutableData` for both directions of the
JSI<->ObjC ArrayBuffer conversion. That is the wrong contract on the argument
(JS -> ObjC/Native) side: an inbound buffer is owned by the caller, not the
native module, and `NSMutableData` must own a resizable backing store it can
`realloc`/`free`, so it cannot durably alias a foreign buffer.

This diff keeps `NSMutableData` for the ObjC/Native -> JS (return) path, but
switches the JS -> ObjC/Native (argument) path to the immutable `NSData`:

- Codegen: `getParamObjCType` now maps `ArrayBufferTypeAnnotation` params to
  `NSData *` (return type stays `NSMutableData *`).
- Runtime: `convertJSIArrayBufferToNSMutableData` is renamed to
  `convertJSIArrayBufferToNSData` and returns an immutable `NSData`. The bytes
  are still eagerly copied, which keeps the result safe to retain, store, or
  dispatch to another thread regardless of whether the source bytes were owned
  by JS or by a native `MutableBuffer`.
- Updated the ObjC unit tests and the codegen `GenerateModuleHObjCpp` snapshot.

Changelog: [INTERNAL]

Reviewed By: javache

Differential Revision: D112582384

fbshipit-source-id: 8ed2dfb3f23e7f39677dfb1824c5530f378705bc

Author: christophpurrer

packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeMethod.js

@@ -221,7 +221,7 @@ function getParamObjCType(
       return notStruct(wrapOptional('NSArray *', !nullable));
     }
     case 'ArrayBufferTypeAnnotation': {
-      return notStruct(wrapOptional('NSMutableData *', !nullable));
+      return notStruct(wrapOptional('NSData *', !nullable));
     }
   }
 

packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleHObjCpp-test.js.snap

@@ -103,8 +103,8 @@ Map {
 @protocol NativeSampleTurboModuleSpec <RCTBridgeModule, RCTTurboModule>
 
 - (NSMutableData *)getArrayBuffer;
-- (void)voidArrayBuffer:(NSMutableData *)arg;
-- (void)voidNullableArrayBuffer:(NSMutableData * _Nullable)arg;
+- (void)voidArrayBuffer:(NSData *)arg;
+- (void)voidNullableArrayBuffer:(NSData * _Nullable)arg;
 - (void)promiseArrayBuffer:(RCTPromiseResolveBlock)resolve
                     reject:(RCTPromiseRejectBlock)reject;
 

packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm

@@ -123,12 +123,11 @@ - (void)testInvokeTurboModuleWithNull
   OCMVerify(OCMTimes(1), [instance_ testMethodWhichTakesObject:nil]);
 }
 
-// A JS ArrayBuffer converts to an NSMutableData that owns an independent copy of
-// the bytes — NSMutableData cannot alias a foreign buffer, so the result stays
-// valid and is safe to mutate after the source buffer is gone. This covers the
-// ArrayBuffer-backed-by-native-MutableBuffer case, which is the one that could in
-// principle have been aliased zero-copy.
-- (void)testArrayBufferConvertsToIndependentNSMutableData
+// A JS ArrayBuffer converts (on the argument path) to an immutable NSData that
+// owns an independent copy of the bytes, so the result stays valid after the
+// source buffer is gone. This covers the ArrayBuffer-backed-by-native-MutableBuffer
+// case, which is the one that could in principle have been aliased zero-copy.
+- (void)testArrayBufferConvertsToIndependentNSData
 {
   constexpr size_t kBufferSize = 64 * 1024;
 
@@ -142,15 +141,15 @@ - (void)testArrayBufferConvertsToIndependentNSMutableData
   id converted =
       TurboModuleConvertUtils::convertJSIValueToObjCObject(*rt, facebook::jsi::Value(*rt, arrayBuffer), nullptr);
 
-  XCTAssertTrue([converted isKindOfClass:[NSMutableData class]]);
-  NSMutableData *data = (NSMutableData *)converted;
+  XCTAssertTrue([converted isKindOfClass:[NSData class]]);
+  NSData *data = (NSData *)converted;
   XCTAssertEqual(data.length, (NSUInteger)kBufferSize);
   XCTAssertEqual(*static_cast<const uint8_t *>(data.bytes), 0xAB);
 
-  // Independent copy: mutating the NSMutableData must not write through to the
-  // source MutableBuffer.
-  *static_cast<uint8_t *>(data.mutableBytes) = 0xCD;
-  XCTAssertEqual(*buffer->data(), 0xAB, @"NSMutableData must not alias the source buffer");
+  // Independent copy: mutating the source MutableBuffer must not write through to
+  // the NSData.
+  *buffer->data() = 0xCD;
+  XCTAssertEqual(*static_cast<const uint8_t *>(data.bytes), 0xAB, @"NSData must not alias the source buffer");
 }
 
 @end

packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm

@@ -199,16 +199,15 @@ size_t size() const override
   };
 }
 
-// Copy the ArrayBuffer's bytes into an NSMutableData. A zero-copy wrap is not
-// possible here: NSMutableData needs its own resizable backing store and cannot
-// alias a foreign buffer (even via initWithBytesNoCopy:length:deallocator:, which
-// copies eagerly for the mutable subclass). Copying also makes the result safe to
-// retain in a block, store, or dispatch to another thread, regardless of whether
-// the bytes were owned by JS (valid only for this callstack) or by a native
-// MutableBuffer (which the JS ArrayBuffer may GC concurrently).
-static NSMutableData *convertJSIArrayBufferToNSMutableData(jsi::Runtime &rt, const jsi::ArrayBuffer &value)
+// Copy the ArrayBuffer's bytes into an immutable NSData. An inbound buffer is
+// owned by the caller, not the native module, so NSData (not NSMutableData) is
+// the correct read-only contract. Copying makes the NSData self-contained and
+// safe to retain in a block, store, or dispatch to another thread, regardless of
+// whether the bytes were owned by JS (valid only for this callstack) or by a
+// native MutableBuffer (which the JS ArrayBuffer may GC concurrently).
+static NSData *convertJSIArrayBufferToNSData(jsi::Runtime &rt, const jsi::ArrayBuffer &value)
 {
-  return [NSMutableData dataWithBytes:value.data(rt) length:value.size(rt)];
+  return [NSData dataWithBytes:value.data(rt) length:value.size(rt)];
 }
 
 id convertJSIValueToObjCObject(
@@ -241,7 +240,7 @@ id convertJSIValueToObjCObject(
       return convertJSIFunctionToCallback(runtime, o.getFunction(runtime), jsInvoker);
     }
     if (o.isArrayBuffer(runtime)) {
-      return convertJSIArrayBufferToNSMutableData(runtime, o.getArrayBuffer(runtime));
+      return convertJSIArrayBufferToNSData(runtime, o.getArrayBuffer(runtime));
     }
     return convertJSIObjectToNSDictionary(runtime, o, jsInvoker, useNSNull);
   }

packages/rn-tester/RNTesterUnitTests/RCTTurboModuleArrayBufferTests.mm

@@ -105,16 +105,17 @@ @implementation RCTTestArrayBufferTurboModule
 
 RCT_EXPORT_MODULE()
 
-RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSMutableData *, testMethodWhichMutatesArrayBuffer : (NSMutableData *)buffer)
+RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSMutableData *, testMethodWhichTransformsArrayBuffer : (NSData *)buffer)
 {
-  auto *bytes = static_cast<uint8_t *>(buffer.mutableBytes);
-  for (NSUInteger i = 0; i < buffer.length; ++i) {
+  NSMutableData *result = [buffer mutableCopy];
+  auto *bytes = static_cast<uint8_t *>(result.mutableBytes);
+  for (NSUInteger i = 0; i < result.length; ++i) {
     bytes[i] = static_cast<uint8_t>((i + 1) * 10);
   }
-  return buffer;
+  return result;
 }
 
-RCT_EXPORT_METHOD(testMethodWhichStoresArrayBuffer : (NSMutableData *)payload)
+RCT_EXPORT_METHOD(testMethodWhichStoresArrayBuffer : (NSData *)payload)
 {
   self.lastReceivedPayload = [payload copy];
 }
@@ -169,8 +170,8 @@ - (void)testSyncArrayBufferRoundTrip
   auto result = module.invokeObjCMethod(
       *rt,
       ArrayBufferKind,
-      "testMethodWhichMutatesArrayBuffer",
-      @selector(testMethodWhichMutatesArrayBuffer:),
+      "testMethodWhichTransformsArrayBuffer",
+      @selector(testMethodWhichTransformsArrayBuffer:),
       args,
       1);