squash! review feedback

Author: bnoordhuis

doc/api/n-api.md

@@ -2575,32 +2575,32 @@ object just created has been garbage collected.
 JavaScript `ArrayBuffer`s are described in
 [Section ArrayBuffer objects][] of the ECMAScript Language Specification.
 
-#### `napi_create_external_sharedarraybuffer`
+#### `node_api_create_external_sharedarraybuffer`
 
 <!-- YAML
 added: REPLACEME
-napiVersion: 1
 -->
 
 ```c
 napi_status
-napi_create_external_sharedarraybuffer(napi_env env,
-                                       void* external_data,
-                                       size_t byte_length,
-                                       void (*finalize_cb)(
-                                          void* external_data,
-                                          void* finalize_hint),
-                                       void* finalize_hint,
-                                       napi_value* result)
+node_api_create_external_sharedarraybuffer(napi_env env,
+                                           void* external_data,
+                                           size_t byte_length,
+                                           void (*finalize_cb)(
+                                              void* external_data,
+                                              void* finalize_hint),
+                                           void* finalize_hint,
+                                           napi_value* result)
 ```
 
 * `[in] env`: The environment that the API is invoked under.
 * `[in] external_data`: Pointer to the underlying byte buffer of the
   `SharedArrayBuffer`.
 * `[in] byte_length`: The length in bytes of the underlying buffer.
 * `[in] finalize_cb`: Optional callback to call when the `SharedArrayBuffer` is
-  being collected. Because a `SharedArrayBuffer` can outlive the environment
-  it was created in, the callback does not get receive a reference to `env`.
+  being collected. Called on an arbitrary thread. Because a `SharedArrayBuffer`
+  can outlive the environment it's created in, the callback does not receive a
+  reference to `env`.
 * `[in] finalize_hint`: Optional hint to pass to the finalize callback during
   collection.
 * `[out] result`: A `napi_value` representing a JavaScript `SharedArrayBuffer`.

src/js_native_api.h

@@ -437,13 +437,16 @@ napi_create_external_arraybuffer(napi_env env,
                                  node_api_basic_finalize finalize_cb,
                                  void* finalize_hint,
                                  napi_value* result);
-NAPI_EXTERN napi_status NAPI_CDECL napi_create_external_sharedarraybuffer(
+#ifdef NAPI_EXPERIMENTAL
+#define NODE_API_EXPERIMENTAL_HAS_CREATE_EXTERNAL_SHAREDARRAYBUFFER
+NAPI_EXTERN napi_status NAPI_CDECL node_api_create_external_sharedarraybuffer(
     napi_env env,
     void* external_data,
     size_t byte_length,
     void (*finalize_cb)(void* external_data, void* finalize_hint),
     void* finalize_hint,
     napi_value* result);
+#endif  // NAPI_EXPERIMENTAL
 #endif  // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
 NAPI_EXTERN napi_status NAPI_CDECL napi_get_arraybuffer_info(
     napi_env env, napi_value arraybuffer, void** data, size_t* byte_length);

src/js_native_api_v8.cc

@@ -136,42 +136,6 @@ napi_status NewExternalString(napi_env env,
   return status;
 }
 
-napi_status NewExternalSharedArrayBuffer(
-    napi_env env,
-    void* external_data,
-    size_t byte_length,
-    void (*finalize_cb)(void* external_data, void* finalize_hint),
-    void* finalize_hint,
-    napi_value* result) {
-  struct FinalizerData {
-    void (*cb)(void* external_data, void* finalize_hint);
-    void* hint;
-  };
-  auto deleter = [](void* external_data, size_t length, void* deleter_data) {
-    if (auto fd = static_cast<FinalizerData*>(deleter_data)) {
-      fd->cb(external_data, fd->hint);
-      delete fd;
-    }
-  };
-  FinalizerData* deleter_data = nullptr;
-  if (finalize_cb != nullptr) {
-    deleter_data = new FinalizerData{finalize_cb, finalize_hint};
-  }
-  auto unique_backing_store = v8::SharedArrayBuffer::NewBackingStore(
-      external_data,
-      byte_length,
-      deleter,
-      reinterpret_cast<void*>(deleter_data));
-  CHECK(!!unique_backing_store);  // Cannot fail.
-  auto shared_backing_store =
-      std::shared_ptr<v8::BackingStore>(std::move(unique_backing_store));
-  auto shared_array_buffer =
-      v8::SharedArrayBuffer::New(env->isolate, shared_backing_store);
-  CHECK_MAYBE_EMPTY(env, shared_array_buffer, napi_generic_failure);
-  *result = v8impl::JsValueFromV8LocalValue(shared_array_buffer);
-  return napi_clear_last_error(env);
-}
-
 class TrackedStringResource : private RefTracker {
  public:
   TrackedStringResource(napi_env env,
@@ -3170,15 +3134,45 @@ napi_create_external_arraybuffer(napi_env env,
       env, buffer, nullptr, nullptr, nullptr, result, nullptr);
 }
 
-napi_status NAPI_CDECL napi_create_external_sharedarraybuffer(
+napi_status NAPI_CDECL node_api_create_external_sharedarraybuffer(
     napi_env env,
     void* external_data,
     size_t byte_length,
     void (*finalize_cb)(void* external_data, void* finalize_hint),
     void* finalize_hint,
     napi_value* result) {
-  return v8impl::NewExternalSharedArrayBuffer(
-      env, external_data, byte_length, finalize_cb, finalize_hint, result);
+#ifdef V8_ENABLE_SANDBOX
+  return napi_set_last_error(env, napi_no_external_buffers_allowed);
+#else
+  struct FinalizerData {
+    void (*cb)(void* external_data, void* finalize_hint);
+    void* hint;
+  };
+  auto deleter = [](void* external_data, size_t length, void* deleter_data) {
+    if (auto fd = static_cast<FinalizerData*>(deleter_data)) {
+      fd->cb(external_data, fd->hint);
+      delete fd;
+    }
+  };
+  FinalizerData* deleter_data = nullptr;
+  if (finalize_cb != nullptr) {
+    deleter_data = new FinalizerData{finalize_cb, finalize_hint};
+  }
+  auto unique_backing_store = v8::SharedArrayBuffer::NewBackingStore(
+      external_data,
+      byte_length,
+      deleter,
+      reinterpret_cast<void*>(deleter_data));
+  CHECK(!!unique_backing_store);  // Cannot fail.
+  auto shared_backing_store =
+      std::shared_ptr<v8::BackingStore>(std::move(unique_backing_store));
+  auto shared_array_buffer = v8::SharedArrayBuffer::New(
+      env->isolate,
+      std::move(shared_backing_store));
+  CHECK_MAYBE_EMPTY(env, shared_array_buffer, napi_generic_failure);
+  *result = v8impl::JsValueFromV8LocalValue(shared_array_buffer);
+  return napi_clear_last_error(env);
+#endif  // V8_ENABLE_SANDBOX
 }
 
 napi_status NAPI_CDECL napi_get_arraybuffer_info(napi_env env,

test/node-api/test_buffer/test_buffer.c

@@ -73,12 +73,12 @@ static napi_value newExternalSharedArrayBuffer(napi_env env,
   napi_value sab;
   NODE_API_CALL(
       env,
-      napi_create_external_sharedarraybuffer(env,
-                                             externalSharedArrayBufferData,
-                                             1,
-                                             freeExternalSharedArrayBuffer,
-                                             NULL,
-                                             &sab));
+      node_api_create_external_sharedarraybuffer(env,
+                                                 externalSharedArrayBufferData,
+                                                 1,
+                                                 freeExternalSharedArrayBuffer,
+                                                 NULL,
+                                                 &sab));
   return sab;
 }