From d4795cd90f27fab0e4a8ef5b5d9f4655ea00498c Mon Sep 17 00:00:00 2001 From: Taylor Woll Date: Tue, 16 Jan 2018 16:32:41 -0800 Subject: [PATCH] chakrashim: implement new.target support native callbacks (#450) Using the release/1.9 new Jsrt API to support callbacks which can receive new.target. --- deps/chakrashim/include/v8.h | 8 +++-- deps/chakrashim/src/v8functiontemplate.cc | 33 ++++++++++---------- deps/chakrashim/src/v8objecttemplate.cc | 38 +++++++++-------------- test/addons/addon.status | 1 - 4 files changed, 36 insertions(+), 44 deletions(-) diff --git a/deps/chakrashim/include/v8.h b/deps/chakrashim/include/v8.h index 7af48464b75..5d4e6f4cb03 100644 --- a/deps/chakrashim/include/v8.h +++ b/deps/chakrashim/include/v8.h @@ -1756,8 +1756,7 @@ class FunctionCallbackInfo { V8_INLINE Local operator[](int i) const; Local Callee() const { return _callee; } Local This() const { return _thisPointer; } - // TODO(jahorto): Implement this once JSRT gains new.target support - Local NewTarget() const { return nullptr; } + Local NewTarget() const { return _newTargetPointer; } Local Holder() const { return _holder; } bool IsConstructCall() const { return _isConstructorCall; } Local Data() const { return _data; } @@ -1771,6 +1770,7 @@ class FunctionCallbackInfo { Value** args, int length, Local _this, + Local _newTarget, Local holder, bool isConstructorCall, Local data, @@ -1778,6 +1778,7 @@ class FunctionCallbackInfo { : _args(args), _length(length), _thisPointer(_this), + _newTargetPointer(_newTarget), _holder(holder), _isConstructorCall(isConstructorCall), _data(data), @@ -1789,6 +1790,7 @@ class FunctionCallbackInfo { Value** _args; int _length; Local _thisPointer; + Local _newTargetPointer; Local _holder; bool _isConstructorCall; Local _data; @@ -2439,7 +2441,7 @@ class V8_EXPORT ObjectTemplate : public Template { friend class FunctionTemplateData; friend class Utils; - Local NewInstance(Handle prototype); + Local NewInstance(Handle constructor); void SetConstructor(Handle constructor); }; diff --git a/deps/chakrashim/src/v8functiontemplate.cc b/deps/chakrashim/src/v8functiontemplate.cc index abf2b863fee..c6bbcf9b832 100755 --- a/deps/chakrashim/src/v8functiontemplate.cc +++ b/deps/chakrashim/src/v8functiontemplate.cc @@ -70,10 +70,10 @@ class FunctionCallbackData : public ExternalData { this->prototype.Reset(nullptr, prototype); } - Local NewInstance() { + Local NewInstance(Handle newTarget) { Utils::EnsureObjectTemplate(&instanceTemplate); return !instanceTemplate.IsEmpty() ? - instanceTemplate->NewInstance(prototype) : Local(); + instanceTemplate->NewInstance(newTarget) : Local(); } bool CheckSignature(Local thisPointer, @@ -91,9 +91,9 @@ class FunctionCallbackData : public ExternalData { static JsValueRef CHAKRA_CALLBACK FunctionInvoked( JsValueRef callee, - bool isConstructCall, JsValueRef *arguments, unsigned short argumentCount, // NOLINT(runtime/int) + JsNativeFunctionInfo *info, void *callbackState) { CHAKRA_VERIFY(argumentCount >= 1); @@ -110,15 +110,16 @@ class FunctionCallbackData : public ExternalData { } Local thisPointer; - ++arguments; // skip the this argument + Local newTargetPointer = info->newTargetArg; + ++arguments; // skip arguments[0] - if (isConstructCall) { - thisPointer = callbackData->NewInstance(); + if (info->isConstructCall) { + thisPointer = callbackData->NewInstance(newTargetPointer); if (thisPointer.IsEmpty()) { return JS_INVALID_REFERENCE; } } else { - thisPointer = static_cast(arguments[-1]); + thisPointer = info->thisArg; } if (callbackData->callback != nullptr) { @@ -132,8 +133,9 @@ class FunctionCallbackData : public ExternalData { reinterpret_cast(arguments), argumentCount - 1, thisPointer, + newTargetPointer, holder, - isConstructCall, + info->isConstructCall, callbackData->data, static_cast(callee)); @@ -142,7 +144,7 @@ class FunctionCallbackData : public ExternalData { // if this is a regualr function call return the result, otherwise this is // a constructor call return the new instance - if (!isConstructCall) { + if (!info->isConstructCall) { return *result; } else if (!result.IsEmpty()) { if (!result->IsUndefined() && !result->IsNull()) { @@ -245,14 +247,11 @@ class FunctionTemplateData : public TemplateData { JsValueRef function = nullptr; { - if (!this->className.IsEmpty()) { - error = JsCreateNamedFunction(*this->className, - FunctionCallbackData::FunctionInvoked, - funcCallbackObjectRef, &function); - } else { - error = JsCreateFunction(FunctionCallbackData::FunctionInvoked, - funcCallbackObjectRef, &function); - } + error = JsCreateEnhancedFunction( + FunctionCallbackData::FunctionInvoked, + this->className.IsEmpty() ? JS_INVALID_REFERENCE : *this->className, + funcCallbackObjectRef, + &function); if (error != JsNoError) { return nullptr; diff --git a/deps/chakrashim/src/v8objecttemplate.cc b/deps/chakrashim/src/v8objecttemplate.cc index 96e795d6976..ab435502e28 100755 --- a/deps/chakrashim/src/v8objecttemplate.cc +++ b/deps/chakrashim/src/v8objecttemplate.cc @@ -847,14 +847,14 @@ JsValueRef CHAKRA_CALLBACK GetSelf( } MaybeLocal ObjectTemplate::NewInstance(Local context) { - return NewInstance(Local()); + return NewInstance(Local()); } Local ObjectTemplate::NewInstance() { return FromMaybe(NewInstance(Local())); } -Local ObjectTemplate::NewInstance(Handle prototype) { +Local ObjectTemplate::NewInstance(Handle constructor) { ObjectTemplateData* objectTemplateData = nullptr; if (!ExternalData::TryGet(this, &objectTemplateData)) { return Local(); @@ -869,31 +869,23 @@ Local ObjectTemplate::NewInstance(Handle prototype) { return Local(); } - // If there was no prototype specifically provided, then try to get one from - // the constructor, if it exists. It's possible that the constructor - // function doesn't have a prototype to provide. - if (prototype.IsEmpty()) { + if (constructor.IsEmpty()) { if (!objectTemplateData->constructor.IsEmpty()) { - Local function = objectTemplateData->constructor->GetFunction(); - - if (!function.IsEmpty()) { - jsrt::IsolateShim* iso = jsrt::IsolateShim::GetCurrent(); - JsValueRef prototypeValue = nullptr; - - if (JsGetProperty(*function, - iso->GetCachedPropertyIdRef( - jsrt::CachedPropertyIdRef::prototype), - &prototypeValue) == JsNoError) { - prototype = Local::New(prototypeValue); - } - } + constructor = objectTemplateData->constructor->GetFunction(); } } - if (!prototype.IsEmpty()) { - if (JsSetPrototype(newInstanceRef, - reinterpret_cast(*prototype)) != JsNoError) { - return Local(); + if (!constructor.IsEmpty()) { + jsrt::IsolateShim* iso = jsrt::IsolateShim::GetCurrent(); + JsValueRef prototypeValue = nullptr; + + if (JsGetProperty(*constructor, + iso->GetCachedPropertyIdRef( + jsrt::CachedPropertyIdRef::prototype), + &prototypeValue) == JsNoError) { + if (JsSetPrototype(newInstanceRef, prototypeValue) != JsNoError) { + return Local(); + } } } diff --git a/test/addons/addon.status b/test/addons/addon.status index 184b1219f6c..1687d37e3c1 100644 --- a/test/addons/addon.status +++ b/test/addons/addon.status @@ -22,7 +22,6 @@ prefix addons # These tests are failing for Node-Chakracore and should eventually be fixed async-hooks-promise/test : SKIP hello-world-esm/test : SKIP -new-target/test : SKIP callback-scope/test : SKIP callback-scope/test-resolve-async : SKIP