diff options
author | Blake Thompson <blake@mapbox.com> | 2019-06-26 13:38:06 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-06-26 13:38:06 -0500 |
commit | a16c67d953926bd9b86359316b983d01825fd0f2 (patch) | |
tree | bab00ce4f91d46a8c2cd1a17a3be18503adf1131 | |
parent | 012ccecaa4329608d2f7c8a30d3e8902b1fee372 (diff) | |
download | qtlocation-mapboxgl-a16c67d953926bd9b86359316b983d01825fd0f2.tar.gz |
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
-rw-r--r-- | platform/node/src/node_map.cpp | 21 | ||||
-rw-r--r-- | platform/node/src/node_map.hpp | 1 | ||||
-rw-r--r-- | platform/node/src/node_request.cpp | 65 | ||||
-rw-r--r-- | platform/node/src/node_request.hpp | 26 |
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"); }; } |