Add N-API compliance tests

CMakeLists.txt

@@ -154,6 +154,17 @@ endif()
 FetchContent_MakeAvailable_With_Message(arcana.cpp)
 set_property(TARGET arcana PROPERTY FOLDER Dependencies)
 
+if(ANDROID)
+ FetchContent_GetProperties(AndroidExtensions)
+ if(NOT AndroidExtensions_POPULATED)
+ FetchContent_Populate(AndroidExtensions)
+ FetchContent_GetProperties(AndroidExtensions)
+ add_subdirectory(${androidextensions_SOURCE_DIR} ${androidextensions_BINARY_DIR})
+ else()
+ add_subdirectory(${androidextensions_SOURCE_DIR} ${androidextensions_BINARY_DIR})
+ endif()
+endif()
+
 if(JSRUNTIMEHOST_POLYFILL_XMLHTTPREQUEST OR JSRUNTIMEHOST_POLYFILL_FETCH)
  FetchContent_MakeAvailable_With_Message(UrlLib)
  set_property(TARGET UrlLib PROPERTY FOLDER Dependencies)

Core/Node-API/CMakeLists.txt

@@ -261,7 +261,24 @@ Make sure Hermes was fetched at the top-level CMakeLists.txt and NAPI_JAVASCRIPT
  message(STATUS "Selected ${NAPI_JAVASCRIPT_ENGINE}")
 endif()
 
-add_library(napi ${SOURCES})
+# On Android, native addons are dlopen'd as standalone .node modules and resolve their napi_* imports
+# from a shared napi at load time -- bionic will not surface a statically-linked host's napi to a
+# dlopen'd module, so the host and every addon must share a single libnapi.so. Default napi to a
+# shared library on Android so that model works out of the box; an integrator who wants a static napi
+# (e.g. for size/packaging) can override with -DJSR_NAPI_SHARED=OFF. The option defaults OFF on other
+# platforms, where napi keeps following the project's default library type (i.e. honors
+# BUILD_SHARED_LIBS).
+set(JSR_NAPI_SHARED_DEFAULT OFF)
+if(ANDROID AND NOT NAPI_JAVASCRIPT_ENGINE STREQUAL "Hermes")
+ set(JSR_NAPI_SHARED_DEFAULT ON)
+endif()
+option(JSR_NAPI_SHARED "Build napi as a shared library (libnapi.so)" ${JSR_NAPI_SHARED_DEFAULT})
+
+if(JSR_NAPI_SHARED)
+ add_library(napi SHARED ${SOURCES})
+else()
+ add_library(napi ${SOURCES})
+endif()
 
 target_include_directories(napi ${INCLUDE_DIRECTORIES})
 target_link_libraries(napi ${LINK_LIBRARIES})
@@ -277,5 +294,11 @@ if(NAPI_JAVASCRIPT_ENGINE STREQUAL "Hermes")
  NAPI_HERMES_MAX_HEAP_SIZE_MB=${NAPI_HERMES_MAX_HEAP_SIZE_MB})
 endif()
 
+# Expose the selected engine as a compile definition so engine-agnostic consumers (the node_lite test
+# harness) can branch on engine capability instead of guessing from the platform
+# (__APPLE__ == JSC / __ANDROID__ == V8 breaks Android-JSC, Linux-V8, Windows-Chakra, ...).
+string(TOUPPER "${NAPI_JAVASCRIPT_ENGINE}" NAPI_ENGINE_UPPER)
+target_compile_definitions(napi PUBLIC JSR_NAPI_ENGINE_${NAPI_ENGINE_UPPER})
+
 set_property(TARGET napi PROPERTY FOLDER Dependencies)
 source_group(TREE ${CMAKE_CURRENT_SOURCE_DIR} FILES ${SOURCES})

Core/Node-API/Source/js_native_api_javascriptcore.cc

@@ -740,6 +740,9 @@ struct napi_ref__ {
  CHECK_NAPI(ReferenceInfo::GetObjectId(env, _value, &_objectId));
  if (_objectId == 0) {
  CHECK_NAPI(ReferenceInfo::Initialize(env, _value, [value = _value](ReferenceInfo* info) {
+ if (info->Env()->shutting_down) {
+ return;
+ }
  auto entry{info->Env()->active_ref_values.find(value)};
  // NOTE: The finalizer callback is actually on a "sentinel" JS object that is linked to the
  // actual JS object we are trying to track. This means it is possible for the tracked object
@@ -841,6 +844,18 @@ void napi_env__::deinit_symbol(JSValueRef symbol) {
  JSValueUnprotect(context, symbol);
 }
 
+void napi_env__::init_function_prototype_call() {
+ // Capture the canonical Function.prototype.call once, at env init, so napi_call_function does not
+ // depend on a target function's own (user-overridable) "call" property.
+ JSObjectRef global = JSContextGetGlobalObject(context);
+ JSValueRef function_ctor = JSObjectGetProperty(context, global, JSString("Function"), nullptr);
+ JSObjectRef function_ctor_obj = JSValueToObject(context, function_ctor, nullptr);
+ JSValueRef prototype = JSObjectGetProperty(context, function_ctor_obj, JSString("prototype"), nullptr);
+ JSObjectRef prototype_obj = JSValueToObject(context, prototype, nullptr);
+ function_prototype_call = JSObjectGetProperty(context, prototype_obj, JSString("call"), nullptr);
+ JSValueProtect(context, function_prototype_call);
+}
+
 // Warning: Keep in-sync with napi_status enum
 static const char* error_messages[] = {
  nullptr,
@@ -1647,14 +1662,27 @@ napi_status napi_call_function(napi_env env,
  CHECK_ARG(env, argv);
  }
 
+ JSObjectRef function_object = ToJSObject(env, func);
+
+ std::vector<JSValueRef> call_args(argc + 1);
+ call_args[0] = ToJSValue(recv);
+ for (size_t i = 0; i < argc; ++i) {
+ call_args[i + 1] = ToJSValue(argv[i]);
+ }
+
  JSValueRef exception{};
- JSValueRef return_value{JSObjectCallAsFunction(
- env->context,
- ToJSObject(env, func),
- JSValueIsUndefined(env->context, ToJSValue(recv)) ? nullptr : ToJSObject(env, recv),
- argc,
- ToJSValues(argv),
- &exception)};
+ // Invoke through the canonical Function.prototype.call (captured at env init), not the target's own
+ // "call" property -- user code could override func.call and change native call behavior.
+ JSObjectRef call_object =
+ JSValueToObject(env->context, env->function_prototype_call, &exception);
+ CHECK_JSC(env, exception);
+
+ JSValueRef return_value{JSObjectCallAsFunction(env->context,
+ call_object,
+ function_object,
+ call_args.size(),
+ call_args.data(),
+ &exception)};
  CHECK_JSC(env, exception);
 
  if (result != nullptr) {

Core/Node-API/Source/js_native_api_javascriptcore.h

@@ -15,11 +15,13 @@ struct napi_env__ {
  napi_extended_error_info last_error{nullptr, nullptr, 0, napi_ok};
  std::unordered_map<napi_value, std::uintptr_t> active_ref_values{};
  std::list<napi_ref> strong_refs{};
+ bool shutting_down{false};
 
  JSValueRef constructor_info_symbol{};
  JSValueRef function_info_symbol{};
  JSValueRef reference_info_symbol{};
  JSValueRef wrapper_info_symbol{};
+ JSValueRef function_prototype_call{};
 
  // Escapable scope bookkeeping: token -> whether that scope has escaped. Values
  // are rooted by the engine rather than by a scope here, so this exists only to
@@ -38,10 +40,13 @@ struct napi_env__ {
  init_symbol(function_info_symbol, "BabylonNative_FunctionInfo");
  init_symbol(reference_info_symbol, "BabylonNative_ReferenceInfo");
  init_symbol(wrapper_info_symbol, "BabylonNative_WrapperInfo");
+ init_function_prototype_call();
  }
 
  ~napi_env__() {
+ shutting_down = true;
  deinit_refs();
+ deinit_symbol(function_prototype_call);
  deinit_symbol(wrapper_info_symbol);
  deinit_symbol(reference_info_symbol);
  deinit_symbol(function_info_symbol);
@@ -64,6 +69,7 @@ struct napi_env__ {
 
  void deinit_refs();
  void init_symbol(JSValueRef& symbol, const char* description);
+ void init_function_prototype_call();
  void deinit_symbol(JSValueRef symbol);
 };
 

Tests/CMakeLists.txt

@@ -1,2 +1,12 @@
 add_subdirectory(UnitTests)
+
+# The Node-API conformance harness (node_lite + the js-native-api addons) is written against the C
+# napi_* API. The JSI backend (Core/Node-API-JSI) implements only the C++ `Napi::` wrapper over jsi and
+# exports no C napi_* symbols, and its napi.h carries its own copy of the napi types -- which collides
+# with the shared <napi/js_native_api_types.h> the harness includes (C2365 redefinition). Skip the
+# harness for JSI rather than trying to reconcile the two type sources.
+if(NOT NAPI_JAVASCRIPT_ENGINE STREQUAL "JSI")
+ add_subdirectory(NodeApi)
+endif()
+
 npm(install --silent)

Tests/NodeApi/.clang-format

@@ -0,0 +1,111 @@
+---
+Language: Cpp
+# BasedOnStyle: Google
+AccessModifierOffset: -1
+AlignAfterOpenBracket: Align
+AlignConsecutiveAssignments: false
+AlignConsecutiveDeclarations: false
+AlignEscapedNewlines: Right
+AlignOperands: true
+AlignTrailingComments: true
+AllowAllParametersOfDeclarationOnNextLine: true
+AllowShortBlocksOnASingleLine: false
+AllowShortCaseLabelsOnASingleLine: false
+AllowShortFunctionsOnASingleLine: Inline
+AllowShortIfStatementsOnASingleLine: true
+AllowShortLoopsOnASingleLine: true
+AlwaysBreakAfterDefinitionReturnType: None
+AlwaysBreakAfterReturnType: None
+AlwaysBreakBeforeMultilineStrings: false
+AlwaysBreakTemplateDeclarations: true
+BinPackArguments: false
+BinPackParameters: false
+BraceWrapping:
+ AfterClass: false
+ AfterControlStatement: false
+ AfterEnum: false
+ AfterFunction: false
+ AfterNamespace: false
+ AfterObjCDeclaration: false
+ AfterStruct: false
+ AfterUnion: false
+ AfterExternBlock: false
+ BeforeCatch: false
+ BeforeElse: false
+ IndentBraces: false
+ SplitEmptyFunction: true
+ SplitEmptyRecord: true
+ SplitEmptyNamespace: true
+BreakBeforeBinaryOperators: None
+BreakBeforeBraces: Attach
+BreakBeforeInheritanceComma: false
+BreakBeforeTernaryOperators: true
+BreakConstructorInitializersBeforeComma: false
+BreakConstructorInitializers: BeforeColon
+BreakAfterJavaFieldAnnotations: false
+BreakStringLiterals: true
+ColumnLimit: 80
+CommentPragmas: '^ IWYU pragma:'
+CompactNamespaces: false
+ConstructorInitializerAllOnOneLineOrOnePerLine: true
+ConstructorInitializerIndentWidth: 4
+ContinuationIndentWidth: 4
+Cpp11BracedListStyle: true
+DerivePointerAlignment: false
+DisableFormat: false
+ExperimentalAutoDetectBinPacking: false
+FixNamespaceComments: true
+ForEachMacros:
+ - foreach
+ - Q_FOREACH
+ - BOOST_FOREACH
+IncludeBlocks: Preserve
+IncludeCategories:
+ - Regex: '^<ext/.*\.h>'
+ Priority: 2
+ - Regex: '^<.*\.h>'
+ Priority: 1
+ - Regex: '^<.*'
+ Priority: 2
+ - Regex: '.*'
+ Priority: 3
+IncludeIsMainRegex: '([-_](test|unittest))?$'
+IndentCaseLabels: true
+IndentPPDirectives: None
+IndentWidth: 2
+IndentWrappedFunctionNames: false
+JavaScriptQuotes: Leave
+JavaScriptWrapImports: true
+KeepEmptyLinesAtTheStartOfBlocks: false
+MacroBlockBegin: ''
+MacroBlockEnd: ''
+MaxEmptyLinesToKeep: 1
+NamespaceIndentation: None
+ObjCBlockIndentWidth: 2
+ObjCSpaceAfterProperty: false
+ObjCSpaceBeforeProtocolList: false
+PenaltyBreakAssignment: 2
+PenaltyBreakBeforeFirstCallParameter: 1
+PenaltyBreakComment: 300
+PenaltyBreakFirstLessLess: 120
+PenaltyBreakString: 1000
+PenaltyExcessCharacter: 1000000
+PenaltyReturnTypeOnItsOwnLine: 200
+PointerAlignment: Left
+ReflowComments: true
+SortIncludes: true
+SortUsingDeclarations: true
+SpaceAfterCStyleCast: false
+SpaceAfterTemplateKeyword: true
+SpaceBeforeAssignmentOperators: true
+SpaceBeforeParens: ControlStatements
+SpaceInEmptyParentheses: false
+SpacesBeforeTrailingComments: 2
+SpacesInAngles: false
+SpacesInContainerLiterals: true
+SpacesInCStyleCastParentheses: false
+SpacesInParentheses: false
+SpacesInSquareBrackets: false
+Standard: Auto
+TabWidth: 8
+UseTab: Never