AsyncWorker: add GetResult() method

Adds an overridable `GetResult()` method, providing arguments to the callback
invoked in `OnOK()`.

Re: nodejs#231 (comment)

doc/async_worker.md

@@ -79,7 +79,7 @@ the `Napi::AsyncWorker::OnOK` callback.
 
 Sets the error message for the error that happened during the execution. Setting
 an error message will cause the `Napi::AsyncWorker::OnError` method to be
-invoked instead of `Napi::AsyncWorker::OnOKOnOK` once the
+invoked instead of `Napi::AsyncWorker::OnOK` once the
 `Napi::AsyncWorker::Execute` method completes.
 
 ```cpp
@@ -107,11 +107,21 @@ virtual void Napi::AsyncWorker::Execute() = 0;
 
 This method is invoked when the computation in the `Execute` method ends.
 The default implementation runs the Callback optionally provided when the AsyncWorker class
-was created.
+was created. The callback will by default receive no arguments. To provide arguments,
+override the `GetResult()` method.
 
 ```cpp
 virtual void Napi::AsyncWorker::OnOK();
 ```
+### GetResult
+
+This method returns the arguments passed to the Callback invoked by the default
+`OnOK()` implementation. The default implementation returns an empty vector,
+providing no arguments to the Callback.
+
+```cpp
+virtual std::vector<napi_value> Napi::AsyncWorker::GetResult(Napi::Env env);
+```
 
 ### OnError
 

napi-inl.h

@@ -3686,7 +3686,7 @@ inline void AsyncWorker::SuppressDestruct() {
 
 inline void AsyncWorker::OnOK() {
   if (!_callback.IsEmpty()) {
-    _callback.Call(_receiver.Value(), std::initializer_list<napi_value>{});
+    _callback.Call(_receiver.Value(), GetResult(_callback.Env()));
   }
 }
 
@@ -3700,7 +3700,13 @@ inline void AsyncWorker::SetError(const std::string& error) {
   _error = error;
 }
 
-inline void AsyncWorker::OnExecute(napi_env /*env*/, void* this_pointer) {
+inline std::vector<napi_value> AsyncWorker::GetResult(Napi::Env env) {
+  return {};
+}
+// The OnExecute method receives an napi_env argument. However, do NOT
+// use it within this method, as it does not run on the main thread and cannot
+// access the napi_env.
+inline void AsyncWorker::OnExecute(napi_env /*DO_NOT_USE*/, void* this_pointer) {
   AsyncWorker* self = static_cast<AsyncWorker*>(this_pointer);
 #ifdef NAPI_CPP_EXCEPTIONS
   try {

napi.h

@@ -1812,6 +1812,7 @@ namespace Napi {
     virtual void OnOK();
     virtual void OnError(const Error& e);
     virtual void Destroy();
+    virtual std::vector<napi_value> GetResult(Napi::Env env);
 
     void SetError(const std::string& error);
 

test/asyncworker.cc

@@ -9,14 +9,25 @@ class TestWorker : public AsyncWorker {
     Object resource = info[1].As<Object>();
     Function cb = info[2].As<Function>();
     Value data = info[3];
+    bool setResult = info[4].As<Boolean>();
 
     TestWorker* worker = new TestWorker(cb, "TestResource", resource);
     worker->Receiver().Set("data", data);
     worker->_succeed = succeed;
+    worker->_setResult = setResult;
     worker->Queue();
   }
 
 protected:
+  std::vector<napi_value> GetResult(Napi::Env env) override {
+    if (_setResult)
+      return {Boolean::New(env, _succeed),
+              String::New(env, _succeed ? "ok" : "error")};
+    else {
+      return {};
+    }
+  }
+
   void Execute() override {
     if (!_succeed) {
       SetError("test error");
@@ -27,6 +38,7 @@ class TestWorker : public AsyncWorker {
   TestWorker(Function cb, const char* resource_name, const Object& resource)
       : AsyncWorker(cb, resource_name, resource) {}
   bool _succeed;
+  bool _setResult;
 };
 
 class TestWorkerNoCallback : public AsyncWorker {

test/asyncworker.js

@@ -58,14 +58,24 @@ function test(binding) {
       assert.strictEqual(typeof e, 'undefined');
       assert.strictEqual(typeof this, 'object');
       assert.strictEqual(this.data, 'test data');
-    }, 'test data');
+    }, 'test data', false);
+
+    binding.asyncworker.doWork(true, {}, function (succeed, succeedString) {
+      assert(arguments.length == 2);
+      assert(succeed);
+      assert(succeedString == "ok");
+      assert.strictEqual(typeof e, 'undefined');
+      assert.strictEqual(typeof this, 'object');
+      assert.strictEqual(this.data, 'test data');
+      console.log("ok!");
+    }, 'test data', true);
 
     binding.asyncworker.doWork(false, {}, function (e) {
       assert.ok(e instanceof Error);
       assert.strictEqual(e.message, 'test error');
       assert.strictEqual(typeof this, 'object');
       assert.strictEqual(this.data, 'test data');
-    }, 'test data');
+    }, 'test data', false);
     return;
   }
 
@@ -76,7 +86,32 @@ function test(binding) {
       assert.strictEqual(typeof e, 'undefined');
       assert.strictEqual(typeof this, 'object');
       assert.strictEqual(this.data, 'test data');
-    }, 'test data');
+    }, 'test data', false);
+
+    hooks.then(actual => {
+      assert.deepStrictEqual(actual, [
+        { eventName: 'init',
+          type: 'TestResource',
+          triggerAsyncId: triggerAsyncId,
+          resource: { foo: 'foo' } },
+        { eventName: 'before' },
+        { eventName: 'after' },
+        { eventName: 'destroy' }
+      ]);
+    }).catch(common.mustNotCall());
+  }
+
+
+  {
+    const hooks = installAsyncHooksForTest();
+    const triggerAsyncId = async_hooks.executionAsyncId();
+    binding.asyncworker.doWork(true, { foo: 'foo' }, function (succeed, succeedString) {
+      assert(arguments.length == 2);
+      assert(succeed);
+      assert(succeedString == "ok");
+      assert.strictEqual(typeof this, 'object');
+      assert.strictEqual(this.data, 'test data');
+    }, 'test data', true);
 
     hooks.then(actual => {
       assert.deepStrictEqual(actual, [
@@ -100,7 +135,7 @@ function test(binding) {
       assert.strictEqual(e.message, 'test error');
       assert.strictEqual(typeof this, 'object');
       assert.strictEqual(this.data, 'test data');
-    }, 'test data');
+    }, 'test data', false);
 
     hooks.then(actual => {
       assert.deepStrictEqual(actual, [