test: port test for make_callback to n-api

Improved test coverage for napi_make_callback by porting the
existing addons/make_callback test to n-api

PR-URL: https://github.com/nodejs/node/pull/12409
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

test/addons-napi/test_make_callback/binding.cc

@@ -0,0 +1,48 @@
+#include <node_api.h>
+#include "../common.h"
+#include <vector>
+
+namespace {
+
+napi_value MakeCallback(napi_env env, napi_callback_info info) {
+  constexpr int kMaxArgs = 10;
+  size_t argc = kMaxArgs;
+  napi_value args[kMaxArgs];
+  NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
+
+  NAPI_ASSERT(env, argc > 0, "Wrong number of arguments");
+
+  napi_value recv = args[0];
+  napi_value func = args[1];
+
+  std::vector<napi_value> argv;
+  for (size_t n = 2; n < argc; n += 1) {
+    argv.push_back(args[n]);
+  }
+
+  napi_valuetype func_type;
+
+  NAPI_CALL(env, napi_typeof(env, func, &func_type));
+
+  napi_value result;
+  if (func_type == napi_function) {
+    NAPI_CALL(env,
+      napi_make_callback(env, recv, func, argv.size(), argv.data(), &result));
+  } else {
+    NAPI_ASSERT(env, false, "Unexpected argument type");
+  }
+
+  return result;
+}
+
+void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
+  napi_value fn;
+  NAPI_CALL_RETURN_VOID(env,
+    napi_create_function(env, NULL, MakeCallback, NULL, &fn));
+  NAPI_CALL_RETURN_VOID(env,
+    napi_set_named_property(env, exports, "makeCallback", fn));
+}
+
+}  // namespace
+
+NAPI_MODULE(binding, Init)

test/addons-napi/test_make_callback/binding.gyp

@@ -0,0 +1,9 @@
+{
+  'targets': [
+    {
+      'target_name': 'binding',
+      'defines': [ 'V8_DEPRECATION_WARNINGS=1' ],
+      'sources': [ 'binding.cc' ]
+    }
+  ]
+}

test/addons-napi/test_make_callback/test.js

@@ -0,0 +1,81 @@
+'use strict';
+
+const common = require('../../common');
+const assert = require('assert');
+const vm = require('vm');
+const binding = require(`./build/${common.buildType}/binding`);
+const makeCallback = binding.makeCallback;
+
+function myMultiArgFunc(arg1, arg2, arg3) {
+  console.log(`MyFunc was called with ${arguments.length} arguments`);
+  assert.strictEqual(arg1, 1);
+  assert.strictEqual(arg2, 2);
+  assert.strictEqual(arg3, 3);
+  return 42;
+}
+
+assert.strictEqual(42, makeCallback(process, common.mustCall(function() {
+  assert.strictEqual(0, arguments.length);
+  assert.strictEqual(this, process);
+  return 42;
+})));
+
+assert.strictEqual(42, makeCallback(process, common.mustCall(function(x) {
+  assert.strictEqual(1, arguments.length);
+  assert.strictEqual(this, process);
+  assert.strictEqual(x, 1337);
+  return 42;
+}), 1337));
+
+assert.strictEqual(42,
+                   makeCallback(this,
+                                common.mustCall(myMultiArgFunc), 1, 2, 3));
+
+// TODO(node-api): napi_make_callback needs to support
+// strings passed for the func argument
+/*
+const recv = {
+  one: common.mustCall(function() {
+    assert.strictEqual(0, arguments.length);
+    assert.strictEqual(this, recv);
+    return 42;
+  }),
+  two: common.mustCall(function(x) {
+    assert.strictEqual(1, arguments.length);
+    assert.strictEqual(this, recv);
+    assert.strictEqual(x, 1337);
+    return 42;
+  }),
+};
+
+assert.strictEqual(42, makeCallback(recv, 'one'));
+assert.strictEqual(42, makeCallback(recv, 'two', 1337));
+
+// Check that callbacks on a receiver from a different context works.
+const foreignObject = vm.runInNewContext('({ fortytwo() { return 42; } })');
+assert.strictEqual(42, makeCallback(foreignObject, 'fortytwo'));
+*/
+
+// Check that the callback is made in the context of the receiver.
+const target = vm.runInNewContext(`
+    (function($Object) {
+      if (Object === $Object)
+        throw new Error('bad');
+      return Object;
+    })
+`);
+assert.notStrictEqual(Object, makeCallback(process, target, Object));
+
+// Runs in inner context.
+const forward = vm.runInNewContext(`
+    (function(forward) {
+      return forward(Object);
+    })
+`);
+// Runs in outer context.
+const endpoint = function($Object) {
+  if (Object === $Object)
+    throw new Error('bad');
+  return Object;
+};
+assert.strictEqual(Object, makeCallback(process, forward, endpoint));

test/addons-napi/test_make_callback_recurse/binding.cc

@@ -0,0 +1,32 @@
+#include <node_api.h>
+#include "../common.h"
+#include <vector>
+
+namespace {
+
+napi_value MakeCallback(napi_env env, napi_callback_info info) {
+  size_t argc = 2;
+  napi_value args[2];
+  NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
+
+  napi_value recv = args[0];
+  napi_value func = args[1];
+
+  napi_make_callback(env,
+    recv, func, 0 /* argc */, nullptr /* argv */, nullptr /* result */);
+
+  return recv;
+}
+
+void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
+  napi_value fn;
+  NAPI_CALL_RETURN_VOID(env,
+    napi_create_function(env, NULL, MakeCallback, NULL, &fn));
+  NAPI_CALL_RETURN_VOID(env,
+    napi_set_named_property(env, exports, "makeCallback", fn));
+}
+
+
+}  // namespace
+
+NAPI_MODULE(binding, Init)

test/addons-napi/test_make_callback_recurse/binding.gyp

@@ -0,0 +1,9 @@
+{
+  'targets': [
+    {
+      'target_name': 'binding',
+      'defines': [ 'V8_DEPRECATION_WARNINGS=1' ],
+      'sources': [ 'binding.cc' ]
+    }
+  ]
+}

test/addons-napi/test_make_callback_recurse/test.js

@@ -0,0 +1,151 @@
+'use strict';
+
+const common = require('../../common');
+const assert = require('assert');
+const domain = require('domain');
+const binding = require(`./build/${common.buildType}/binding`);
+const makeCallback = binding.makeCallback;
+
+// Make sure this is run in the future.
+const mustCallCheckDomains = common.mustCall(checkDomains);
+
+
+// Make sure that using MakeCallback allows the error to propagate.
+assert.throws(function() {
+  makeCallback({}, function() {
+    throw new Error('hi from domain error');
+  });
+}, /^Error: hi from domain error$/);
+
+
+// Check the execution order of the nextTickQueue and MicrotaskQueue in
+// relation to running multiple MakeCallback's from bootstrap,
+// node::MakeCallback() and node::AsyncWrap::MakeCallback().
+// TODO(trevnorris): Is there a way to verify this is being run during
+// bootstrap?
+(function verifyExecutionOrder(arg) {
+  const results = [];
+
+  // Processing of the MicrotaskQueue is manually handled by node. They are not
+  // processed until after the nextTickQueue has been processed.
+  Promise.resolve(1).then(common.mustCall(function() {
+    results.push(7);
+  }));
+
+  // The nextTick should run after all immediately invoked calls.
+  process.nextTick(common.mustCall(function() {
+    results.push(3);
+
+    // Run same test again but while processing the nextTickQueue to make sure
+    // the following MakeCallback call breaks in the middle of processing the
+    // queue and allows the script to run normally.
+    process.nextTick(common.mustCall(function() {
+      results.push(6);
+    }));
+
+    makeCallback({}, common.mustCall(function() {
+      results.push(4);
+    }));
+
+    results.push(5);
+  }));
+
+  results.push(0);
+
+  // MakeCallback is calling the function immediately, but should then detect
+  // that a script is already in the middle of execution and return before
+  // either the nextTickQueue or MicrotaskQueue are processed.
+  makeCallback({}, common.mustCall(function() {
+    results.push(1);
+  }));
+
+  // This should run before either the nextTickQueue or MicrotaskQueue are
+  // processed. Previously MakeCallback would not detect this circumstance
+  // and process them immediately.
+  results.push(2);
+
+  setImmediate(common.mustCall(function() {
+    for (let i = 0; i < results.length; i++) {
+      assert.strictEqual(results[i], i,
+                         `verifyExecutionOrder(${arg}) results: ${results}`);
+    }
+    if (arg === 1) {
+      // The tests are first run on bootstrap during LoadEnvironment() in
+      // src/node.cc. Now run the tests through node::MakeCallback().
+      setImmediate(function() {
+        makeCallback({}, common.mustCall(function() {
+          verifyExecutionOrder(2);
+        }));
+      });
+    } else if (arg === 2) {
+      // setTimeout runs via the TimerWrap, which runs through
+      // AsyncWrap::MakeCallback(). Make sure there are no conflicts using
+      // node::MakeCallback() within it.
+      setTimeout(common.mustCall(function() {
+        verifyExecutionOrder(3);
+      }), 10);
+    } else if (arg === 3) {
+      mustCallCheckDomains();
+    } else {
+      throw new Error('UNREACHABLE');
+    }
+  }));
+}(1));
+
+
+function checkDomains() {
+  // Check that domains are properly entered/exited when called in multiple
+  // levels from both node::MakeCallback() and AsyncWrap::MakeCallback
+  setImmediate(common.mustCall(function() {
+    const d1 = domain.create();
+    const d2 = domain.create();
+    const d3 = domain.create();
+
+    makeCallback({domain: d1}, common.mustCall(function() {
+      assert.strictEqual(d1, process.domain);
+      makeCallback({domain: d2}, common.mustCall(function() {
+        assert.strictEqual(d2, process.domain);
+        makeCallback({domain: d3}, common.mustCall(function() {
+          assert.strictEqual(d3, process.domain);
+        }));
+        assert.strictEqual(d2, process.domain);
+      }));
+      assert.strictEqual(d1, process.domain);
+    }));
+  }));
+
+  setTimeout(common.mustCall(function() {
+    const d1 = domain.create();
+    const d2 = domain.create();
+    const d3 = domain.create();
+
+    makeCallback({domain: d1}, common.mustCall(function() {
+      assert.strictEqual(d1, process.domain);
+      makeCallback({domain: d2}, common.mustCall(function() {
+        assert.strictEqual(d2, process.domain);
+        makeCallback({domain: d3}, common.mustCall(function() {
+          assert.strictEqual(d3, process.domain);
+        }));
+        assert.strictEqual(d2, process.domain);
+      }));
+      assert.strictEqual(d1, process.domain);
+    }));
+  }), 1);
+
+  function testTimer(id) {
+    // Make sure nextTick, setImmediate and setTimeout can all recover properly
+    // after a thrown makeCallback call.
+    const d = domain.create();
+    d.on('error', common.mustCall(function(e) {
+      assert.strictEqual(e.message, `throw from domain ${id}`);
+    }));
+    makeCallback({domain: d}, function() {
+      throw new Error(`throw from domain ${id}`);
+    });
+    throw new Error('UNREACHABLE');
+  }
+
+  process.nextTick(common.mustCall(testTimer), 3);
+  setImmediate(common.mustCall(testTimer), 2);
+  setTimeout(common.mustCall(testTimer), 1, 1);
+}