diff --git a/src/node_file_source.cpp b/src/node_file_source.cpp index 3409f03af..0ff4accdb 100644 --- a/src/node_file_source.cpp +++ b/src/node_file_source.cpp @@ -8,46 +8,12 @@ namespace node_mbgl { -//////////////////////////////////////////////////////////////////////////////////////////////// -// Static Node Methods - -v8::Persistent NodeFileSource::constructorTemplate; - -void NodeFileSource::Init(v8::Handle target) { - NanScope(); - - v8::Local t = NanNew(New); - - t->InstanceTemplate()->SetInternalFieldCount(1); - t->SetClassName(NanNew("FileSource")); - - NanAssignPersistent(constructorTemplate, t); - - target->Set(NanNew("FileSource"), t->GetFunction()); -} - -NAN_METHOD(NodeFileSource::New) { - NanScope(); - - if (!args.IsConstructCall()) { - return NanThrowTypeError("Use the new operator to create new FileSource objects"); - } - - auto fs = new NodeFileSource(); - fs->Wrap(args.This()); - - NanReturnValue(args.This()); -} - -//////////////////////////////////////////////////////////////////////////////////////////////// -// Instance - struct NodeFileSource::Action { const enum : bool { Add, Cancel } type; mbgl::Resource const resource; }; -NodeFileSource::NodeFileSource() : +NodeFileSource::NodeFileSource(v8::Handle options_) : queue(new Queue(uv_default_loop(), [this](Action &action) { if (action.type == Action::Add) { processAdd(action.resource); @@ -56,6 +22,8 @@ NodeFileSource::NodeFileSource() : } })) { + NanAssignPersistent(options, options_->ToObject()); + // Make sure that the queue doesn't block the loop from exiting. queue->unref(); } @@ -63,6 +31,8 @@ NodeFileSource::NodeFileSource() : NodeFileSource::~NodeFileSource() { queue->stop(); queue = nullptr; + + NanDisposePersistent(options); } mbgl::Request* NodeFileSource::request(const mbgl::Resource& resource, uv_loop_t* loop, Callback callback) { @@ -119,15 +89,20 @@ void NodeFileSource::processAdd(const mbgl::Resource& resource) { queue->ref(); } - auto handle = NanObjectWrapHandle(this); - auto requestHandle = NanNew(NodeRequest::Create(handle, resource)); + auto requestHandle = NanNew(NodeRequest::Create(this, resource)); v8::Persistent requestPersistent; NanAssignPersistent(requestPersistent, requestHandle); pending.emplace(resource, std::move(requestPersistent)); +#if (NODE_MODULE_VERSION > NODE_0_10_MODULE_VERSION) + auto requestFunction = v8::Local::New(v8::Isolate::GetCurrent(), options)->Get(NanNew("request")).As(); +#else + auto requestFunction = options->Get(NanNew("request")).As(); +#endif + v8::Local argv[] = { requestHandle }; - NanMakeCallback(handle, NanNew("request"), 1, argv); + NanMakeCallback(NanGetCurrentContext()->Global(), requestFunction, 1, argv); } void NodeFileSource::processCancel(const mbgl::Resource& resource) { @@ -152,14 +127,20 @@ void NodeFileSource::processCancel(const mbgl::Resource& resource) { queue->unref(); } - auto handle = NanObjectWrapHandle(this); - if (handle->Has(NanNew("cancel"))) { +#if (NODE_MODULE_VERSION > NODE_0_10_MODULE_VERSION) + auto optionsObject = v8::Local::New(v8::Isolate::GetCurrent(), options); + if (optionsObject->Has(NanNew("cancel"))) { + auto cancelFunction = optionsObject->Get(NanNew("cancel")).As(); +#else + if (options->Has(NanNew("cancel"))) { + auto cancelFunction = options->Get(NanNew("cancel")).As(); +#endif v8::Local argv[] = { requestHandle }; - NanMakeCallback(handle, NanNew("cancel"), 1, argv); + NanMakeCallback(NanGetCurrentContext()->Global(), cancelFunction, 1, argv); } // Set the request handle in the request wrapper handle to null - ObjectWrap::Unwrap(requestHandle)->cancel(); + node::ObjectWrap::Unwrap(requestHandle)->cancel(); } } diff --git a/src/node_file_source.hpp b/src/node_file_source.hpp index 06b1c3d32..9dde87a2e 100644 --- a/src/node_file_source.hpp +++ b/src/node_file_source.hpp @@ -19,19 +19,9 @@ namespace node_mbgl { namespace util { template class AsyncQueue; } -class NodeFileSource : public node::ObjectWrap, public mbgl::FileSource { - //////////////////////////////////////////////////////////////////////////////////////////////// - // Static Node Methods +class NodeFileSource : public mbgl::FileSource { public: - static void Init(v8::Handle target); - static NAN_METHOD(New); - - static v8::Persistent constructorTemplate; - - //////////////////////////////////////////////////////////////////////////////////////////////// - // Instance -public: - NodeFileSource(); + NodeFileSource(v8::Handle); ~NodeFileSource(); mbgl::Request* request(const mbgl::Resource&, uv_loop_t*, Callback); @@ -47,6 +37,8 @@ private: void processAdd(const mbgl::Resource&); void processCancel(const mbgl::Resource&); + v8::Persistent options; + private: #if (NODE_MODULE_VERSION > NODE_0_10_MODULE_VERSION) std::unordered_map>, mbgl::Resource::Hash> pending; diff --git a/src/node_map.cpp b/src/node_map.cpp index e8dc28110..8a9245419 100644 --- a/src/node_map.cpp +++ b/src/node_map.cpp @@ -1,5 +1,4 @@ #include "node_map.hpp" -#include "node_file_source.hpp" #include #include @@ -61,23 +60,22 @@ NAN_METHOD(NodeMap::New) { return NanThrowTypeError("Use the new operator to create new Map objects"); } - if (args.Length() < 1 || !NanHasInstance(NodeFileSource::constructorTemplate, args[0])) { - return NanThrowTypeError("Requires a FileSource as first argument"); + if (args.Length() < 1 || !args[0]->IsObject()) { + return NanThrowTypeError("Requires FileSource options as first argument"); } - auto source = args[0]->ToObject(); + auto options = args[0]->ToObject(); // Check that request() and cancel() are defined. - if (!source->Has(NanNew("request")) || !source->Get(NanNew("request"))->IsFunction()) { - return NanThrowError("FileSource must have a request member function"); + if (!options->Has(NanNew("request")) || !options->Get(NanNew("request"))->IsFunction()) { + return NanThrowError("FileSource options must have a request member function"); } - if (!source->Has(NanNew("cancel")) || !source->Get(NanNew("cancel"))->IsFunction()) { - return NanThrowError("FileSource must have a cancel member function"); + if (!options->Has(NanNew("cancel")) || !options->Get(NanNew("cancel"))->IsFunction()) { + return NanThrowError("FileSource options must have a cancel member function"); } - try { - auto nodeMap = new NodeMap(source); + auto nodeMap = new NodeMap(options); nodeMap->Wrap(args.This()); } catch(std::exception &ex) { return NanThrowError(ex.what()); @@ -309,8 +307,6 @@ void NodeMap::release() { valid = false; - NanDisposePersistent(source); - uv_close(reinterpret_cast(async), [] (uv_handle_t *handle) { delete reinterpret_cast(handle); }); @@ -322,14 +318,12 @@ void NodeMap::release() { //////////////////////////////////////////////////////////////////////////////////////////////// // Instance -NodeMap::NodeMap(v8::Handle source_) : +NodeMap::NodeMap(v8::Handle options) : view(sharedDisplay()), - fs(*ObjectWrap::Unwrap(source_)), + fs(options), map(std::make_unique(view, fs, mbgl::MapMode::Still)), async(new uv_async_t) { - NanAssignPersistent(source, source_); - async->data = this; #if UV_VERSION_MAJOR == 0 && UV_VERSION_MINOR <= 10 uv_async_init(uv_default_loop(), async, [](uv_async_t *as, int) { diff --git a/src/node_map.hpp b/src/node_map.hpp index d442d93bb..0e2879800 100644 --- a/src/node_map.hpp +++ b/src/node_map.hpp @@ -1,5 +1,7 @@ #pragma once +#include "node_file_source.hpp" + #include #include @@ -14,8 +16,6 @@ namespace node_mbgl { -class NodeFileSource; - class NodeMap : public node::ObjectWrap { struct RenderOptions; class RenderWorker; @@ -52,7 +52,7 @@ private: v8::Persistent source; mbgl::HeadlessView view; - NodeFileSource &fs; + NodeFileSource fs; std::unique_ptr map; std::exception_ptr error; diff --git a/src/node_mapbox_gl_native.cpp b/src/node_mapbox_gl_native.cpp index 7a161b2ed..9ba0abbd0 100644 --- a/src/node_mapbox_gl_native.cpp +++ b/src/node_mapbox_gl_native.cpp @@ -5,7 +5,6 @@ #include #pragma GCC diagnostic pop -#include "node_file_source.hpp" #include "node_map.hpp" #include "node_log.hpp" #include "node_request.hpp" @@ -13,7 +12,6 @@ void RegisterModule(v8::Handle exports) { NanScope(); - node_mbgl::NodeFileSource::Init(exports); node_mbgl::NodeMap::Init(exports); node_mbgl::NodeRequest::Init(exports); diff --git a/src/node_request.cpp b/src/node_request.cpp index e01162ff5..b30211e1a 100644 --- a/src/node_request.cpp +++ b/src/node_request.cpp @@ -31,21 +31,22 @@ NAN_METHOD(NodeRequest::New) { NanScope(); // Extract the pointer from the first argument - if (args.Length() < 2 || !NanHasInstance(NodeFileSource::constructorTemplate, args[0]) || !args[1]->IsExternal()) { + if (args.Length() < 2 || !args[0]->IsExternal() || !args[1]->IsExternal()) { return NanThrowTypeError("Cannot create Request objects explicitly"); } + auto source = reinterpret_cast(args[0].As()->Value()); auto resource = reinterpret_cast(args[1].As()->Value()); - auto req = new NodeRequest(args[0]->ToObject(), *resource); + auto req = new NodeRequest(source, *resource); req->Wrap(args.This()); NanReturnValue(args.This()); } -v8::Handle NodeRequest::Create(v8::Handle source, const mbgl::Resource& resource) { +v8::Handle NodeRequest::Create(NodeFileSource* source, const mbgl::Resource& resource) { NanEscapableScope(); - v8::Local argv[] = { NanNew(source), + v8::Local argv[] = { NanNew(const_cast(source)), NanNew(const_cast(&resource)) }; auto instance = NanNew(constructorTemplate)->GetFunction()->NewInstance(2, argv); @@ -61,7 +62,7 @@ NAN_METHOD(NodeRequest::Respond) { return NanThrowError("Request has already been responded to, or was canceled."); } - auto source = ObjectWrap::Unwrap(NanNew(nodeRequest->source)); + auto source = nodeRequest->source; auto resource = std::move(nodeRequest->resource); if (args.Length() < 1) { @@ -128,14 +129,11 @@ NAN_METHOD(NodeRequest::Respond) { //////////////////////////////////////////////////////////////////////////////////////////////// // Instance -NodeRequest::NodeRequest(v8::Local source_, const mbgl::Resource& resource_) - : resource(std::make_unique(resource_)) { - NanAssignPersistent(source, source_); -} +NodeRequest::NodeRequest(NodeFileSource* source_, const mbgl::Resource& resource_) + : source(source_), + resource(std::make_unique(resource_)) {} -NodeRequest::~NodeRequest() { - NanDisposePersistent(source); -} +NodeRequest::~NodeRequest() {} void NodeRequest::cancel() { resource.reset(); diff --git a/src/node_request.hpp b/src/node_request.hpp index 0fbf91fc6..a690904ef 100644 --- a/src/node_request.hpp +++ b/src/node_request.hpp @@ -11,10 +11,10 @@ #include -class NodeFileSource; - namespace node_mbgl { +class NodeFileSource; + class NodeRequest : public node::ObjectWrap { //////////////////////////////////////////////////////////////////////////////////////////////// // Static Node Methods @@ -23,20 +23,20 @@ public: static NAN_METHOD(New); static NAN_METHOD(Respond); - static v8::Handle Create(v8::Handle source, const mbgl::Resource& resource); + static v8::Handle Create(NodeFileSource*, const mbgl::Resource&); static v8::Persistent constructorTemplate; //////////////////////////////////////////////////////////////////////////////////////////////// // Instance public: - NodeRequest(v8::Local source, const mbgl::Resource& resource); + NodeRequest(NodeFileSource* source, const mbgl::Resource& resource); ~NodeRequest(); void cancel(); private: - v8::Persistent source; + NodeFileSource* source; std::unique_ptr resource; }; diff --git a/test/js/concurrency.test.js b/test/js/concurrency.test.js index e57767fa7..52ed7cf68 100644 --- a/test/js/concurrency.test.js +++ b/test/js/concurrency.test.js @@ -16,16 +16,16 @@ function renderTest(style, info, dir, key) { var remaining = 10; var start = +new Date; - var fileSource = new mbgl.FileSource(); - fileSource.request = function(req) { + var options = {}; + options.request = function(req) { fs.readFile(path.join(suitePath, decodeURIComponent(req.url)), function(err, data) { req.respond(err, { data: data }); t.error(err); }); }; - fileSource.cancel = function() {}; + options.cancel = function() {}; - var map = new mbgl.Map(fileSource); + var map = new mbgl.Map(options); map.load(style); function render() { diff --git a/test/js/file_source.test.js b/test/js/file_source.test.js deleted file mode 100644 index 72f2180f2..000000000 --- a/test/js/file_source.test.js +++ /dev/null @@ -1,21 +0,0 @@ -'use strict'; - -/* jshint node: true */ - -var test = require('tape'); -var mbgl = require('../..'); - -test('FileSource', function(t) { - t.test('constructor', function(t) { - t.test('must be called with new', function(t) { - t.throws(function() { - mbgl.FileSource(); - }, /Use the new operator to create new FileSource objects/); - t.end(); - }); - - t.end(); - }); - - t.end(); -}); diff --git a/test/js/gzip.test.js b/test/js/gzip.test.js index c839dd5c2..b3c8724b5 100644 --- a/test/js/gzip.test.js +++ b/test/js/gzip.test.js @@ -27,14 +27,14 @@ function filePath(name) { }, {}); } -function setup(fileSource, callback) { - callback(new mbgl.Map(fileSource)); +function setup(options, callback) { + callback(new mbgl.Map(options)); } -function getFileSource(gzip, t) { - var fileSource = new mbgl.FileSource(); +function getFileSourceOptions(gzip, t) { + var options = {}; - fileSource.request = function(req) { + options.request = function(req) { var parts = req.url.split('.'); var filetype = parts[parts.length - 1]; @@ -53,9 +53,9 @@ function getFileSource(gzip, t) { }); }; - fileSource.cancel = function(req) {}; + options.cancel = function(req) {}; - return fileSource; + return options; } test('gzip', function(t) { @@ -64,7 +64,7 @@ test('gzip', function(t) { if (msg.severity == 'ERROR') t.error(msg); }); - setup(getFileSource(true, t), function(map) { + setup(getFileSourceOptions(true, t), function(map) { map.load(style); map.render({}, function(err, data) { mbgl.removeAllListeners('message'); @@ -110,7 +110,7 @@ test('gzip', function(t) { } }); - setup(getFileSource(false, t), function(map) { + setup(getFileSourceOptions(false, t), function(map) { map.load(style); map.render({}, function(err, data) { map.release(); diff --git a/test/js/map.test.js b/test/js/map.test.js index edeab8370..b60b21a6c 100644 --- a/test/js/map.test.js +++ b/test/js/map.test.js @@ -20,8 +20,8 @@ function filePath(name) { }, {}); } -function setup(fileSource, callback) { - callback(new mbgl.Map(fileSource)); +function setup(options, callback) { + callback(new mbgl.Map(options)); } test('Map', function(t) { @@ -34,47 +34,43 @@ test('Map', function(t) { t.end(); }); - t.test('should require a FileSource object as first parameter', function(t) { + t.test('should require a FileSource options object as first parameter', function(t) { t.throws(function() { new mbgl.Map(); - }, /Requires a FileSource as first argument/); + }, /Requires FileSource options as first argument/); t.throws(function() { - new mbgl.Map('fileSource'); - }, /Requires a FileSource as first argument/); - - t.throws(function() { - new mbgl.Map({}); - }, /Requires a FileSource as first argument/); + new mbgl.Map('options'); + }, /Requires FileSource options as first argument/); t.end(); }); - t.test('should require the FileSource object to have request and cancel methods', function(t) { - var fileSource = new mbgl.FileSource(); + t.test('should require the FileSource options object to have request and cancel methods', function(t) { + var options = {}; t.throws(function() { - new mbgl.Map(fileSource); - }, /FileSource must have a request member function/); + new mbgl.Map(options); + }, /FileSource options must have a request member function/); - fileSource.request = 'test'; + options.request = 'test'; t.throws(function() { - new mbgl.Map(fileSource); - }, /FileSource must have a request member function/); + new mbgl.Map(options); + }, /FileSource options must have a request member function/); - fileSource.request = function() {}; + options.request = function() {}; t.throws(function() { - new mbgl.Map(fileSource); - }, /FileSource must have a cancel member function/); + new mbgl.Map(options); + }, /FileSource options must have a cancel member function/); - fileSource.cancel = 'test'; + options.cancel = 'test'; t.throws(function() { - new mbgl.Map(fileSource); - }, /FileSource must have a cancel member function/); + new mbgl.Map(options); + }, /FileSource options must have a cancel member function/); - fileSource.cancel = function() {}; + options.cancel = function() {}; t.doesNotThrow(function() { - new mbgl.Map(fileSource); + new mbgl.Map(options); }); t.end(); @@ -84,13 +80,13 @@ test('Map', function(t) { }); t.test('load styles', function(t) { - var fileSource = new mbgl.FileSource(); - fileSource.request = function() {}; - fileSource.cancel = function() {}; + var options = {}; + options.request = function() {}; + options.cancel = function() {}; t.test('requires a string or object as the first parameter', function(t) { t.test('requires a map style as first argument', function(t) { - setup(fileSource, function(map) { + setup(options, function(map) { t.throws(function() { map.load(); }, /Requires a map style as first argument/); @@ -102,7 +98,7 @@ test('Map', function(t) { }); t.test('expect either an object or array at root', { timeout: 1000 }, function(t) { - setup(fileSource, function(map) { + setup(options, function(map) { mbgl.once('message', function(msg) { t.equal(msg.severity, 'ERROR'); t.equal(msg.class, 'ParseStyle'); @@ -121,7 +117,7 @@ test('Map', function(t) { }); t.test('accepts an empty stylesheet string', function(t) { - setup(fileSource, function(map) { + setup(options, function(map) { t.doesNotThrow(function() { map.load('{}'); }); @@ -133,7 +129,7 @@ test('Map', function(t) { }); t.test('accepts a JSON stylesheet', { timeout: 1000 }, function(t) { - setup(fileSource, function(map) { + setup(options, function(map) { t.doesNotThrow(function() { map.load(style); }); @@ -145,7 +141,7 @@ test('Map', function(t) { }); t.test('accepts a stringified stylesheet', { timeout: 1000 }, function(t) { - setup(fileSource, function(map) { + setup(options, function(map) { t.doesNotThrow(function() { map.load(JSON.stringify(style)); }); @@ -161,12 +157,12 @@ test('Map', function(t) { t.test('request coalesting', function(t) { var requestList = []; - var fileSourceDoNotReply = new mbgl.FileSource(); + var optionsDoNotReply = {}; // This file source will never reply to any // request other than "./fixtures/tiles.tilejson" which // will force the cancellation of the pending requests. - fileSourceDoNotReply.request = function(req) { + optionsDoNotReply.request = function(req) { if (req.url != "./fixtures/tiles.tilejson") { requestList.push(req); return; @@ -177,16 +173,16 @@ test('Map', function(t) { }); }; - fileSourceDoNotReply.cancel = function() { + optionsDoNotReply.cancel = function() { requestList.pop(); }; t.test('merge requests/cancels for the same resource', function(t) { - var map1 = new mbgl.Map(fileSourceDoNotReply); - var map2 = new mbgl.Map(fileSourceDoNotReply); - var map3 = new mbgl.Map(fileSourceDoNotReply); - var map4 = new mbgl.Map(fileSourceDoNotReply); - var map5 = new mbgl.Map(fileSourceDoNotReply); + var map1 = new mbgl.Map(optionsDoNotReply); + var map2 = new mbgl.Map(optionsDoNotReply); + var map3 = new mbgl.Map(optionsDoNotReply); + var map4 = new mbgl.Map(optionsDoNotReply); + var map5 = new mbgl.Map(optionsDoNotReply); map1.load(style); map2.load(style); @@ -231,13 +227,13 @@ test('Map', function(t) { }); var vectorTileRequestCount = 0; - var fileSourceDelay = new mbgl.FileSource(); + var optionsDelay = {}; // This file source will add a little delay before // replying so we make sure that all the map objects // request the same resource will get a reply from the // same request. - fileSourceDelay.request = function(req) { + optionsDelay.request = function(req) { var timeout = 0; if (req.url != "./fixtures/tiles.tilejson") { @@ -254,16 +250,16 @@ test('Map', function(t) { setTimeout(readFile, timeout); }; - fileSourceDelay.cancel = function() { + optionsDelay.cancel = function() { t.fail("Should never cancel"); }; t.test('one request for the same resource notifies multiple map objects', function(t) { - var map1 = new mbgl.Map(fileSourceDelay); - var map2 = new mbgl.Map(fileSourceDelay); - var map3 = new mbgl.Map(fileSourceDelay); - var map4 = new mbgl.Map(fileSourceDelay); - var map5 = new mbgl.Map(fileSourceDelay); + var map1 = new mbgl.Map(optionsDelay); + var map2 = new mbgl.Map(optionsDelay); + var map3 = new mbgl.Map(optionsDelay); + var map4 = new mbgl.Map(optionsDelay); + var map5 = new mbgl.Map(optionsDelay); map1.load(style); map2.load(style); @@ -298,16 +294,16 @@ test('Map', function(t) { }); t.test('render argument requirements', function(t) { - var fileSource = new mbgl.FileSource(); - fileSource.request = function(req) { + var options = {}; + options.request = function(req) { fs.readFile(path.join('test', req.url), function(err, data) { req.respond(err, { data: data }); }); }; - fileSource.cancel = function() {}; + options.cancel = function() {}; t.test('requires an object as the first parameter', function(t) { - setup(fileSource, function(map) { + setup(options, function(map) { t.throws(function() { map.render(); }, /First argument must be an options object/); @@ -323,7 +319,7 @@ test('Map', function(t) { }); t.test('requires a callback as the second parameter', function(t) { - setup(fileSource, function(map) { + setup(options, function(map) { t.throws(function() { map.render({}); }, /Second argument must be a callback function/); @@ -339,7 +335,7 @@ test('Map', function(t) { }); t.test('requires a style to be set', function(t) { - setup(fileSource, function(map) { + setup(options, function(map) { t.throws(function() { map.render({}, function() {}); }, /Style is not loaded/); @@ -358,7 +354,7 @@ test('Map', function(t) { t.ok(msg.text.match(/Failed to load/), 'error text matches'); }); - setup(fileSource, function(map) { + setup(options, function(map) { map.load(style); map.render({ zoom: 1 }, function(err, data) { mbgl.removeAllListeners('message'); @@ -373,7 +369,7 @@ test('Map', function(t) { }); t.test('double release', function(t) { - setup(fileSource, function(map) { + setup(options, function(map) { map.release(); t.throws(function() { @@ -385,7 +381,7 @@ test('Map', function(t) { }); t.test('returns an image', function(t) { - setup(fileSource, function(map) { + setup(options, function(map) { map.load(style); map.render({}, function(err, data) { t.error(err); diff --git a/test/render.test.js b/test/render.test.js index f55fa5608..2468b7242 100644 --- a/test/render.test.js +++ b/test/render.test.js @@ -39,16 +39,16 @@ function renderTest(style, info, base, key) { clearTimeout(watchdog); }); - var fileSource = new mbgl.FileSource(); - fileSource.request = function(req) { + var options = {}; + options.request = function(req) { fs.readFile(path.join(suitePath, decodeURIComponent(req.url)), function(err, data) { req.respond(err, { data: data }); t.error(err); }); }; - fileSource.cancel = function() {}; + options.cancel = function() {}; - var map = new mbgl.Map(fileSource); + var map = new mbgl.Map(options); map.load(style); map.render(info[key], function(err, data) {