From 5c2df852cd34d3f03621179b24d08857ae81e44b Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 6 Nov 2020 11:13:09 -0800 Subject: [PATCH] converter: implement SameObject Generate idempotent versions of property getter results for attributes marked as `SameObject`. --- index.js | 82 ++++++++++++++++++++++++++++------------ test/class/class-impl.cc | 2 +- test/class/class-impl.h | 9 +++++ test/class/class.idl | 7 ++++ test/class/test.js | 14 +++++++ test/webgpu/test.js | 6 +++ webidl-napi-inl.h | 76 +++++++++++++++++++++++++++++++++---- webidl-napi.h | 23 +++++++++-- 8 files changed, 183 insertions(+), 36 deletions(-) diff --git a/index.js b/index.js index 56166de..d20957f 100755 --- a/index.js +++ b/index.js @@ -372,7 +372,7 @@ function generateParamRetrieval(sigs, maxArgs, isForConstructor) { ].join('\n'); } -function generateCall(ifname, sig, indent) { +function generateCall(ifname, sig, indent, sameObjAttrCount) { function argToNativeCall(idlType, index, indent) { return [ `NAPI_CALL(`, @@ -412,10 +412,9 @@ function generateCall(ifname, sig, indent) { // If this is not a static method or a constructor, declare and retrieve the // native instance `cc_rcv` corresponding to the JS instance in `js_rcv`. ...((sig.special !== 'static' && sig.type != 'constructor') ? [ - `void* cc_rcv_raw;`, `${ifname}* cc_rcv;`, - `NAPI_CALL(env, napi_unwrap(env, js_rcv, &cc_rcv_raw));`, - `cc_rcv = static_cast<${ifname}*>(cc_rcv_raw);`, + `NAPI_CALL(env,`, + ` WebIdlNapi::Wrapping<${ifname}>::Retrieve(env, js_rcv, &cc_rcv));`, `` ] : []) ] : []), @@ -448,13 +447,11 @@ function generateCall(ifname, sig, indent) { // it into the JS object we're constructing. ...(sig.type === 'constructor' ? [ `NAPI_CALL(env,`, - ` napi_wrap(`, + ` WebIdlNapi::Wrapping<${ifname}>::Create(`, ` env,`, ` js_rcv,`, - ` static_cast(ret),`, - ` &WebIdlNapi::ObjectWrapDestructor<${ifname}>,`, - ` nullptr,`, - ` nullptr));` + ` ret,`, + ` ${sameObjAttrCount}));` ] : []), // Special handling for promises. We need to call `Conclude()` before // returning to JS to at least create the `napi_deferred` and even resolve @@ -468,7 +465,7 @@ function generateCall(ifname, sig, indent) { .join('\n'); } -function generateIfaceOperation(ifname, opname, sigs) { +function generateIfaceOperation(ifname, opname, sigs, sameObjAttrCount) { if (sigs.length === 0) { // If we have no signatures, generate a trivial one. sigs = [ { @@ -508,7 +505,10 @@ function generateIfaceOperation(ifname, opname, sigs) { // Handle the case in a constructor where the `argv[0]` was an external. ...(opname === 'constructor' ? [ ` if (external != nullptr) {`, - generateCall(ifname, { type: 'constructor', external: true }, ' '), + generateCall(ifname, + { type: 'constructor', external: true }, + ' ', + sameObjAttrCount), ` return nullptr;`, ` }`, ] : []), @@ -520,10 +520,10 @@ function generateIfaceOperation(ifname, opname, sigs) { ...(sigs.length > 1 ? [ sigs.map((sig, index) => [ ` if (sig_idx == ${index}) {`, - generateCall(ifname, sig, ' '), + generateCall(ifname, sig, ' ', sameObjAttrCount), ' }' ].join('\n')).join('\n else\n') ] - : [ generateCall(ifname, sigs[0], ' ') ]), + : [ generateCall(ifname, sigs[0], ' ', sameObjAttrCount) ]), // If the op has a return type, compute it and store the resulting // `napi_value` in `js_ret`. ...(hasReturn ? [ @@ -539,7 +539,7 @@ function generateIfaceOperation(ifname, opname, sigs) { ].join('\n'); } -function generateIfaceAttribute(ifname, attribute) { +function generateIfaceAttribute(ifname, attribute, sameObjIdx) { const nativeAttributeType = generateNativeType(attribute.idlType); function generateAccessor(slug) { return [ @@ -566,9 +566,25 @@ function generateIfaceAttribute(ifname, attribute) { ` &js_rcv,`, ` nullptr));`, ``, - ` void* raw_data;`, - ` NAPI_CALL(env, napi_unwrap(env, js_rcv, &raw_data));`, - ` ${ifname}* cc_rcv = static_cast<${ifname}*>(raw_data);`, + ` ${ifname}* cc_rcv;`, + ...((sameObjIdx >= 0 && slug === 'get') ? [ + ` WebIdlNapi::Wrapping<${ifname}>* wrapping;`, + ` NAPI_CALL(env,`, + ` WebIdlNapi::Wrapping<${ifname}>::Retrieve(`, + ` env,`, + ` js_rcv,`, + ` &cc_rcv,`, + ` ${sameObjIdx},`, + ` &result,`, + ` &wrapping));`, + ` if (result != nullptr) return result;` + ] : [ + ` NAPI_CALL(env,`, + ` WebIdlNapi::Wrapping<${ifname}>::Retrieve(`, + ` env,`, + ` js_rcv,`, + ` &cc_rcv));`, + ]), ``, ...(slug === 'set' ? [ ` NAPI_CALL(`, @@ -584,6 +600,9 @@ function generateIfaceAttribute(ifname, attribute) { ` env,`, ` cc_rcv->${attribute.name},`, ` &result));`, + ...((sameObjIdx >= 0 && slug === 'get') ? [ + ` NAPI_CALL(env, wrapping->SetRef(env, ${sameObjIdx}, result));`, + ] : []) ]), ` return result;`, `}`, @@ -708,11 +727,12 @@ function generateIfaceConverters(ifaceName) { ` napi_env env,`, ` napi_value val,`, ` ${ifaceName}* result) {`, - ` void* data;`, - ` napi_status status = napi_unwrap(env, val, &data);`, + ` ${ifaceName}* data;`, + ` napi_status status =`, + ` WebIdlNapi::Wrapping<${ifaceName}>::Retrieve(env, val, &data);`, ` if (status != napi_ok) return status;`, ``, - ` *result = *static_cast<${ifaceName}*>(data);`, + ` *result = *data;`, ` return napi_ok;`, `}` ].join('\n'); @@ -731,8 +751,17 @@ function generateIface(iface) { const collapsedCtors = iface.members.filter((item) => (item.type === 'constructor')) - const attributes = iface.members - .filter((item) => (item.type === 'attribute')); + const { attrs, sameObjAttrs } = + iface.members.reduce((soFar, item) => { + if (item.type === 'attribute') { + const list = ((item.extAttrs.filter(({ name }) => + (name === 'SameObject'))).length > 0) + ? 'sameObjAttrs' + : 'attrs'; + soFar[list].push(item); + } + return soFar; + }, { attrs: [], sameObjAttrs: [] }); return [ [ @@ -747,11 +776,14 @@ function generateIface(iface) { // `generateIfaceOperation`. That way, only one binding is generated for all // signatures of an operation. generateIfaceConverters(iface.name), - generateIfaceOperation(iface.name, 'constructor', collapsedCtors), + generateIfaceOperation(iface.name, 'constructor', collapsedCtors, + sameObjAttrs.length), ...Object.entries(collapsedOps).map(([opname, sigs]) => generateIfaceOperation(iface.name, opname, sigs)), - ...attributes.map((item) => generateIfaceAttribute(iface.name, item)), - generateIfaceInit(iface.name, collapsedOps, attributes) + ...attrs.map((item) => generateIfaceAttribute(iface.name, item)), + ...sameObjAttrs.map((item, idx) => + generateIfaceAttribute(iface.name, item, idx)), + generateIfaceInit(iface.name, collapsedOps, [...attrs, ...sameObjAttrs]) ].join('\n\n'); } diff --git a/test/class/class-impl.cc b/test/class/class-impl.cc index 279ecfc..004a36e 100644 --- a/test/class/class-impl.cc +++ b/test/class/class-impl.cc @@ -32,7 +32,7 @@ Incrementor::Incrementor(): Incrementor(0) {} Incrementor::Incrementor(DOMString initial_value) : Incrementor(std::stoul(initial_value)) {} -Incrementor::Incrementor(unsigned long initial_value) { +Incrementor::Incrementor(unsigned long initial_value): props{"blah", 42} { val = new value__(initial_value); val->val = initial_value; } diff --git a/test/class/class-impl.h b/test/class/class-impl.h index c853bf2..e23e241 100644 --- a/test/class/class-impl.h +++ b/test/class/class-impl.h @@ -6,6 +6,11 @@ typedef struct value__* Value; class Incrementor; +struct Properties { + DOMString name; + unsigned long count; +}; + class Decrementor { public: void operator=(const Decrementor& other); @@ -23,6 +28,10 @@ class Incrementor { Incrementor(unsigned long initial); Incrementor(DOMString initial); unsigned long increment(); + + Properties props; + Properties settableProps; + Decrementor getDecrementor(); friend class Decrementor; ~Incrementor(); diff --git a/test/class/class.idl b/test/class/class.idl index 7f74f5f..3a95688 100644 --- a/test/class/class.idl +++ b/test/class/class.idl @@ -3,10 +3,17 @@ interface Decrementor { unsigned long decrement(); }; +dictionary Properties { + DOMString name; + unsigned long count; +}; + interface Incrementor { constructor(); constructor(unsigned long initial); constructor(DOMString initial); unsigned long increment(); Decrementor getDecrementor(); + [SameObject] readonly attribute Properties props; + attribute Properties settableProps; }; diff --git a/test/class/test.js b/test/class/test.js index 241ff53..9eddd6a 100644 --- a/test/class/test.js +++ b/test/class/test.js @@ -4,6 +4,20 @@ const assert = require('assert'); test(require('bindings')({ bindings: 'class', module_root: __dirname })); function test(binding) { + { + const inc = new binding.Incrementor(49); + const props1 = inc.props; + const props2 = inc.props; + assert.strictEqual(props1, props2); + + const settableProps1 = inc.settableProps; + const settableProps2 = inc.settableProps; + assert.deepStrictEqual(settableProps1, settableProps2); + + const expectedNewValue = { name: "new name", count: 99 }; + inc.settableProps = expectedNewValue; + assert.deepStrictEqual(inc.settableProps, expectedNewValue); + } { assert.strictEqual((new binding.Incrementor(12)).increment(), 13); assert.strictEqual((new binding.Incrementor()).increment(), 1); diff --git a/test/webgpu/test.js b/test/webgpu/test.js index db57380..5046ba4 100644 --- a/test/webgpu/test.js +++ b/test/webgpu/test.js @@ -19,4 +19,10 @@ async function test(binding) { await testAdapter({ powerPreference: 'high-performance' }); await testAdapter(); + + // Make sure the `gpu` property is `SameObject`. + const nav = new binding.Navigator(); + const gpu1 = nav.gpu; + const gpu2 = nav.gpu; + assert.strictEqual(gpu1, gpu2); } diff --git a/webidl-napi-inl.h b/webidl-napi-inl.h index adbd311..45d3576 100644 --- a/webidl-napi-inl.h +++ b/webidl-napi-inl.h @@ -238,13 +238,6 @@ inline napi_status IsConstructCall(napi_env env, return status; } -template -static void ObjectWrapDestructor(napi_env env, void* data, void* hint) { - (void) env; - (void) hint; - delete reinterpret_cast(data); -} - inline napi_status PickSignature(napi_env env, size_t argc, napi_value* argv, @@ -435,5 +428,74 @@ inline napi_ref InstanceData::GetConstructor(const char* name) { return ctors[name]; } +// static +template +napi_status Wrapping::Create(napi_env env, + napi_value js_rcv, + T* cc_rcv, + size_t same_obj_count) { + Wrapping* wrapping = new Wrapping; + wrapping->native = cc_rcv; + if (same_obj_count > 0) + wrapping->refs.resize(same_obj_count, nullptr); + return napi_wrap(env, js_rcv, wrapping, Destroy, nullptr, nullptr); +} + +// static +template +napi_status Wrapping::Retrieve(napi_env env, + napi_value js_rcv, + T** cc_rcv, + int ref_idx, + napi_value* ref, + Wrapping** get_wrapping) { + void* data = nullptr; + + napi_status status = napi_unwrap(env, js_rcv, &data); + if (status != napi_ok) return status; + + Wrapping*wrapping = static_cast*>(data); + + if (ref_idx >= 0 && + ref_idx < wrapping->refs.size() && + wrapping->refs[ref_idx] != nullptr) { + napi_value ref_value = nullptr; + + status = napi_get_reference_value(env, wrapping->refs[ref_idx], &ref_value); + if (status != napi_ok) return status; + + if (ref != nullptr) *ref = ref_value; + } + + if (get_wrapping != nullptr) *get_wrapping = wrapping; + *cc_rcv = wrapping->native; + return napi_ok; +} + +template +inline napi_status +Wrapping::SetRef(napi_env env, int idx, napi_value same_obj) { + napi_ref ref; + + napi_status status = napi_create_reference(env, same_obj, 1, &ref); + if (status != napi_ok) return status; + + refs[idx] = ref; + return napi_ok; +} + +// static +template +void Wrapping::Destroy(napi_env env, void* data, void* hint) { + (void) hint; + Wrapping* wrapping = static_cast*>(data); + + for (napi_ref ref: wrapping->refs) + if (ref != nullptr) + NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, ref)); + delete wrapping->native; + delete wrapping; +} + } // end of namespace WebIdlNapi #endif // WEBIDL_NAPI_INL_H diff --git a/webidl-napi.h b/webidl-napi.h index 6f25494..e4816d0 100644 --- a/webidl-napi.h +++ b/webidl-napi.h @@ -81,9 +81,6 @@ static napi_status IsConstructCall(napi_env env, const char* ifname, bool* result); -template -static void ObjectWrapDestructor(napi_env env, void* data, void* hint); - template class Converter { public: @@ -151,6 +148,26 @@ class InstanceData { napi_finalize cb = nullptr; }; +template +class Wrapping { + public: + static napi_status Create(napi_env env, + napi_value js_rcv, + T* cc_rcv, + size_t same_obj_count = 0); + static napi_status Retrieve(napi_env env, + napi_value js_rcv, + T** cc_rcv, + int ref_idx = -1, + napi_value* ref = nullptr, + Wrapping** wrapping = nullptr); + napi_status SetRef(napi_env env, int idx, napi_value same_obj); + private: + static void Destroy(napi_env env, void* data, void* hint); + T* native = nullptr; + std::vector refs; +}; + } // end of namespace WebIdlNapi #include "webidl-napi-inl.h"