Fixes problems associated with node 10 and NAN (#14847)

* Fixes problems associated with node 10 and NAN

* Follow up removal of unnecessary linking to map object

* Remove header left over from debugging

4 files changed, 60 insertions, 53 deletions

diff --git a/platform/node/src/node_map.cpp b/platform/node/src/node_map.cpp
index f88b6c6c79..e59982ad99 100644
--- a/platform/node/src/node_map.cpp
+++ b/platform/node/src/node_map.cpp
@@ -544,7 +544,9 @@ void NodeMap::renderFinished() {
             },
             img.data.get()
         ).ToLocalChecked();
-        img.data.release();
+        if (!pixels.IsEmpty()) {
+            img.data.release();
+        }
 
         v8::Local<v8::Value> argv[] = {
             Nan::Null(),
@@ -1244,20 +1246,19 @@ std::unique_ptr<mbgl::AsyncRequest> NodeFileSource::request(const mbgl::Resource
     // *this while we're still executing code.
     nodeMap->handle();
 
+    auto asyncRequest = std::make_unique<node_mbgl::NodeAsyncRequest>();
+
     v8::Local<v8::Value> argv[] = {
         Nan::New<v8::External>(nodeMap),
-        Nan::New<v8::External>(&callback_)
+        Nan::New<v8::External>(&callback_),
+        Nan::New<v8::External>(asyncRequest.get()),
+        Nan::New(resource.url).ToLocalChecked(),
+        Nan::New<v8::Integer>(resource.kind)
     };
 
-    auto instance = Nan::NewInstance(Nan::New(node_mbgl::NodeRequest::constructor), 2, argv).ToLocalChecked();
-
-    Nan::Set(instance, Nan::New("url").ToLocalChecked(), Nan::New(resource.url).ToLocalChecked());
-    Nan::Set(instance, Nan::New("kind").ToLocalChecked(), Nan::New<v8::Integer>(resource.kind));
-
-    auto req = Nan::ObjectWrap::Unwrap<node_mbgl::NodeRequest>(instance);
-    req->Execute();
+    Nan::NewInstance(Nan::New(node_mbgl::NodeRequest::constructor), 5, argv).ToLocalChecked();
 
-    return std::make_unique<node_mbgl::NodeRequest::NodeAsyncRequest>(req);
+    return asyncRequest;
 }
 
 } // namespace node_mbgl
diff --git a/platform/node/src/node_map.hpp b/platform/node/src/node_map.hpp
index d45dbec92b..486754c1c3 100644
--- a/platform/node/src/node_map.hpp
+++ b/platform/node/src/node_map.hpp
@@ -93,6 +93,7 @@ public:
 
 struct NodeFileSource : public mbgl::FileSource {
     NodeFileSource(NodeMap* nodeMap_) : nodeMap(nodeMap_) {}
+    ~NodeFileSource() {}
     std::unique_ptr<mbgl::AsyncRequest> request(const mbgl::Resource&, mbgl::FileSource::Callback) final;
     NodeMap* nodeMap;
 };
diff --git a/platform/node/src/node_request.cpp b/platform/node/src/node_request.cpp
index 8c26d44583..78f50ea9ce 100644
--- a/platform/node/src/node_request.cpp
+++ b/platform/node/src/node_request.cpp
@@ -8,11 +8,11 @@
 namespace node_mbgl {
 
 NodeRequest::NodeRequest(
-    NodeMap* target_,
-    mbgl::FileSource::Callback callback_)
-    : AsyncWorker(nullptr),
-    target(target_),
-    callback(std::move(callback_)) {
+    mbgl::FileSource::Callback callback_,
+    NodeAsyncRequest* asyncRequest_)
+    : callback(std::move(callback_)),
+    asyncRequest(asyncRequest_) {
+        asyncRequest->request = this;
 }
 
 NodeRequest::~NodeRequest() {
@@ -40,10 +40,16 @@ void NodeRequest::Init() {
 void NodeRequest::New(const Nan::FunctionCallbackInfo<v8::Value>& info) {
     auto target = reinterpret_cast<NodeMap*>(info[0].As<v8::External>()->Value());
     auto callback = reinterpret_cast<mbgl::FileSource::Callback*>(info[1].As<v8::External>()->Value());
+    auto asyncRequest = reinterpret_cast<NodeAsyncRequest*>(info[2].As<v8::External>()->Value());
 
-    auto request = new NodeRequest(target, *callback);
+    auto request = new NodeRequest(*callback, asyncRequest);
 
     request->Wrap(info.This());
+    request->Ref();
+    Nan::Set(info.This(), Nan::New("url").ToLocalChecked(), info[3]);
+    Nan::Set(info.This(), Nan::New("kind").ToLocalChecked(), info[4]);
+    v8::Local<v8::Value> argv[] = { info.This() };
+    request->asyncResource->runInAsyncScope(Nan::To<v8::Object>(target->handle()->GetInternalField(1)).ToLocalChecked(), "request", 1, argv);
     info.GetReturnValue().Set(info.This());
 }
 
@@ -52,7 +58,9 @@ void NodeRequest::HandleCallback(const Nan::FunctionCallbackInfo<v8::Value>& inf
 
     // Move out of the object so callback() can only be fired once.
     auto callback = std::move(request->callback);
+    request->callback = {};
     if (!callback) {
+        request->unref();
         return info.GetReturnValue().SetUndefined();
     }
 
@@ -65,12 +73,18 @@ void NodeRequest::HandleCallback(const Nan::FunctionCallbackInfo<v8::Value>& inf
         auto msg = Nan::New("message").ToLocalChecked();
 
         if (Nan::Has(err, msg).FromJust()) {
-            request->SetErrorMessage(*Nan::Utf8String(
-                Nan::Get(err, msg).ToLocalChecked()));
+            response.error = std::make_unique<mbgl::Response::Error>(
+                mbgl::Response::Error::Reason::Other,
+                *Nan::Utf8String(Nan::Get(err, msg).ToLocalChecked())
+            );
         }
     } else if (info[0]->IsString()) {
-        request->SetErrorMessage(*Nan::Utf8String(info[0]));
+        response.error = std::make_unique<mbgl::Response::Error>(
+            mbgl::Response::Error::Reason::Other,
+            *Nan::Utf8String(info[0])
+        );
     } else if (info.Length() < 2 || !info[1]->IsObject()) {
+        request->unref();
         return Nan::ThrowTypeError("Second argument must be a response object");
     } else {
         auto res = Nan::To<v8::Object>(info[1]).ToLocalChecked();
@@ -104,43 +118,34 @@ void NodeRequest::HandleCallback(const Nan::FunctionCallbackInfo<v8::Value>& inf
                     node::Buffer::Length(data)
                 );
             } else {
+                request->unref();
                 return Nan::ThrowTypeError("Response data must be a Buffer");
             }
         }
     }
 
-    if (request->ErrorMessage()) {
-        response.error = std::make_unique<mbgl::Response::Error>(
-            mbgl::Response::Error::Reason::Other,
-            request->ErrorMessage()
-        );
-    }
-
     // Send the response object to the NodeFileSource object
     callback(response);
+    request->unref();
     info.GetReturnValue().SetUndefined();
 }
 
-void NodeRequest::Execute() {
-    v8::Local<v8::Value> argv[] = { handle() };
-
-    Nan::AsyncResource res("mbgl:execute");
-    res.runInAsyncScope(Nan::To<v8::Object>(target->handle()->GetInternalField(1)).ToLocalChecked(), "request", 1, argv);
+void NodeRequest::unref() {
+  Nan::HandleScope scope;
+  delete asyncResource;
+  asyncResource = nullptr;
+  Unref();
 }
 
-NodeRequest::NodeAsyncRequest::NodeAsyncRequest(NodeRequest* request_) : request(request_) {
-    assert(request);
+NodeAsyncRequest::NodeAsyncRequest() : request(nullptr) {}
 
-    // Make sure the JS object has a pointer to this so that it can remove
-    // its pointer in the destructor
-    request->asyncRequest = this;
-}
-
-NodeRequest::NodeAsyncRequest::~NodeAsyncRequest() {
+NodeAsyncRequest::~NodeAsyncRequest() {
     if (request) {
         // Remove the callback function because the AsyncRequest was
         // canceled and we are no longer interested in the result.
-        request->callback = {};
+        if (request->callback) {
+            request->callback = {};
+        }
         request->asyncRequest = nullptr;
     }
 }
diff --git a/platform/node/src/node_request.hpp b/platform/node/src/node_request.hpp
index 7d7679a3c7..830d262b40 100644
--- a/platform/node/src/node_request.hpp
+++ b/platform/node/src/node_request.hpp
@@ -11,18 +11,19 @@
 
 namespace node_mbgl {
 
-class NodeMap;
+class NodeRequest;
+
+struct NodeAsyncRequest : public mbgl::AsyncRequest {
+    NodeAsyncRequest();
+    ~NodeAsyncRequest() override;
+    NodeRequest* request;
+};
+
+class NodeRequest : public Nan::ObjectWrap {
 
-class NodeRequest : public Nan::ObjectWrap,
-                    public Nan::AsyncWorker {
 public:
-    struct NodeAsyncRequest : public mbgl::AsyncRequest {
-        NodeAsyncRequest(NodeRequest*);
-        ~NodeAsyncRequest() override;
-        NodeRequest* request;
-    };
 
-    NodeRequest(NodeMap*, mbgl::FileSource::Callback);
+    NodeRequest(mbgl::FileSource::Callback, NodeAsyncRequest*);
     ~NodeRequest();
 
     static Nan::Persistent<v8::Function> constructor;
@@ -32,12 +33,11 @@ public:
     static void New(const Nan::FunctionCallbackInfo<v8::Value>&);
     static void HandleCallback(const Nan::FunctionCallbackInfo<v8::Value>&);
 
-    void Execute();
+    void unref();
 
-private:
-    NodeMap* target;
     mbgl::FileSource::Callback callback;
-    NodeAsyncRequest* asyncRequest = nullptr;
+    NodeAsyncRequest* asyncRequest;
+    Nan::AsyncResource* asyncResource = new Nan::AsyncResource("mbgl:execute");
 };
 
 }