n-api: Fix test-addons-napi failures (#249)

- Reimplement napi_make_callback to call node::MakeCallback
   using V8 (chakrashim) types. We always knew the direct call to
   JsCallFunction() was incorrect there.
 - Re-enable the two tests that were failing due to napi_make_callback

Fixes: #246

src/node_api_jsrt.cc

@@ -1623,22 +1623,22 @@ napi_status napi_make_callback(napi_env env,
                                size_t argc,
                                const napi_value* argv,
                                napi_value* result) {
-  JsValueRef object = reinterpret_cast<JsValueRef>(recv);
-  JsValueRef function = reinterpret_cast<JsValueRef>(func);
-  std::vector<JsValueRef> args(argc + 1);
-  args[0] = object;
-  for (size_t i = 0; i < argc; i++) {
-    args[i + 1] = reinterpret_cast<JsValueRef>(argv[i]);
-  }
-  JsValueRef returnValue;
-  CHECK_JSRT(JsCallFunction(
-    function,
-    args.data(),
-    static_cast<uint16_t>(argc + 1),
-    &returnValue));
+  v8::Isolate* isolate = v8impl::V8IsolateFromJsEnv(env);
+  v8::Local<v8::Object> v8recv =
+    v8impl::V8LocalValueFromJsValue(recv).As<v8::Object>();
+  v8::Local<v8::Function> v8func =
+    v8impl::V8LocalValueFromJsValue(func).As<v8::Function>();
+  v8::Local<v8::Value>* v8argv =
+    reinterpret_cast<v8::Local<v8::Value>*>(const_cast<napi_value*>(argv));
+
+  // TODO(jasongin): Expose JSRT or N-API version of node::MakeCallback?
+  v8::Local<v8::Value> v8result =
+    node::MakeCallback(isolate, v8recv, v8func, argc, v8argv);
+
   if (result != nullptr) {
-    *result = reinterpret_cast<napi_value>(returnValue);
+    *result = v8impl::JsValueFromV8LocalValue(v8result);
   }
+
   return napi_ok;
 }
 

test/addons-napi/test_async/test.js

@@ -3,12 +3,6 @@ const common = require('../../common');
 const assert = require('assert');
 const child_process = require('child_process');
 
-if (common.isChakraEngine) {
-  common.skip('Skipped for node-chakracore till #246 is fixed.');
-  return;
-}
-
-
 const test_async = require(`./build/${common.buildType}/test_async`);
 
 const testException = 'test_async_cb_exception';

test/addons-napi/test_make_callback_recurse/test.js

@@ -6,11 +6,6 @@ const domain = require('domain');
 const binding = require(`./build/${common.buildType}/binding`);
 const makeCallback = binding.makeCallback;
 
-if (common.isChakraEngine) {
-  common.skip('Skipped for node-chakracore till #246 is fixed.');
-  return;
-}
-
 // Make sure this is run in the future.
 const mustCallCheckDomains = common.mustCall(checkDomains);