chakrashim: implement new.target support native callbacks (#450)

Using the release/1.9 new Jsrt API to support callbacks which can receive new.target.
nodejs · Jan 17, 2018 · a1be45b · a1be45b
1 parent 3097fb1
commit a1be45b
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 44 deletions.
diff --git a/deps/chakrashim/include/v8.h b/deps/chakrashim/include/v8.h
@@ -1756,8 +1756,7 @@ class FunctionCallbackInfo {
   V8_INLINE Local<Value> operator[](int i) const;
   Local<Function> Callee() const { return _callee; }
   Local<Object> This() const { return _thisPointer; }
-  // TODO(jahorto): Implement this once JSRT gains new.target support
-  Local<Value> NewTarget() const { return nullptr; }
+  Local<Value> NewTarget() const { return _newTargetPointer; }
   Local<Object> Holder() const { return _holder; }
   bool IsConstructCall() const { return _isConstructorCall; }
   Local<Value> Data() const { return _data; }
@@ -1771,13 +1770,15 @@ class FunctionCallbackInfo {
     Value** args,
     int length,
     Local<Object> _this,
+    Local<Object> _newTarget,
     Local<Object> holder,
     bool isConstructorCall,
     Local<Value> data,
     Local<Function> callee)
        : _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<Object> _thisPointer;
+  Local<Object> _newTargetPointer;
   Local<Object> _holder;
   bool _isConstructorCall;
   Local<Value> _data;
@@ -2439,7 +2441,7 @@ class V8_EXPORT ObjectTemplate : public Template {
   friend class FunctionTemplateData;
   friend class Utils;
 
-  Local<Object> NewInstance(Handle<Object> prototype);
+  Local<Object> NewInstance(Handle<Function> constructor);
   void SetConstructor(Handle<FunctionTemplate> constructor);
 };
 

diff --git 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<Object> NewInstance() {
+  Local<Object> NewInstance(Handle<Function> newTarget) {
     Utils::EnsureObjectTemplate(&instanceTemplate);
     return !instanceTemplate.IsEmpty() ?
-      instanceTemplate->NewInstance(prototype) : Local<Object>();
+      instanceTemplate->NewInstance(newTarget) : Local<Object>();
   }
 
   bool CheckSignature(Local<Object> 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<Object> thisPointer;
-    ++arguments;  // skip the this argument
+    Local<Function> 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<Object*>(arguments[-1]);
+      thisPointer = info->thisArg;
     }
 
     if (callbackData->callback != nullptr) {
@@ -132,8 +133,9 @@ class FunctionCallbackData : public ExternalData {
         reinterpret_cast<Value**>(arguments),
         argumentCount - 1,
         thisPointer,
+        newTargetPointer,
         holder,
-        isConstructCall,
+        info->isConstructCall,
         callbackData->data,
         static_cast<Function*>(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
@@ -847,14 +847,14 @@ JsValueRef CHAKRA_CALLBACK GetSelf(
 }
 
 MaybeLocal<Object> ObjectTemplate::NewInstance(Local<Context> context) {
-  return NewInstance(Local<Object>());
+  return NewInstance(Local<Function>());
 }
 
 Local<Object> ObjectTemplate::NewInstance() {
   return FromMaybe(NewInstance(Local<Context>()));
 }
 
-Local<Object> ObjectTemplate::NewInstance(Handle<Object> prototype) {
+Local<Object> ObjectTemplate::NewInstance(Handle<Function> constructor) {
   ObjectTemplateData* objectTemplateData = nullptr;
   if (!ExternalData::TryGet(this, &objectTemplateData)) {
     return Local<Object>();
@@ -869,31 +869,23 @@ Local<Object> ObjectTemplate::NewInstance(Handle<Object> prototype) {
     return Local<Object>();
   }
 
-  // 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> 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<Object>::New(prototypeValue);
-        }
-      }
+      constructor = objectTemplateData->constructor->GetFunction();
     }
   }
 
-  if (!prototype.IsEmpty()) {
-    if (JsSetPrototype(newInstanceRef,
-                       reinterpret_cast<JsValueRef>(*prototype)) != JsNoError) {
-      return Local<Object>();
+  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<Object>();
+        }
     }
   }
 

diff --git a/test/addons/addon.status b/test/addons/addon.status
@@ -22,6 +22,5 @@ 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