From 1035d2eb7d9dc60ec367b165594c44495388bacf Mon Sep 17 00:00:00 2001 From: Kenny Yuan Date: Tue, 13 Dec 2016 14:34:32 +0800 Subject: [PATCH 1/2] Re-implement ArrayBuffer, now using v8::ArrayBuffer type instead of node's buffer. v8::ArrayBuffer can work on an external buffer without owning it. Closes #90 --- templates/helperHeader.dot | 68 ++++++++++++++-- templates/helperHeaderArray.dot | 2 +- templates/nanCxxHeaderPropertyVars.def | 2 +- templates/nanCxxImplMethod.def | 6 +- templates/nanCxxImplProperty.def | 14 +--- templates/nanCxxImplPropertyVarsRelease.def | 2 +- test/{buffer => arraybuffer}/addon.cpp | 0 .../arraybuffer.widl} | 2 + test/{buffer => arraybuffer}/binding.gyp | 0 test/arraybuffer/buffer.cpp | 24 ++++++ test/arraybuffer/buffer.h | 43 ++++++++++ test/buffer/buffer.cpp | 27 ------- test/buffer/buffer.h | 38 --------- test/test-arraybuffer.js | 79 +++++++++++++++++++ test/test-buffer.js | 57 ------------- 15 files changed, 214 insertions(+), 150 deletions(-) rename test/{buffer => arraybuffer}/addon.cpp (100%) rename test/{buffer/buffer.widl => arraybuffer/arraybuffer.widl} (88%) rename test/{buffer => arraybuffer}/binding.gyp (100%) create mode 100644 test/arraybuffer/buffer.cpp create mode 100644 test/arraybuffer/buffer.h delete mode 100644 test/buffer/buffer.cpp delete mode 100644 test/buffer/buffer.h create mode 100644 test/test-arraybuffer.js delete mode 100644 test/test-buffer.js diff --git a/templates/helperHeader.dot b/templates/helperHeader.dot index d3d383c..a03b84c 100644 --- a/templates/helperHeader.dot +++ b/templates/helperHeader.dot @@ -11,9 +11,63 @@ #define EXTRACT_v8_String(x) EXTRACT_v8_string(x) #define EXTRACT_v8_float(x) EXTRACT_v8_double(x) -struct ArrayBuffer { - char* data; - uint32_t size; +class ArrayBufferHelper { + public: + ArrayBufferHelper(void* data, uint32_t length) { + Set(data, length); + } + + ArrayBufferHelper(const ArrayBufferHelper& rhs) { + CopyFrom(rhs); + } + + ~ArrayBufferHelper() {} + + ArrayBufferHelper& operator = (const ArrayBufferHelper& rhs) { + CopyFrom(rhs); + return *this; + } + + void Set(void* data, uint32_t length, bool external = true) { + data_ = data; + length_ = length; + external_buffer_ = external; + } + + v8::Local GetArrayBuffer() const { + auto mode = external_buffer_ ? + v8::ArrayBufferCreationMode::kExternalized : + v8::ArrayBufferCreationMode::kInternalized; + + return v8::ArrayBuffer::New( + v8::Isolate::GetCurrent(), data_, length_, mode); + } + + bool operator == (const ArrayBufferHelper& rhs) { + if (this != &rhs) { + return data_ == rhs.data_ && + length_ == rhs.length_ && + external_buffer_ == rhs.external_buffer_; + } + return true; + } + + void* GetData() const { return data_; } + uint32_t GetLength() const { return length_; } + bool IsExternalBuffer() const { return external_buffer_; } + + private: + void CopyFrom(const ArrayBufferHelper& rhs) { + if (this != &rhs) { + data_ = rhs.data_; + length_ = rhs.length_; + external_buffer_ = rhs.external_buffer_; + } + } + + void* data_; + uint32_t length_; + bool external_buffer_; }; inline v8::Local EXTRACT_v8_array(v8::Local value) { @@ -163,16 +217,14 @@ inline v8::Local EXTRACT_v8_function(v8::Local value, i return v8::Local::Cast(value); } -inline ArrayBuffer EXTRACT_v8_ArrayBuffer(v8::Local value) { +inline ArrayBufferHelper EXTRACT_v8_ArrayBuffer(v8::Local value) { if (value->IsObject()) { } else { // throw ... } - ArrayBuffer arrayBuffer; - arrayBuffer.data = node::Buffer::Data(value->ToObject()); - arrayBuffer.size = node::Buffer::Length(value->ToObject()); - return arrayBuffer; + v8::ArrayBuffer* buffer = v8::ArrayBuffer::Cast(*value); + return ArrayBufferHelper(buffer->GetContents().Data(), buffer->GetContents().ByteLength()); } // Helper: to deal with const members in interface of *.idl file diff --git a/templates/helperHeaderArray.dot b/templates/helperHeaderArray.dot index 3e5d349..a47de44 100644 --- a/templates/helperHeaderArray.dot +++ b/templates/helperHeaderArray.dot @@ -63,7 +63,7 @@ class ArrayHelper { double GetDouble(uint32_t index) const { return EXTRACT_v8_double(v8_array_->Get(index)); } std::string GetString(uint32_t index) const { return EXTRACT_v8_string(v8_array_->Get(index)); } v8::Local GetValue(uint32_t index) const; - ArrayBuffer GetArrayBuffer(uint32_t index) const { return EXTRACT_v8_ArrayBuffer(v8_array_->Get(index)); } + ArrayBufferHelper GetArrayBuffer(uint32_t index) const { return EXTRACT_v8_ArrayBuffer(v8_array_->Get(index)); } v8::Local GetObject(uint32_t index) const { return EXTRACT_v8_object(v8_array_->Get(index)); } v8::Local GetArray(uint32_t index) const { return EXTRACT_v8_array(v8_array_->Get(index)); } v8::Local GetFunction(uint32_t index) const { return EXTRACT_v8_function(v8_array_->Get(index)); } diff --git a/templates/nanCxxHeaderPropertyVars.def b/templates/nanCxxHeaderPropertyVars.def index 5274f28..78bb506 100644 --- a/templates/nanCxxHeaderPropertyVars.def +++ b/templates/nanCxxHeaderPropertyVars.def @@ -8,7 +8,7 @@ if (p.static) { staticStr = 'static '; } }} -{{? isInterface(p.idlType, it.refTypeMap) || (p.idlType.idlType == 'ArrayBuffer') }} +{{? isInterface(p.idlType, it.refTypeMap) }} {{=staticStr}}std::unique_ptr> {{=p.name}}_; {{?}} {{~}} diff --git a/templates/nanCxxImplMethod.def b/templates/nanCxxImplMethod.def index e4bb42d..c9cf8dc 100644 --- a/templates/nanCxxImplMethod.def +++ b/templates/nanCxxImplMethod.def @@ -128,10 +128,8 @@ var argStr = args.join(', '); info.GetReturnValue().Set(Nan::New(value).ToLocalChecked()); {{?? method.idlType.idlType === 'ArrayBuffer' }} // Return type is ArrayBuffer - ArrayBuffer arrayBuffer = {{=functionCallPrefix}}{{=methodName}}({{=argStr}}); - v8::Local value; - if (Nan::NewBuffer(arrayBuffer.data, arrayBuffer.size).ToLocal(&value)) - info.GetReturnValue().Set(value); + auto arrayBuffer = {{=functionCallPrefix}}{{=methodName}}({{=argStr}}); + info.GetReturnValue().Set(arrayBuffer.GetArrayBuffer()); {{?? isInterface(method.idlType, it.refTypeMap)}} // Return type is an interface object auto implObj = {{=functionCallPrefix}}{{=methodName}}({{=argStr}}); diff --git a/templates/nanCxxImplProperty.def b/templates/nanCxxImplProperty.def index 49cb33a..061296e 100644 --- a/templates/nanCxxImplProperty.def +++ b/templates/nanCxxImplProperty.def @@ -68,19 +68,7 @@ NAN_GETTER({{=className}}::{{=p.name}}Getter) { {{?? isTypedArray(p.idlType) }} info.GetReturnValue().Set(impl_val.GetTypedArray()); {{?? p.idlType.idlType === 'ArrayBuffer' }} - if (! {{=myself}}{{=p.name}}_) { - v8::Local value; - if(Nan::NewBuffer(impl_val.data, impl_val.size).ToLocal(&value)) { - {{=myself}}{{=p.name}}_.reset(new Nan::Persistent(value)); - } - } - - if ({{=myself}}{{=p.name}}_) { - v8::Local obj = Nan::New(*{{=myself}}{{=p.name}}_); - info.GetReturnValue().Set(obj); - } else { - info.GetReturnValue().Set(Nan::Undefined()); - } + info.GetReturnValue().Set(impl_val.GetArrayBuffer()); {{?? true}} info.GetReturnValue().Set(Nan::New(impl_val){{=str}}); {{?}} diff --git a/templates/nanCxxImplPropertyVarsRelease.def b/templates/nanCxxImplPropertyVarsRelease.def index 60d049d..7785d24 100644 --- a/templates/nanCxxImplPropertyVarsRelease.def +++ b/templates/nanCxxImplPropertyVarsRelease.def @@ -2,7 +2,7 @@ {{#def.members}} {{~ getNonStaticPropertyMembers(it) :p:i }} -{{? isInterface(p.idlType, it.refTypeMap) || (p.idlType.idlType === 'ArrayBuffer')}} +{{? isInterface(p.idlType, it.refTypeMap) }} {{=p.name}}_.reset(); {{?}} {{~}} diff --git a/test/buffer/addon.cpp b/test/arraybuffer/addon.cpp similarity index 100% rename from test/buffer/addon.cpp rename to test/arraybuffer/addon.cpp diff --git a/test/buffer/buffer.widl b/test/arraybuffer/arraybuffer.widl similarity index 88% rename from test/buffer/buffer.widl rename to test/arraybuffer/arraybuffer.widl index 83e3127..6b85733 100644 --- a/test/buffer/buffer.widl +++ b/test/arraybuffer/arraybuffer.widl @@ -8,4 +8,6 @@ interface Buffer { ArrayBuffer getArrayBuffer(); DOMString buffer2String(ArrayBuffer buffer); + + static ArrayBuffer getCommonData(); }; diff --git a/test/buffer/binding.gyp b/test/arraybuffer/binding.gyp similarity index 100% rename from test/buffer/binding.gyp rename to test/arraybuffer/binding.gyp diff --git a/test/arraybuffer/buffer.cpp b/test/arraybuffer/buffer.cpp new file mode 100644 index 0000000..8ba8fa5 --- /dev/null +++ b/test/arraybuffer/buffer.cpp @@ -0,0 +1,24 @@ +// Copyright (c) 2016 Intel Corporation. All rights reserved. +// Use of this source code is governed by a MIT-style license that can be +// found in the LICENSE file. + +#include "buffer.h" + +#include + +Buffer::Buffer() { + data_ = "hello world!"; +} + +Buffer::~Buffer() { +} + +ArrayBufferHelper Buffer::getArrayBuffer() const { + char* text = "hello world!"; + return ArrayBufferHelper(text, strlen(text)); +} + +ArrayBufferHelper Buffer::getCommonData() { + static char* raw_buf = "static buffer test data"; + return ArrayBufferHelper(raw_buf, strlen(raw_buf)); +} diff --git a/test/arraybuffer/buffer.h b/test/arraybuffer/buffer.h new file mode 100644 index 0000000..39cdf50 --- /dev/null +++ b/test/arraybuffer/buffer.h @@ -0,0 +1,43 @@ +// Copyright (c) 2016 Intel Corporation. All rights reserved. +// Use of this source code is governed by a MIT-style license that can be +// found in the LICENSE file. + +#ifndef _BUFFER_H_ +#define _BUFFER_H_ + +#include +#include + +#include + +#include "gen/generator_helper.h" + +class Buffer { + public: + Buffer(); + ~Buffer(); + + public: + ArrayBufferHelper getArrayBuffer() const; + + ArrayBufferHelper get_data() const { + return ArrayBufferHelper((void*)data_.c_str(), data_.length()); + } + + void set_data(const ArrayBufferHelper& buffer) { + data_ = std::string((const char*)buffer.GetData(), + buffer.GetLength()); + } + + std::string buffer2String(const ArrayBufferHelper& arrayBuffer) { + return std::string((const char*)arrayBuffer.GetData(), + arrayBuffer.GetLength()); + } + + static ArrayBufferHelper getCommonData(); + + private: + std::string data_; +}; + +#endif // _BUFFER_H_ diff --git a/test/buffer/buffer.cpp b/test/buffer/buffer.cpp deleted file mode 100644 index 5c3cb84..0000000 --- a/test/buffer/buffer.cpp +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright (c) 2016 Intel Corporation. All rights reserved. -// Use of this source code is governed by a MIT-style license that can be -// found in the LICENSE file. - -#include "buffer.h" - -#include - -Buffer::Buffer() { -} - -Buffer::~Buffer() { -} - -ArrayBuffer Buffer::getArrayBuffer() const { - const char* text = "hello world!"; - - uint32_t size = strlen(text); - // Use malloc to allocate memory for node array buffer. - char* buf = reinterpret_cast(malloc(size)); - memcpy(buf, text, size); - - ArrayBuffer arrayBuffer; - arrayBuffer.data = buf; - arrayBuffer.size = size; - return arrayBuffer; -} diff --git a/test/buffer/buffer.h b/test/buffer/buffer.h deleted file mode 100644 index 41fed1d..0000000 --- a/test/buffer/buffer.h +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) 2016 Intel Corporation. All rights reserved. -// Use of this source code is governed by a MIT-style license that can be -// found in the LICENSE file. - -#ifndef _BUFFER_H_ -#define _BUFFER_H_ - -#include -#include - -#include - -#include "gen/generator_helper.h" - -class Buffer { - public: - Buffer(); - ~Buffer(); - - public: - ArrayBuffer getArrayBuffer() const; - - ArrayBuffer get_data() const { - return getArrayBuffer(); - } - - void set_data(const ArrayBuffer& new_value) { - } - - std::string buffer2String(const ArrayBuffer& arrayBuffer) { - return std::string(arrayBuffer.data, arrayBuffer.size); - } - - private: - ArrayBuffer data_; -}; - -#endif // _BUFFER_H_ diff --git a/test/test-arraybuffer.js b/test/test-arraybuffer.js new file mode 100644 index 0000000..de06274 --- /dev/null +++ b/test/test-arraybuffer.js @@ -0,0 +1,79 @@ +// Copyright (c) 2016 Intel Corporation. All rights reserved. +// Use of this source code is governed by a MIT-style license that can be +// found in the LICENSE file. + +'use strict'; + +/* global describe, it */ +var assert = require('assert'); +var buildAddon = require('../lib/addon-builder.js').buildAddon; +var compile = require('../lib/compile.js').compile; +var path = require('path'); + +var Buffer; + +function bufToString(buf) { + return String.fromCharCode.apply(null, new Uint8Array(buf)); +} + +function stringToArrayBuffer(str) { + var buf = new ArrayBuffer(str.length); + var bufView = new Uint8Array(buf); + for (var i = 0, strLen = str.length; i < strLen; ++i) { + bufView[i] = str.charCodeAt(i); + } + return buf; +} + +describe('widl-nan Unit Test - Buffer', function() { + it('Generating binding C++ code', function() { + return compile([ + 'test/arraybuffer/arraybuffer.widl' + ], 'test/arraybuffer/gen'); + }); + + it('Building addon', function() { + // building addon maybe slow + this.timeout(100000); + + return buildAddon('test/arraybuffer'); + }); + + it('Loading addon', function() { + var addonDir = path.join(path.dirname(__filename), 'arraybuffer'); + var addon = require('bindings')( + // eslint-disable-next-line camelcase + {bindings: 'testerAddon', module_root: addonDir}); + + Buffer = addon.Buffer; + assert.equal(typeof Buffer, 'function'); + }); + + it('Method returning an ArrayBuffer', done => { + var buf = new Buffer(); + assert.equal(bufToString(buf.getArrayBuffer()), 'hello world!'); + done(); + }); + + it('Static method returning an ArrayBuffer', done => { + assert.equal(bufToString(Buffer.getCommonData()), 'static buffer test data'); + assert.equal(bufToString(Buffer.getCommonData()), 'static buffer test data'); + assert.equal(bufToString(Buffer.getCommonData()), 'static buffer test data'); + done(); + }); + + it('Object property as ArrayBuffer', done => { + var buf = new Buffer(); + assert.equal(bufToString(buf.data), 'hello world!'); + buf.data = stringToArrayBuffer('hello universe!'); + assert.equal(bufToString(buf.data), 'hello universe!'); + done(); + }); + + it('Passing ArrayBuffer as paramter', done => { + var buf = new Buffer(); + const buffer = stringToArrayBuffer('Array buffer string test...'); + assert.equal(buf.buffer2String(buffer), 'Array buffer string test...'); + done(); + }); +}); diff --git a/test/test-buffer.js b/test/test-buffer.js deleted file mode 100644 index c18c1c8..0000000 --- a/test/test-buffer.js +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright (c) 2016 Intel Corporation. All rights reserved. -// Use of this source code is governed by a MIT-style license that can be -// found in the LICENSE file. - -'use strict'; - -/* global describe, it */ -var assert = require('assert'); -var buildAddon = require('../lib/addon-builder.js').buildAddon; -var compile = require('../lib/compile.js').compile; -var path = require('path'); - -var BufferTest; - -describe('widl-nan Unit Test - Buffer', function() { - it('Generating binding C++ code', function() { - return compile([ - 'test/buffer/buffer.widl' - ], 'test/buffer/gen'); - }); - - it('Building addon', function() { - // building addon maybe slow - this.timeout(100000); - - return buildAddon('test/buffer'); - }); - - it('Loading addon', function() { - var addonDir = path.join(path.dirname(__filename), 'buffer'); - var addon = require('bindings')( - // eslint-disable-next-line camelcase - {bindings: 'testerAddon', module_root: addonDir}); - - BufferTest = addon.Buffer; - assert.equal(typeof BufferTest, 'function'); - }); - - it('Method returning an ArrayBuffer', done => { - var bufferTest = new BufferTest(); - assert.equal(bufferTest.getArrayBuffer().toString(), 'hello world!'); - done(); - }); - - it('Object property as ArrayBuffer', done => { - var bufferTest = new BufferTest(); - assert.equal(bufferTest.data.toString(), 'hello world!'); - done(); - }); - - it('Passing ArrayBuffer as paramter', done => { - var bufferTest = new BufferTest(); - const buffer = new Buffer('This is node Buffer'); - assert.equal(bufferTest.buffer2String(buffer), 'This is node Buffer'); - done(); - }); -}); From 7a4a43f1205b9635ab18221bcf266144ac3f4818 Mon Sep 17 00:00:00 2001 From: Kenny Yuan Date: Tue, 13 Dec 2016 14:39:35 +0800 Subject: [PATCH 2/2] Make lint tool happy --- test/arraybuffer/buffer.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/arraybuffer/buffer.h b/test/arraybuffer/buffer.h index 39cdf50..e31e4e1 100644 --- a/test/arraybuffer/buffer.h +++ b/test/arraybuffer/buffer.h @@ -21,7 +21,9 @@ class Buffer { ArrayBufferHelper getArrayBuffer() const; ArrayBufferHelper get_data() const { - return ArrayBufferHelper((void*)data_.c_str(), data_.length()); + return ArrayBufferHelper( + const_cast(reinterpret_cast(data_.c_str())), + data_.length()); } void set_data(const ArrayBufferHelper& buffer) {